You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "hy144328 (via GitHub)" <gi...@apache.org> on 2026/07/12 17:49:18 UTC
[PR] refactor: begin transaction to automatically commit [superset]
hy144328 opened a new pull request, #41979:
URL: https://github.com/apache/superset/pull/41979
See #40273
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "hy144328 (via GitHub)" <gi...@apache.org>.
hy144328 commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4962931754
> LGTM, just needs a rebase. Making great progress!
Force-pushed rebase to resolve merge conflicts.
Otherwise, still good to go. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4965411970
Rebased and resolved conflicts... hopefully CI passes now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4965417862
Haven't investigated it, but[ this bot comment](https://github.com/apache/superset/pull/41979/#discussion_r3574368604) does sound like it's worth looking into.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3566807555
##########
superset/extensions/metadb.py:
##########
@@ -445,15 +445,16 @@ def insert_row(self, row: Row) -> int:
query = self._table.insert().values(**row)
with self.engine_context() as engine:
- connection = engine.connect()
- result = connection.execute(query)
+ with engine.begin() as connection:
+ result = connection.execute(query)
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced execution result variable to satisfy the mandatory type-hinting requirement. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is also newly added Python code, and `result` is a local variable that can be annotated but currently is not. The omission fits the custom type-hinting rule.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7ca38f062afe4047ad6a4f64b598f060&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7ca38f062afe4047ad6a4f64b598f060&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/metadb.py
**Line:** 449:449
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced execution result variable to satisfy the mandatory type-hinting requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=df91ed4b174a674a71f785dc8ab1764254f5716624adbfd159e40fef388d4928&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=df91ed4b174a674a71f785dc8ab1764254f5716624adbfd159e40fef388d4928&reaction=dislike'>๐</a>
##########
superset/extensions/metadb.py:
##########
@@ -417,12 +417,12 @@ def get_data(
query = self._build_sql(bounds, order, limit, offset)
with self.engine_context() as engine:
- connection = engine.connect()
- rows = connection.execute(query)
- for i, row in enumerate(rows):
- data = dict(zip(self.columns, row, strict=False))
- data["rowid"] = data[self._rowid] if self._rowid else i
- yield data
+ with engine.connect() as connection:
+ rows = connection.execute(query)
Review Comment:
**Suggestion:** Add an explicit type annotation to this local query result variable so it complies with the required type-hinting rule. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is newly added Python code and the local variable `rows` is type-annotatable, but it is introduced without an explicit type hint. That matches the rule requiring type hints on relevant new variables.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=582131e7be964a6ebef31be9b80a6560&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=582131e7be964a6ebef31be9b80a6560&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/metadb.py
**Line:** 421:421
**Comment:**
*Custom Rule: Add an explicit type annotation to this local query result variable so it complies with the required type-hinting rule.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=0dead9e7b7013ec70140ab8cbd4e434e745c054bcfa50d34a8476c6584db0e4f&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=0dead9e7b7013ec70140ab8cbd4e434e745c054bcfa50d34a8476c6584db0e4f&reaction=dislike'>๐</a>
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -137,9 +141,9 @@ def test_superset(mocker: MockerFixture, app_context: None, table1: None) -> Non
# Skip test if superset:// dialect can't be loaded (common in Docker)
pytest.skip(f"Superset dialect not available: {e}")
- conn = engine.connect()
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20)]
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
Review Comment:
**Suggestion:** Add an explicit type annotation for `results` when assigning query output so the new variable follows the type-hint requirement. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The new local variable `results` is introduced without any type annotation in modified Python code. Under the stated type-hint rule, this is a relevant variable that can be annotated, so the suggestion matches a real violation.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d849a841ad1c4aa0bda1c29175de47c0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d849a841ad1c4aa0bda1c29175de47c0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/extensions/test_sqlalchemy.py
**Line:** 145:145
**Comment:**
*Custom Rule: Add an explicit type annotation for `results` when assigning query output so the new variable follows the type-hint requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=b22dd4f22c2d25412736b232f366ac3ff497be1cc019c133caca560e2f9c612e&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=b22dd4f22c2d25412736b232f366ac3ff497be1cc019c133caca560e2f9c612e&reaction=dislike'>๐</a>
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -178,9 +182,9 @@ def test_superset_limit(mocker: MockerFixture, app_context: None, table1: None)
# Skip test if superset:// dialect can't be loaded (common in Docker)
pytest.skip(f"Superset dialect not available: {e}")
- conn = engine.connect()
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10)]
+ with engine.connect() as conn:
Review Comment:
**Suggestion:** Add an explicit type annotation for `results` in this new assignment to keep local variables in modified code type-hinted. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is another newly added `results` assignment in modified Python code, and it also lacks an explicit type annotation. That fits the type-hint rule for relevant variables.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dde051e6056a4362ab9b0fa86d1b3239&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dde051e6056a4362ab9b0fa86d1b3239&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/extensions/test_sqlalchemy.py
**Line:** 185:185
**Comment:**
*Custom Rule: Add an explicit type annotation for `results` in this new assignment to keep local variables in modified code type-hinted.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=11e9dd59538ccf5eafb53d9e6bf5a1dffc49a3e3f01c7cd01b9d1acf3820a58e&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=11e9dd59538ccf5eafb53d9e6bf5a1dffc49a3e3f01c7cd01b9d1acf3820a58e&reaction=dislike'>๐</a>
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -257,22 +261,27 @@ def test_dml(
# Skip test if superset:// dialect can't be loaded (common in Docker)
pytest.skip(f"Superset dialect not available: {e}")
- conn = engine.connect()
-
- conn.execute(text('INSERT INTO "database1.table1" (a, b) VALUES (3, 30)'))
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20), (3, 30)]
- conn.execute(text('UPDATE "database1.table1" SET b=35 WHERE a=3'))
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20), (3, 35)]
- conn.execute(text('DELETE FROM "database1.table1" WHERE b>20'))
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20)]
-
- with pytest.raises(ProgrammingError) as excinfo:
- conn.execute(
- text("""INSERT INTO "database2.table2" (a, b) VALUES (3, 'thirty')""")
- )
+ with engine.begin() as conn:
+ conn.execute(text('INSERT INTO "database1.table1" (a, b) VALUES (3, 30)'))
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
+ assert list(results) == [(1, 10), (2, 20), (3, 30)]
+ with engine.begin() as conn:
+ conn.execute(text('UPDATE "database1.table1" SET b=35 WHERE a=3'))
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
+ assert list(results) == [(1, 10), (2, 20), (3, 35)]
+ with engine.begin() as conn:
+ conn.execute(text('DELETE FROM "database1.table1" WHERE b>20'))
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
+ assert list(results) == [(1, 10), (2, 20)]
+
+ with engine.begin() as conn:
+ with pytest.raises(ProgrammingError) as excinfo:
Review Comment:
**Suggestion:** Add an explicit type annotation for `excinfo` in this new context-manager binding to satisfy the type-hint rule for relevant variables. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The code introduces `excinfo` as a new variable in modified Python code without any annotation. Since the rule covers relevant variables that can be annotated, this is a valid type-hint finding.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7c92e82808cf443fa1edb9c7fb80d89e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7c92e82808cf443fa1edb9c7fb80d89e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/extensions/test_sqlalchemy.py
**Line:** 281:281
**Comment:**
*Custom Rule: Add an explicit type annotation for `excinfo` in this new context-manager binding to satisfy the type-hint rule for relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=6be038d7a635fd440ba51be27566dc64656e2183e64483d99f69c52c12040551&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=6be038d7a635fd440ba51be27566dc64656e2183e64483d99f69c52c12040551&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "hy144328 (via GitHub)" <gi...@apache.org>.
hy144328 commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3566810688
##########
superset/extensions/metadb.py:
##########
@@ -417,12 +417,12 @@ def get_data(
query = self._build_sql(bounds, order, limit, offset)
with self.engine_context() as engine:
- connection = engine.connect()
- rows = connection.execute(query)
- for i, row in enumerate(rows):
- data = dict(zip(self.columns, row, strict=False))
- data["rowid"] = data[self._rowid] if self._rowid else i
- yield data
+ with engine.connect() as connection:
+ rows = connection.execute(query)
Review Comment:
Nice to have but not a regression to the status quo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41979:
URL: https://github.com/apache/superset/pull/41979
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3574368604
##########
superset/extensions/metadb.py:
##########
@@ -445,15 +445,16 @@ def insert_row(self, row: Row) -> int:
query = self._table.insert().values(**row)
with self.engine_context() as engine:
- connection = engine.connect()
- result = connection.execute(query)
+ with engine.begin() as connection:
+ result = connection.execute(query)
- # return rowid
- if self._rowid:
- return result.inserted_primary_key[0]
+ # return rowid
+ if self._rowid:
+ return result.inserted_primary_key[0]
query = select(func.count()).select_from(self._table)
- return connection.execute(query).scalar()
+ with engine.connect() as connection:
+ return connection.execute(query).scalar()
Review Comment:
**Suggestion:** When there is no integer primary key, the returned row identifier is derived from a separate `COUNT(*)` query after the insert transaction has already committed. Under concurrent inserts/deletes this count can change between statements, so the method may return a row id that does not correspond to the row just inserted. Compute and return the fallback identifier within the same transactional scope (or use a deterministic per-row key) to avoid cross-request races. [race condition]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ Superset meta DB INSERTs can return wrong row identifiers.
- โ ๏ธ Shillelagh virtual tables observe nondeterministic row index values.
- โ ๏ธ Cross-database experimental feature behaves unpredictably under concurrency.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Enable the Superset meta database feature flag `ENABLE_SUPERSET_META_DB` (see
superset/config.py:768) and configure a `Database` record with `allow_dml=True` so that
`SupersetShillelaghAdapter._set_columns` (superset/extensions/metadb.py:299-308) sets
`self._allow_dml` and leaves `self._rowid` as `None` for a table without a single integer
primary key (superset/extensions/metadb.py:276, 344-350).
2. Use the experimental `superset://` SQLAlchemy dialect (registered in setup.py:8-12 and
described in superset/db_engine_specs/superset.py:12-17) to open two concurrent client
connections that both issue `INSERT` statements into the same Superset meta DB virtual
table backed by the non-rowid underlying table configured in step 1.
3. For each `INSERT`, the Shillelagh engine invokes `SupersetShillelaghAdapter.insert_row`
(superset/extensions/metadb.py:428-79). Because `self._rowid` is `None`, after executing
the insert inside `with engine.begin() as connection:` (lines 68-71), control falls
through to the fallback branch at lines 455-457, which builds
`select(func.count()).select_from(self._table)` and executes it on a fresh
`engine.connect()` connection after the transaction has already committed.
4. If, between the commit in line 69-71 and the `COUNT(*)` execution in lines 455-457,
another client inserts or deletes rows on the same table, the returned scalar count
reflects the concurrent mutations rather than just the row just inserted, so the integer
returned by `insert_row` no longer uniquely identifies the newly inserted row and can
instead represent a different row position, yielding a non-deterministic and potentially
incorrect row identifier under concurrent access.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ec11b598e4e640198b29ead1e7a8aec2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ec11b598e4e640198b29ead1e7a8aec2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/extensions/metadb.py
**Line:** 455:457
**Comment:**
*Race Condition: When there is no integer primary key, the returned row identifier is derived from a separate `COUNT(*)` query after the insert transaction has already committed. Under concurrent inserts/deletes this count can change between statements, so the method may return a row id that does not correspond to the row just inserted. Compute and return the fallback identifier within the same transactional scope (or use a deterministic per-row key) to avoid cross-request races.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=055fea0f12b8f7ff7ffa326c69c4cdf60fb0dc1df45d3c859600408f1acd30a0&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=055fea0f12b8f7ff7ffa326c69c4cdf60fb0dc1df45d3c859600408f1acd30a0&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "hy144328 (via GitHub)" <gi...@apache.org>.
hy144328 commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3566810207
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -137,9 +141,9 @@ def test_superset(mocker: MockerFixture, app_context: None, table1: None) -> Non
# Skip test if superset:// dialect can't be loaded (common in Docker)
pytest.skip(f"Superset dialect not available: {e}")
- conn = engine.connect()
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20)]
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
Review Comment:
Nice to have but not a regression to the status quo.
##########
superset/extensions/metadb.py:
##########
@@ -445,15 +445,16 @@ def insert_row(self, row: Row) -> int:
query = self._table.insert().values(**row)
with self.engine_context() as engine:
- connection = engine.connect()
- result = connection.execute(query)
+ with engine.begin() as connection:
+ result = connection.execute(query)
Review Comment:
Nice to have but not a regression to the status quo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4952162890
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4952911399
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "hy144328 (via GitHub)" <gi...@apache.org>.
hy144328 commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3611293184
##########
superset/extensions/metadb.py:
##########
@@ -445,15 +445,16 @@ def insert_row(self, row: Row) -> int:
query = self._table.insert().values(**row)
with self.engine_context() as engine:
- connection = engine.connect()
- result = connection.execute(query)
+ with engine.begin() as connection:
+ result = connection.execute(query)
- # return rowid
- if self._rowid:
- return result.inserted_primary_key[0]
+ # return rowid
+ if self._rowid:
+ return result.inserted_primary_key[0]
query = select(func.count()).select_from(self._table)
- return connection.execute(query).scalar()
+ with engine.connect() as connection:
+ return connection.execute(query).scalar()
Review Comment:
The scope of this PR is a refactor.
The status quo is non-transactional auto-commit. https://docs.sqlalchemy.org/en/14/core/connections.html#setting-transaction-isolation-levels-including-dbapi-autocommit
Thus, not putting DML and DQL in a single transaction is not a regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4952176274
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41979?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `16 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.99%. Comparing base ([`13c5a32`](https://app.codecov.io/gh/apache/superset/commit/13c5a3240274973db7bbc90f804a18e47526ec60?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2705d72`](https://app.codecov.io/gh/apache/superset/commit/2705d721bd0cbc9a57765713e5e32294c39952cd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/41979?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/extensions/metadb.py](https://app.codecov.io/gh/apache/superset/pull/41979?src=pr&el=tree&filepath=superset%2Fextensions%2Fmetadb.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZXh0ZW5zaW9ucy9tZXRhZGIucHk=) | 0.00% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/41979?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (13c5a32) and HEAD (2705d72). Click for more details.
>
> <details><summary>HEAD has 38 uploads less than BASE</summary>
>
>| Flag | BASE (13c5a32) | HEAD (2705d72) |
>|------|------|------|
>|hive|5|1|
>|python|32|2|
>|presto|5|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41979 +/- ##
==========================================
- Coverage 64.88% 55.99% -8.90%
==========================================
Files 2742 2742
Lines 153378 153379 +1
Branches 35168 35168
==========================================
- Hits 99516 85880 -13636
- Misses 51958 66673 +14715
+ Partials 1904 826 -1078
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage ฮ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.07% <0.00%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.06% <0.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.11% <0.00%> (-18.30%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/41979/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41979?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "hy144328 (via GitHub)" <gi...@apache.org>.
hy144328 commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3566809676
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -257,22 +261,27 @@ def test_dml(
# Skip test if superset:// dialect can't be loaded (common in Docker)
pytest.skip(f"Superset dialect not available: {e}")
- conn = engine.connect()
-
- conn.execute(text('INSERT INTO "database1.table1" (a, b) VALUES (3, 30)'))
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20), (3, 30)]
- conn.execute(text('UPDATE "database1.table1" SET b=35 WHERE a=3'))
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20), (3, 35)]
- conn.execute(text('DELETE FROM "database1.table1" WHERE b>20'))
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10), (2, 20)]
-
- with pytest.raises(ProgrammingError) as excinfo:
- conn.execute(
- text("""INSERT INTO "database2.table2" (a, b) VALUES (3, 'thirty')""")
- )
+ with engine.begin() as conn:
+ conn.execute(text('INSERT INTO "database1.table1" (a, b) VALUES (3, 30)'))
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
+ assert list(results) == [(1, 10), (2, 20), (3, 30)]
+ with engine.begin() as conn:
+ conn.execute(text('UPDATE "database1.table1" SET b=35 WHERE a=3'))
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
+ assert list(results) == [(1, 10), (2, 20), (3, 35)]
+ with engine.begin() as conn:
+ conn.execute(text('DELETE FROM "database1.table1" WHERE b>20'))
+ with engine.connect() as conn:
+ results = conn.execute(text('SELECT * FROM "database1.table1"'))
+ assert list(results) == [(1, 10), (2, 20)]
+
+ with engine.begin() as conn:
+ with pytest.raises(ProgrammingError) as excinfo:
Review Comment:
Nice to have but not a regression to the status quo.
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -178,9 +182,9 @@ def test_superset_limit(mocker: MockerFixture, app_context: None, table1: None)
# Skip test if superset:// dialect can't be loaded (common in Docker)
pytest.skip(f"Superset dialect not available: {e}")
- conn = engine.connect()
- results = conn.execute(text('SELECT * FROM "database1.table1"'))
- assert list(results) == [(1, 10)]
+ with engine.connect() as conn:
Review Comment:
Nice to have but not a regression to the status quo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4964245344
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41979:
URL: https://github.com/apache/superset/pull/41979#issuecomment-4966500527
**Bito Automatic Review Skipped โ PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] refactor: begin transaction to automatically commit [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41979:
URL: https://github.com/apache/superset/pull/41979#discussion_r3574330954
##########
tests/unit_tests/db_engine_specs/test_sqlite.py:
##########
@@ -121,12 +121,13 @@ def test_time_grain_expressions(dttm: str, grain: str, expected: str) -> None:
from superset.db_engine_specs.sqlite import SqliteEngineSpec
engine = create_engine("sqlite://")
- connection = engine.connect()
- connection.execute(text("CREATE TABLE t (dttm DATETIME)"))
- connection.execute(text("INSERT INTO t VALUES (:dttm)"), {"dttm": dttm})
+ with engine.begin() as connection:
+ connection.execute(text("CREATE TABLE t (dttm DATETIME)"))
+ connection.execute(text("INSERT INTO t VALUES (:dttm)"), {"dttm": dttm})
# pylint: disable=protected-access
expression = SqliteEngineSpec._time_grain_expressions[grain].format(col="dttm")
sql = f"SELECT {expression} FROM t" # noqa: S608
- result = connection.execute(text(sql)).scalar()
+ with engine.connect() as connection:
+ result = connection.execute(text(sql)).scalar()
Review Comment:
**Suggestion:** Add an explicit type annotation for this newly introduced local value to satisfy the type-hint requirement for relevant variables. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is newly added Python code introducing a local variable without an annotation, and the project rule requires type hints for relevant variables that can be annotated. The variable `result` is a clear candidate for a type hint, so the violation is real.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e43473a4b326473099be2cae159804e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e43473a4b326473099be2cae159804e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/db_engine_specs/test_sqlite.py
**Line:** 132:132
**Comment:**
*Custom Rule: Add an explicit type annotation for this newly introduced local value to satisfy the type-hint requirement for relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=6c9ed7952a9c905582dfbdc4036a9d0046639f51d554280d1c8c0ee120bb0c05&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41979&comment_hash=6c9ed7952a9c905582dfbdc4036a9d0046639f51d554280d1c8c0ee120bb0c05&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org