codeant-ai-for-open-source[bot] commented on code in PR #42706:
URL: https://github.com/apache/superset/pull/42706#discussion_r3704053258
##########
superset/models/helpers.py:
##########
@@ -4018,6 +4018,18 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
template_processor=template_processor
)
is_metric_filter = True
+ elif (
+ col_obj is None
+ and isinstance(flt_col, str)
+ and flt_col in adhoc_columns_by_label
+ ):
+ sqla_col, _unused = self.adhoc_column_to_sqla(
+ col=adhoc_columns_by_label[flt_col],
+ template_processor=template_processor,
+ )
Review Comment:
**Suggestion:** The generic type returned by `adhoc_column_to_sqla` is
discarded, and this call does not set `force_type_check=True`, so expressions
that are not backed by metadata leave `adhoc_generic_type` unset. Numeric and
temporal filter values therefore fall back to `GenericDataType.STRING`, causing
values to be quoted or otherwise coerced incorrectly for comparisons and `IN`
filters. Preserve the returned generic type and force the type probe for this
label-based filter path. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Numeric adhoc-column filters can compare against incorrectly typed bind
values.
- ⚠️ `IN` filters may be quoted or rejected by the database.
- ⚠️ Table-chart server-side pagination can return incorrect filtered
results.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=202b5ab79b034a4ca8b5b1b31c59095a&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=202b5ab79b034a4ca8b5b1b31c59095a&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:** 4026:4029
**Comment:**
*Type Error: The generic type returned by `adhoc_column_to_sqla` is
discarded, and this call does not set `force_type_check=True`, so expressions
that are not backed by metadata leave `adhoc_generic_type` unset. Numeric and
temporal filter values therefore fall back to `GenericDataType.STRING`, causing
values to be quoted or otherwise coerced incorrectly for comparisons and `IN`
filters. Preserve the returned generic type and force the type probe for this
label-based filter path.
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%2F42706&comment_hash=4d15c9aa83fcf5afdbd7ba5bbd0da73557848e46cac5e80e75c06bfbf85c6687&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42706&comment_hash=4d15c9aa83fcf5afdbd7ba5bbd0da73557848e46cac5e80e75c06bfbf85c6687&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -461,13 +461,14 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
// only take relevant page size options
const pageSizeOptions = useMemo(() => {
- const getServerPagination = (n: number) => n <= rowCount;
+ const getServerPagination = (n: number) =>
+ n <= Math.max(rowCount, serverPageLength);
Review Comment:
**Suggestion:** The page-size filter does not account for the currently
selected `serverPaginationData.pageSize`. If a user selects a page size larger
than both `rowCount` and the configured `serverPageLength`—for example, selects
20 when there are 15 rows and the configured length is 10—option 20 is removed
from `pageSizeOptions`. `DataTable` then cannot find the active option and sets
the displayed page size to 0. Include the active server page size in the upper
bound so the current selection remains available. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Table pagination displays zero after result counts shrink.
- ⚠️ Server search and dashboard filtering expose the stale-size state.
- ⚠️ Users cannot see the actual active rows-per-page value.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3789b2a779604b30b60cf69a9163a3d7&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=3789b2a779604b30b60cf69a9163a3d7&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-frontend/plugins/plugin-chart-table/src/TableChart.tsx
**Line:** 464:465
**Comment:**
*Incorrect Condition Logic: The page-size filter does not account for
the currently selected `serverPaginationData.pageSize`. If a user selects a
page size larger than both `rowCount` and the configured `serverPageLength`—for
example, selects 20 when there are 15 rows and the configured length is
10—option 20 is removed from `pageSizeOptions`. `DataTable` then cannot find
the active option and sets the displayed page size to 0. Include the active
server page size in the upper bound so the current selection remains available.
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%2F42706&comment_hash=531076725cdd48a4a8a2075ec81e5c58fba6af222eebb901ab98a7f466f78f1c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42706&comment_hash=531076725cdd48a4a8a2075ec81e5c58fba6af222eebb901ab98a7f466f78f1c&reaction=dislike'>👎</a>
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -4762,3 +4762,56 @@ def test_adhoc_type_probe_does_not_get_sampling_retry(
table.adhoc_column_to_sqla(adhoc_col)
retry.assert_not_called()
+
+
+def test_filter_adhoc_column(database: Database) -> None:
+ """
+ Test that filter works with adhoc column labels.
+ When filter contains a string that matches the label of an adhoc column
+ in the columns list, it should correctly convert to a SQLAlchemy column
+ instead of raising QueryObjectValidationError.
+ """
+ from superset.connectors.sqla.models import SqlaTable, TableColumn
+
+ table = SqlaTable(
+ table_name="test_table",
+ database=database,
+ columns=[
+ TableColumn(column_name="name", type="TEXT"),
+ TableColumn(column_name="real_name", type="TEXT"),
+ ],
+ )
Review Comment:
**Suggestion:** The regression test covers only a string-valued `ILIKE`
filter whose expression is a direct physical column reference. It does not
exercise the adhoc generic type returned by `adhoc_column_to_sqla`, so numeric
or date expressions can still mis-coerce comparison and `IN` filter values
while this test continues to pass. Add cases using numeric/date expressions and
operators that verify the compiled predicates use the correct value types.
[code quality]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Numeric adhoc label filters may generate invalid predicates.
- ❌ Date adhoc label filters may return incorrect results.
- ⚠️ Existing test covers only text metadata resolution.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=81ac99030d1c42c4bbc4781678be76ef&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=81ac99030d1c42c4bbc4781678be76ef&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:** 4767:4783
**Comment:**
*Code Quality: The regression test covers only a string-valued `ILIKE`
filter whose expression is a direct physical column reference. It does not
exercise the adhoc generic type returned by `adhoc_column_to_sqla`, so numeric
or date expressions can still mis-coerce comparison and `IN` filter values
while this test continues to pass. Add cases using numeric/date expressions and
operators that verify the compiled predicates use the correct value types.
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%2F42706&comment_hash=ac51a4e4cb773fc64f740b6e461aafc609c9ac9b6dd11f34ac1d664060090c35&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42706&comment_hash=ac51a4e4cb773fc64f740b6e461aafc609c9ac9b6dd11f34ac1d664060090c35&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]
