codeant-ai-for-open-source[bot] commented on code in PR #37890:
URL: https://github.com/apache/superset/pull/37890#discussion_r2798911342
##########
superset/db_engine_specs/kusto.py:
##########
@@ -201,6 +212,46 @@ def get_dbapi_exception_mapping(cls) ->
dict[type[Exception], type[Exception]]:
kusto_exceptions.ProgrammingError: SupersetDBAPIProgrammingError,
}
+ @classmethod
+ def handle_null_filter(
+ cls,
+ sqla_col: Any,
+ op: Any,
+ ) -> Any:
+ """
+ Handle null/not null filter operations for KQL.
+
+ In KQL, null checks use functions:
+ - isnull(col) for IS NULL
+ - isnotnull(col) for IS NOT NULL
+
+ :param sqla_col: SQLAlchemy column element
+ :param op: Filter operator (IS_NULL or IS_NOT_NULL)
+ :return: SQLAlchemy expression for the null filter
+ """
+ from superset.utils import core as utils
+
+ if op == utils.FilterOperator.IS_NULL:
+ return text(f"isnull({sqla_col})")
+ if op == utils.FilterOperator.IS_NOT_NULL:
+ return text(f"isnotnull({sqla_col})")
Review Comment:
**Suggestion:** Building the null filter with `text(f"isnull({sqla_col})")`
and `text(f"isnotnull({sqla_col})")` relies on the string representation of a
SQLAlchemy column, which is dialect-agnostic and can produce incorrectly quoted
or aliased identifiers in KQL, bypassing SQLAlchemy's compiler and potentially
resulting in invalid queries or broken bindings; instead, construct the
expression using SQLAlchemy function objects so the dialect can render it
correctly. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ KQL IS NULL filters may fail for quoted identifiers.
- ❌ KQL IS NOT NULL filters may fail for complex expressions.
- ⚠️ Dashboards using such filters can show empty/error states.
- ⚠️ Inconsistent identifier rendering between filters and main query.
```
</details>
```suggestion
from sqlalchemy import func
if op == utils.FilterOperator.IS_NULL:
return func.isnull(sqla_col)
if op == utils.FilterOperator.IS_NOT_NULL:
return func.isnotnull(sqla_col)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Kusto KQL database in Superset using the `KustoKqlEngineSpec`
engine
(`superset/db_engine_specs/kusto.py`, class `KustoKqlEngineSpec` where
`handle_null_filter` is defined around lines 215–239).
2. Create or edit a chart in Explore that uses this KQL-backed dataset and
add a filter
with operator `IS NULL` (or `IS NOT NULL`) on a column whose SQLAlchemy
object is not a
simple bare identifier, e.g. a column with spaces or reserved words that
requires
quoting/aliasing by the Kusto dialect.
3. When the query is built, Superset's generic query-building code (in the
engine-spec
layer defined in `superset/db_engine_specs/base.py`, which delegates
null-handling to
`handle_null_filter`) calls `KustoKqlEngineSpec.handle_null_filter(...)`
with the
SQLAlchemy column object `sqla_col` and the appropriate `FilterOperator`.
4. `handle_null_filter` at `superset/db_engine_specs/kusto.py:215-239`
constructs
`text(f"isnull({sqla_col})")` / `text(f"isnotnull({sqla_col})")`, which uses
`str(sqla_col)` under SQLAlchemy's default compilation instead of the Kusto
dialect; this
can yield incorrectly quoted or mismatched identifiers in the final KQL
string, causing
Kusto to return a syntax or "unknown column" error when the chart is run,
whereas using
`func.isnull(sqla_col)` / `func.isnotnull(sqla_col)` would let the Kusto
dialect render
the expression consistently with the rest of the query.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/kusto.py
**Line:** 233:237
**Comment:**
*Logic Error: Building the null filter with
`text(f"isnull({sqla_col})")` and `text(f"isnotnull({sqla_col})")` relies on
the string representation of a SQLAlchemy column, which is dialect-agnostic and
can produce incorrectly quoted or aliased identifiers in KQL, bypassing
SQLAlchemy's compiler and potentially resulting in invalid queries or broken
bindings; instead, construct the expression using SQLAlchemy function objects
so the dialect can render it correctly.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37890&comment_hash=3243fc2ca1ba321df511f2108fac1b372b6515d25c1b080328356ad534f4458f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37890&comment_hash=3243fc2ca1ba321df511f2108fac1b372b6515d25c1b080328356ad534f4458f&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]
