codeant-ai-for-open-source[bot] commented on code in PR #41346:
URL: https://github.com/apache/superset/pull/41346#discussion_r3532971494
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts:
##########
@@ -378,11 +402,19 @@ const buildQuery: BuildQuery<TableChartFormData> = (
JSON.stringify(options?.extras?.cachedChanges?.[formData.slice_id]) !==
JSON.stringify(queryObject.filters)
) {
- queryObject = { ...queryObject, row_offset: 0 };
+ // Reset to the first page: restore the full first-page row_limit rather
+ // than carrying over the last page's capped value.
+ queryObject = {
+ ...queryObject,
+ row_offset: 0,
+ row_limit: firstPageRowLimit,
Review Comment:
**Suggestion:** The filter-reset branch now overwrites
`queryObject.row_limit` with `firstPageRowLimit` for all server-pagination
requests, including download requests. When a CSV/JSON download is triggered
after filters change, this shrinks the export to the first page size instead of
using the configured export row limit. Only apply this row-limit reset to
interactive paged queries (non-download), so download queries keep their
intended full limit. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Server-paginated table CSV exports can be truncated.
- ❌ JSON result exports may return only first-page rows.
- ⚠️ Users may misinterpret incomplete exported datasets.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. AgGridTableChartPlugin registers `buildQuery` as its query builder in
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/index.ts:31-32,84-85`,
and the
default export from `buildQuery.ts` is the cached wrapper
`cachedBuildQuery()` defined at
`buildQuery.ts:299-320`, which always passes `extras: { cachedChanges }`
into the inner
`buildQuery`.
2. When a server-paginated chart is first run (formData.server_pagination =
true), the
inner `buildQuery` in `buildQuery.ts:61-295` executes; at the end it calls
`options?.hooks?.setCachedChanges({ [formData.slice_id]: queryObject.filters
})` at
`buildQuery.ts:423-426`, seeding `cachedChanges[<slice_id>]` with the
current filters for
subsequent requests.
3. After the user changes filters (e.g., via Explore or dashboard filters),
the next query
for this slice uses the same cached wrapper, so
`options.extras.cachedChanges[formData.slice_id]` still holds the old
filters; in a
CSV/JSON export call, core chart code invokes the same plugin `buildQuery`
but with
`formData.result_format = 'csv'` or `'json'` and `formData.result_type =
'results'` while
keeping `formData.server_pagination = true`. Inside `buildQuery.ts`, this
makes
`isDownloadQuery` true at lines 225-228, so the download branch sets
`moreProps.row_limit
= Number(formDataCopy.row_limit) || 0` and `moreProps.row_offset = 0` at
`buildQuery.ts:230-233` to cover the full export limit.
4. In the same download invocation, because `formData.server_pagination` is
true and
`options.extras.cachedChanges[formData.slice_id]` (old filters) differs from
the new
`queryObject.filters`, the filter-reset guard at `buildQuery.ts:399-404` is
satisfied; the
block then rewrites `queryObject` at lines 407-410 to `{ ...queryObject,
row_offset: 0,
row_limit: firstPageRowLimit }` using `firstPageRowLimit` from
`buildQuery.ts:219-222`,
which is capped to at most the page size. As a result, the export query that
previously
targeted the full configured `row_limit` is silently truncated to only
`pageSize` rows, so
CSV/JSON downloads after a filter change can return only a single page of
data instead of
the intended full export size whenever the export is the first query after
the filter
change.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dc9dd50d6998467b8622ecd7b4ba4b5c&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=dc9dd50d6998467b8622ecd7b4ba4b5c&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-ag-grid-table/src/buildQuery.ts
**Line:** 407:410
**Comment:**
*Api Mismatch: The filter-reset branch now overwrites
`queryObject.row_limit` with `firstPageRowLimit` for all server-pagination
requests, including download requests. When a CSV/JSON download is triggered
after filters change, this shrinks the export to the first page size instead of
using the configured export row limit. Only apply this row-limit reset to
interactive paged queries (non-download), so download queries keep their
intended full limit.
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%2F41346&comment_hash=660dfd320c53fc20fe3b01b1bdcc6965575bc65286252fa408427702fba4f0c6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41346&comment_hash=660dfd320c53fc20fe3b01b1bdcc6965575bc65286252fa408427702fba4f0c6&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]
