codeant-ai-for-open-source[bot] commented on code in PR #42595:
URL: https://github.com/apache/superset/pull/42595#discussion_r3679108170
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -2405,6 +2405,59 @@ def
test_get_sqla_query_allows_jinja_templated_custom_sql_metric_with_columns(
assert "{{" not in sql
+def test_get_sqla_query_virtual_dataset_filter_values_drill_to_detail(
+ database: Database,
+) -> None:
+ """
+ Regression for #35263: a Jinja-templated virtual dataset that calls
+ ``filter_values()`` in its own SQL must see filters sent in the native
+ ``{col, op, val}`` format that Drill to Detail/Drill by use, not just
+ the ``adhoc_filters`` format used by ordinary chart/explore requests.
+ Without this, Jinja-based datasets return zero rows when drilled into,
+ even though the parent chart shows data for the selected value.
+ """
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ table = SqlaTable(
+ database=database,
+ schema=None,
+ table_name="t",
+ sql=(
+ "SELECT a, b FROM t WHERE 1=1 "
+ "{% if filter_values('b') %} "
+ "AND b IN {{ filter_values('b') | where_in }} "
+ "{% endif %}"
+ ),
+ columns=[
+ TableColumn(column_name="a", type="INTEGER"),
+ TableColumn(column_name="b", type="TEXT"),
+ ],
+ )
+
+ result = table.get_sqla_query(
+ columns=["a", "b"],
+ metrics=[],
+ extras={},
+ filter=[{"col": "b", "op": "IN", "val": ["Alice"]}],
+ granularity=None,
+ is_timeseries=False,
+ )
+ assert result is not None
+
+ with database.get_sqla_engine() as engine:
+ sql = str(
+ result.sqla_query.compile(
+ dialect=engine.dialect,
+ compile_kwargs={"literal_binds": True},
+ )
+ )
+
+ assert "'Alice'" in sql, (
+ "filter_values() should resolve native drill-to-detail-style "
+ f"filters inside a virtual dataset's own SQL. Generated SQL: {sql}"
+ )
+
Review Comment:
**Suggestion:** The assertion can pass even when `filter_values('b')`
receives no native query-context filters: `get_sqla_query` independently
applies the input filter as an outer `WHERE` predicate, so the compiled SQL
will still contain `'Alice'` while the virtual dataset's inner SQL remains
`WHERE 1=1`. Inspect or assert the rendered virtual-dataset subquery
specifically (or execute the query against data that distinguishes inner
filtering) to make this a valid regression test. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Regression test can pass when native Jinja filter fallback is broken.
- ⚠️ Issue #35263 may regress without CI detection.
- ⚠️ The test does not verify filtering inside virtual dataset SQL.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6d83d38a8a234f468d4c94666fb252d9&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=6d83d38a8a234f468d4c94666fb252d9&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:** 2455:2459
**Comment:**
*Possible Bug: The assertion can pass even when `filter_values('b')`
receives no native query-context filters: `get_sqla_query` independently
applies the input filter as an outer `WHERE` predicate, so the compiled SQL
will still contain `'Alice'` while the virtual dataset's inner SQL remains
`WHERE 1=1`. Inspect or assert the rendered virtual-dataset subquery
specifically (or execute the query against data that distinguishes inner
filtering) to make this a valid regression test.
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%2F42595&comment_hash=14946c64b27b6192abcdb097843ce13973649f015e8385e5572995b0acda65b8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42595&comment_hash=14946c64b27b6192abcdb097843ce13973649f015e8385e5572995b0acda65b8&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]
