codeant-ai-for-open-source[bot] commented on code in PR #42752:
URL: https://github.com/apache/superset/pull/42752#discussion_r3710423540
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -2683,6 +2683,54 @@ def
test_calculated_column_non_boolean_filter_is_parenthesized(
f"Generated SQL: {sql}"
)
+def test_get_sqla_query_in_filter_preserves_float_precision(
+ database: Database,
+) -> None:
+ """
+ Test that mixing integer and float values in an "IN" filter does not
+ truncate the float value's decimal precision when the compiled query
+ binds parameters (see #33206).
+ """
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ table = SqlaTable(
+ database=database,
+ schema=None,
+ table_name="t",
+ columns=[
+ TableColumn(column_name="global_sales", type="FLOAT"),
+ ],
+ )
+
+ sqla_query = table.get_sqla_query(
+ columns=["global_sales"],
+ filter=[
+ {
+ "col": "global_sales",
+ "op": "IN",
+ "val": [33, 29.02],
+ },
+ ],
+ extras={},
+ is_timeseries=False,
+ metrics=[],
+ )
+
+ with database.get_sqla_engine() as engine:
+ sql = str(
+ sqla_query.sqla_query.compile(
+ dialect=engine.dialect,
+ compile_kwargs={"literal_binds": True},
+ )
Review Comment:
**Suggestion:** Using `literal_binds=True` removes the bind-parameter path
that caused the production regression, because SQLAlchemy renders the Python
values directly into the SQL text. This assertion can pass even when execution
with bound parameters still infers an integer type from the first value and
truncates the decimal. Assert the generated bind parameter types/values or
execute the query against an engine that reproduces the affected behavior.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Regression test does not cover bound-parameter execution.
- ⚠️ Precision truncation could regress undetected in numeric IN filters.
- ⚠️ Query correctness remains unverified for affected database dialects.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e5d484d4932c4b15929cda444caf18ca&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=e5d484d4932c4b15929cda444caf18ca&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/models/helpers_test.py
**Line:** 2721:2724
**Comment:**
*Incomplete Implementation: Using `literal_binds=True` removes the
bind-parameter path that caused the production regression, because SQLAlchemy
renders the Python values directly into the SQL text. This assertion can pass
even when execution with bound parameters still infers an integer type from the
first value and truncates the decimal. Assert the generated bind parameter
types/values or execute the query against an engine that reproduces the
affected behavior.
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%2F42752&comment_hash=3bac678380ed5705170593932c6f18ff7c568cfd11ceb5ff69da837349b6629b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42752&comment_hash=3bac678380ed5705170593932c6f18ff7c568cfd11ceb5ff69da837349b6629b&reaction=dislike'>👎</a>
##########
superset/models/helpers.py:
##########
@@ -4096,6 +4096,17 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
else:
cond = is_null_cond
else:
+ # Normalize mixed int/float values before binding,
since
+ # SQLAlchemy may infer the bind parameter type from the
+ # first element and silently truncate other values
+ # (see #33206)
+ if target_generic_type ==
utils.GenericDataType.NUMERIC and any(
+ isinstance(v, float) for v in eq
+ ):
+ eq = [
+ float(v) if isinstance(v, (int, float)) else v
+ for v in eq
+ ]
Review Comment:
**Suggestion:** Converting every integer in a mixed list with `float(v)`
loses precision for integers larger than the exact range of IEEE-754 doubles.
For example, `9007199254740993` becomes `9007199254740992.0`, so an
integer-column filter can match the wrong row or fail to match the requested
ID. Preserve integer values and only force the bind type to a floating type
without changing their numeric representation. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ BigInteger `IN` filters can lose exact integer values.
- ⚠️ Chart query results can omit or misidentify matching rows.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9f93b0fa5e4e4574a6c4b79f59b8f6ee&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=9f93b0fa5e4e4574a6c4b79f59b8f6ee&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/models/helpers.py
**Line:** 4106:4109
**Comment:**
*Type Error: Converting every integer in a mixed list with `float(v)`
loses precision for integers larger than the exact range of IEEE-754 doubles.
For example, `9007199254740993` becomes `9007199254740992.0`, so an
integer-column filter can match the wrong row or fail to match the requested
ID. Preserve integer values and only force the bind type to a floating type
without changing their numeric representation.
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%2F42752&comment_hash=6a2bd0be6a77cf8c8c670454f674a0bb2a463a46fede4cdc869cfc88a4b6e1ac&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42752&comment_hash=6a2bd0be6a77cf8c8c670454f674a0bb2a463a46fede4cdc869cfc88a4b6e1ac&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]
