codeant-ai-for-open-source[bot] commented on code in PR #39976:
URL: https://github.com/apache/superset/pull/39976#discussion_r3226843141
##########
superset/sql/parse.py:
##########
@@ -722,7 +780,13 @@ def is_mutating(self) -> bool:
def format(self, comments: bool = True) -> str:
"""
Pretty-format the SQL statement.
+
+ When a string-rewriting operation (e.g. splice-mode RLS) has cached a
+ verbatim result in ``_raw_sql``, return it as-is — the whole point of
+ those operations is to avoid the dialect generator round-trip.
"""
+ if self._raw_sql is not None:
+ return self._raw_sql
Review Comment:
**Suggestion:** Returning cached raw SQL unconditionally ignores the
`comments` argument, so callers requesting comment-free SQL will still receive
comments whenever splice mode cached `_raw_sql`. This breaks the existing
contract of `format(comments=...)` and can surface invalid SQL for engines that
disallow comments. Honor the `comments` flag when `_raw_sql` is present (e.g.,
strip comments or regenerate appropriately for that case). [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Engines disallowing comments may receive SQL still containing comments.
- ⚠️ SQLScript.format(comments=False) no longer strips comments after RLS.
- ⚠️ Query rendering paths relying on comment stripping behave
inconsistently.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure or test a database engine with splice-mode RLS by setting
`database.db_engine_spec.rls_method = RLSMethod.AS_PREDICATE_SPLICE`, which
is the
intended override described in superset/db_engine_specs/base.py:21-25.
2. Execute a SQL query containing comments (e.g. `SELECT 1 -- inline
comment`) so it is
parsed into SQLScript/SQLStatement via SQLScript.__init__ at
superset/sql/parse.py:276-283
and SQLStatement.split_script/_parse at superset/sql/parse.py:117-189.
3. When superset/utils/rls.apply_rls() is called from either the SQL executor
(superset/sql/execution/executor.py:730-745) or SQL Lab path
(superset/sql_lab.py:433-437), it builds a non-empty predicates mapping and
invokes
`statement.apply_rls(catalog, schema, predicates, method)`
(superset/utils/rls.py:51-66);
for `method == RLSMethod.AS_PREDICATE_SPLICE`, SQLStatement.apply_rls() calls
`_apply_rls_splice()` which sets `self._raw_sql` to the spliced SQL string at
superset/sql/parse.py:172-179.
4. Later, callers that explicitly request comment-stripped SQL, such as
`_prepare_statement_blocks` in superset/sql/execution/celery_task.py:13-22
and the SQL Lab
block builder in superset/sql_lab.py:463-467, call
`statement.format(comments=db_engine_spec.allows_sql_comments)`;
SQLStatement.format() at
superset/sql/parse.py:51-66 returns `_raw_sql` unconditionally when it is
not None,
ignoring the `comments` flag, so even when `allows_sql_comments` is False
(e.g.
KustoSqlEngineSpec in superset/db_engine_specs/kusto.py:4-12) or when
query_render uses
`SQLScript(sql, engine).format(comments=False)`
(superset/sqllab/query_render.py:83-85),
the resulting SQL still contains comments contrary to the API contract.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fsql%2Fparse.py%0A%2A%2ALine%3A%2A%2A%20788%3A789%0A%2A%2AComment%3A%2A%2A%0A%09%2AApi%20Mismatch%3A%20Returning%20cached%20raw%20SQL%20unconditionally%20ignores%20the%20%60comments%60%20argument%2C%20so%20callers%20requesting%20comment-free%20SQL%20will%20still%20receive%20comments%20whenever%20splice%20mode%20cached%20%60_raw_sql%60.%20This%20breaks%20the%20existing%20contract%20of%20%60format%28comments%3D...%29%60%20and%20can%20surface%20invalid%20SQL%20for%20engines%20that%20disallow%20comments.%20Honor%20the%20%60comments%60%20flag%20when%20%60_raw_sql%60%20is%20present%20%28e.g.%2C%20strip%20comments%20or%20regenerate%20appropriately%20for%20that%20case%29.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20i
mplement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fsql%2Fparse.py%0A%2A%2ALine%3A%2A%2A%20788%3A789%0A%2A%2AComment%3A%2A%2A%0A%09%2AApi%20Mismatch%3A%20Returning%20cached%20raw%20SQL%20unconditionally%20ignores%20the%20%60comments%60%20argument%2C%20so%20callers%20requesting%20comment-free%20SQL%20will%20still%20receive%20comments%20whenever%20splice%20mode%20cached%20%60_raw_sql%60.%20This%20breaks%20the%20existing%20contract%20of%20%60format%28comments%3D...%29%60%20and%20c
an%20surface%20invalid%20SQL%20for%20engines%20that%20disallow%20comments.%20Honor%20the%20%60comments%60%20flag%20when%20%60_raw_sql%60%20is%20present%20%28e.g.%2C%20strip%20comments%20or%20regenerate%20appropriately%20for%20that%20case%29.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/sql/parse.py
**Line:** 788:789
**Comment:**
*Api Mismatch: Returning cached raw SQL unconditionally ignores the
`comments` argument, so callers requesting comment-free SQL will still receive
comments whenever splice mode cached `_raw_sql`. This breaks the existing
contract of `format(comments=...)` and can surface invalid SQL for engines that
disallow comments. Honor the `comments` flag when `_raw_sql` is present (e.g.,
strip comments or regenerate appropriately for that case).
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%2F39976&comment_hash=c0d6ace55b25b59000e0c23724eff414efc0d5ed1f3750dc791e5bca880b6a4f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39976&comment_hash=c0d6ace55b25b59000e0c23724eff414efc0d5ed1f3750dc791e5bca880b6a4f&reaction=dislike'>👎</a>
##########
superset/sql/parse.py:
##########
@@ -626,10 +639,55 @@ def split_script(
script: str,
engine: str,
) -> list[SQLStatement]:
+ asts = [ast for ast in cls._parse(script, engine) if ast]
+ sources = cls._split_source(script, engine, len(asts))
return [
- cls(ast=ast, engine=engine) for ast in cls._parse(script, engine)
if ast
+ cls(ast=ast, engine=engine, source=source)
+ for ast, source in zip(asts, sources, strict=False)
]
+ @classmethod
+ def _split_source(
+ cls,
+ script: str,
+ engine: str,
+ expected_count: int,
+ ) -> list[str | None]:
+ """
+ Slice ``script`` into per-statement substrings using top-level
semicolon
+ positions from the tokenizer. Returns a list of length
``expected_count``;
+ any entry is ``None`` if the slicing didn't yield a usable substring.
+
+ The returned substrings preserve the original byte content of the
script
+ for each statement — necessary for splice-mode RLS, which rewrites the
+ original SQL rather than regenerating from the AST.
+ """
+ none_result: list[str | None] = [None] * expected_count
+ dialect = SQLGLOT_DIALECTS.get(engine)
+ try:
+ tokens = list(Dialect.get_or_raise(dialect).tokenize(script))
+ except sqlglot.errors.SqlglotError:
+ return none_result
+
+ # Top-level semicolon offsets (depth 0).
+ boundaries: list[int] = []
+ depth = 0
+ for tok in tokens:
+ if tok.token_type == sqlglot.tokens.TokenType.L_PAREN:
+ depth += 1
+ elif tok.token_type == sqlglot.tokens.TokenType.R_PAREN:
+ depth -= 1
+ elif tok.token_type == sqlglot.tokens.TokenType.SEMICOLON and
depth == 0:
+ boundaries.append(tok.start)
+
+ starts = [0, *(b + 1 for b in boundaries)]
+ ends = [*boundaries, len(script)]
+ sources = [script[s:e].strip() for s, e in zip(starts, ends,
strict=False)]
+ sources = [s for s in sources if s]
+ if len(sources) != expected_count:
+ return none_result
Review Comment:
**Suggestion:** `_split_source` drops empty slices and then requires an
exact count match with parsed AST statements; scripts with trailing comment
segments after semicolons can produce an extra non-empty slice, causing all
sources to fall back to `None`. That later makes splice-mode RLS fail with
"requires the source SQL string" for otherwise valid SQL scripts. Make source
splitting robust to trailing comment-only fragments instead of invalidating all
statements. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Multi-statement scripts with trailing comments break splice-mode RLS.
- ⚠️ Valid SQL scripts can fail with internal ValueError.
- ⚠️ Future engines using splice method see reduced robustness.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Submit a multi-statement SQL script with trailing comments through a real
entrypoint
such as SQL Lab or the SQL executor, for example the pattern used in
tests/unit_tests/sql/parse_tests.py:55-62: `-- comment\nSET @value =
42;\nSELECT @value as
foo;\n-- trailing comment`.
2. The script is parsed by `SQLScript(rendered_sql, engine)`
(superset/sql/parse.py:276-283), which for SQL engines uses
SQLStatement.split_script();
split_script() calls SQLStatement._parse to get a list of AST statements and
then calls
`_split_source(script, engine, len(asts))` at superset/sql/parse.py:117-125.
3. Inside `_split_source`, the tokenizer finds top-level semicolon positions
(superset/sql/parse.py:153-162) and computes `starts`/`ends`; `sources =
[script[s:e].strip() for s, e in zip(starts, ends, strict=False)]` at line
685 yields one
slice per semicolon-separated region, including a non-empty trailing
comment-only slice
after the final semicolon, and the subsequent filter `sources = [s for s in
sources if s]`
at line 686 keeps all of them so `len(sources)` becomes three while
`expected_count` (the
number of parsed AST statements) is only two.
4. Because `len(sources) != expected_count`, `_split_source` returns a list
of Nones at
line 688, so each SQLStatement created in split_script has `_source_sql` set
to None; when
a database engine later enables splice-mode RLS
(RLSMethod.AS_PREDICATE_SPLICE) and
superset/utils/rls.apply_rls() calls `statement.apply_rls(...)`
(superset/utils/rls.py:49-66), `_apply_rls_splice` in
superset/sql/parse.py:151-170 raises
the "Splice-mode RLS requires the source SQL string" ValueError for
otherwise valid
scripts that merely include trailing comment fragments.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fsql%2Fparse.py%0A%2A%2ALine%3A%2A%2A%20685%3A688%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20%60_split_source%60%20drops%20empty%20slices%20and%20then%20requires%20an%20exact%20count%20match%20with%20parsed%20AST%20statements%3B%20scripts%20with%20trailing%20comment%20segments%20after%20semicolons%20can%20produce%20an%20extra%20non-empty%20slice%2C%20causing%20all%20sources%20to%20fall%20back%20to%20%60None%60.%20That%20later%20makes%20splice-mode%20RLS%20fail%20with%20%22requires%20the%20source%20SQL%20string%22%20for%20otherwise%20valid%20SQL%20scripts.%20Make%20source%20splitting%20robust%20to%20trailing%20comment-only%20fragments%20instead%20of%20invalidating%20all%20statements.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20yo
u%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fsql%2Fparse.py%0A%2A%2ALine%3A%2A%2A%20685%3A688%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20%60_split_source%60%20drops%20empty%20slices%20and%20then%20requires%20an%20exact%20count%20match%20with%20parsed%20AST%20statements%3B%20scripts%20with%20trailing%20comment%20segments%20after%20semicolons%20can%20produce%20an%20extra%20non-empty%20slice%2C%20causing%20all%20sources%20to%20fall%20bac
k%20to%20%60None%60.%20That%20later%20makes%20splice-mode%20RLS%20fail%20with%20%22requires%20the%20source%20SQL%20string%22%20for%20otherwise%20valid%20SQL%20scripts.%20Make%20source%20splitting%20robust%20to%20trailing%20comment-only%20fragments%20instead%20of%20invalidating%20all%20statements.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/sql/parse.py
**Line:** 685:688
**Comment:**
*Logic Error: `_split_source` drops empty slices and then requires an
exact count match with parsed AST statements; scripts with trailing comment
segments after semicolons can produce an extra non-empty slice, causing all
sources to fall back to `None`. That later makes splice-mode RLS fail with
"requires the source SQL string" for otherwise valid SQL scripts. Make source
splitting robust to trailing comment-only fragments instead of invalidating all
statements.
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%2F39976&comment_hash=2b73827ec4fc8a7e60273ddec5024ae2d6edfd1f381dc8b537bbef4e651fc495&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39976&comment_hash=2b73827ec4fc8a7e60273ddec5024ae2d6edfd1f381dc8b537bbef4e651fc495&reaction=dislike'>👎</a>
##########
superset/sql/parse.py:
##########
@@ -902,29 +973,70 @@ def apply_rls(
self,
catalog: str | None,
schema: str | None,
- predicates: dict[Table, list[exp.Expression]],
+ predicates: dict[Table, list[str]],
method: RLSMethod,
) -> None:
"""
Apply relevant RLS rules to the statement inplace.
:param catalog: The default catalog for non-qualified table names
:param schema: The default schema for non-qualified table names
+ :param predicates: Mapping of fully qualified ``Table`` to raw
predicate
+ SQL strings.
:param method: The method to use for applying the rules.
"""
if not predicates:
return
+ if method == RLSMethod.AS_PREDICATE_SPLICE:
+ self._apply_rls_splice(catalog, schema, predicates)
+ return
Review Comment:
**Suggestion:** The guard only checks whether the `predicates` dict is
empty, but callers populate it with every referenced table even when each value
is an empty list. That means splice mode still runs on "no-op" RLS, which can
unexpectedly raise `ValueError` when `_source_sql` is missing and can also
alter formatting behavior even though no predicates exist. Short-circuit when
there are no actual predicate strings (for example `not
any(predicates.values())`) before entering splice mode. [incorrect condition
logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ No-op RLS with splice can raise ValueError unexpectedly.
- ⚠️ Utils.apply_rls reports False but underlying call fails.
- ⚠️ AST-only statements cannot safely opt into splice.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Construct a SQLStatement from an AST without a source string as in
tests/unit_tests/sql/parse_tests.py:2733-2734, e.g. `ast = parse_one("SELECT
* FROM
some_table")` and `statement = SQLStatement(ast=ast, engine="postgresql")`,
which leaves
`BaseSQLStatement._source_sql` as `None` (superset/sql/parse.py:354-379).
2. Build a predicates mapping that has a key but no predicate strings, e.g.
`{Table("some_table", "schema1", "catalog1"): []}`, matching how
superset/utils/rls.apply_rls() constructs `predicates` with potentially
empty lists per
table (superset/utils/rls.py:51-64).
3. Call `statement.apply_rls("catalog1", "schema1", predicates,
RLSMethod.AS_PREDICATE_SPLICE)` which executes SQLStatement.apply_rls() at
superset/sql/parse.py:972-149; because the dict is non-empty, the guard at
line 988 `if
not predicates:` does not short-circuit.
4. With `method == RLSMethod.AS_PREDICATE_SPLICE`, `_apply_rls_splice()` is
invoked
(superset/sql/parse.py:151-179), immediately checks `self._source_sql` and
raises a
ValueError at line 167 even though there are no actual predicate strings, so
a logical
no-op RLS call fails; the same pattern will occur via
superset/utils/rls.apply_rls()
(superset/utils/rls.py:49-66) once a database engine spec sets `rls_method =
RLSMethod.AS_PREDICATE_SPLICE` (superset/db_engine_specs/base.py:21-25).
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fsql%2Fparse.py%0A%2A%2ALine%3A%2A%2A%20988%3A993%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncorrect%20Condition%20Logic%3A%20The%20guard%20only%20checks%20whether%20the%20%60predicates%60%20dict%20is%20empty%2C%20but%20callers%20populate%20it%20with%20every%20referenced%20table%20even%20when%20each%20value%20is%20an%20empty%20list.%20That%20means%20splice%20mode%20still%20runs%20on%20%22no-op%22%20RLS%2C%20which%20can%20unexpectedly%20raise%20%60ValueError%60%20when%20%60_source_sql%60%20is%20missing%20and%20can%20also%20alter%20formatting%20behavior%20even%20though%20no%20predicates%20exist.%20Short-circuit%20when%20there%20are%20no%20actual%20predicate%20strings%20%28for%20example%20%60not%20any%28predicates.values%28%29%29%60%29%20before%20entering%20splice%20mode.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%2
0issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fsql%2Fparse.py%0A%2A%2ALine%3A%2A%2A%20988%3A993%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncorrect%20Condition%20Logic%3A%20The%20guard%20only%20checks%20whether%20the%20%60predicates%60%20dict%20is%20empty%2C%20but%20callers%20populate%20it%20with%20every%20referenced%20table%20even%20when%20each%20value%20is%20an%20empty%20list.%20Tha
t%20means%20splice%20mode%20still%20runs%20on%20%22no-op%22%20RLS%2C%20which%20can%20unexpectedly%20raise%20%60ValueError%60%20when%20%60_source_sql%60%20is%20missing%20and%20can%20also%20alter%20formatting%20behavior%20even%20though%20no%20predicates%20exist.%20Short-circuit%20when%20there%20are%20no%20actual%20predicate%20strings%20%28for%20example%20%60not%20any%28predicates.values%28%29%29%60%29%20before%20entering%20splice%20mode.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/sql/parse.py
**Line:** 988:993
**Comment:**
*Incorrect Condition Logic: The guard only checks whether the
`predicates` dict is empty, but callers populate it with every referenced table
even when each value is an empty list. That means splice mode still runs on
"no-op" RLS, which can unexpectedly raise `ValueError` when `_source_sql` is
missing and can also alter formatting behavior even though no predicates exist.
Short-circuit when there are no actual predicate strings (for example `not
any(predicates.values())`) before entering splice mode.
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%2F39976&comment_hash=07faeffb047de3986b9f104cb47c1cc2f0880e59a5699477038aa38fa085c154&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39976&comment_hash=07faeffb047de3986b9f104cb47c1cc2f0880e59a5699477038aa38fa085c154&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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
