codeant-ai-for-open-source[bot] commented on code in PR #41024:
URL: https://github.com/apache/superset/pull/41024#discussion_r3456854734
##########
superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts:
##########
@@ -229,11 +229,26 @@ const buildQuery: BuildQuery<TableChartFormData> = (
}
if (!isDownloadQuery && formDataCopy.server_pagination) {
- const pageSize = ownState.pageSize ?? formDataCopy.server_page_length;
- const currentPage = ownState.currentPage ?? 0;
-
- moreProps.row_limit = pageSize;
- moreProps.row_offset = currentPage * pageSize;
+ const pageSize =
+ Number(ownState.pageSize ?? formDataCopy.server_page_length) || 0;
+ const rowLimit = Number(formDataCopy.row_limit) || 0;
+
+ // Never page past the configured row limit. Clamping the page to the
last
+ // one that still falls within the limit keeps the request inside the cap
+ // and avoids emitting row_limit: 0, which the backend treats as
+ // "no limit" rather than "no rows" (see helpers.py get_sqla_query).
+ const lastPage =
+ rowLimit > 0 && pageSize > 0
+ ? Math.max(Math.ceil(rowLimit / pageSize) - 1, 0)
+ : Number(ownState.currentPage) || 0;
+ const currentPage = Math.min(Number(ownState.currentPage) || 0,
lastPage);
+ const rowOffset = currentPage * pageSize;
+ const remainingRows =
+ rowLimit > 0 ? Math.max(rowLimit - rowOffset, 0) : pageSize;
+
+ moreProps.row_limit =
+ rowLimit > 0 ? Math.min(pageSize, remainingRows) : pageSize;
+ moreProps.row_offset = rowOffset;
Review Comment:
**Suggestion:** `row_limit` is now page-dependent, but when cached filters
change the code only resets `row_offset` to 0 and does not recompute
`row_limit`. If the user was on a capped last page (for example
`row_limit=120`, `pageSize=50`, page 3 -> `row_limit=20`), the reset query for
page 1 will still request only 20 rows instead of 50. Recompute `row_limit`
after forcing offset/page reset (or rebuild pagination props using page 0) so
first-page fetch size is correct after filter changes. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ First page after filter changes returns fewer rows.
- ⚠️ Table chart server pagination inconsistent with configured page size.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open or create a Table chart that uses the Table plugin (registered in
`superset-frontend/plugins/plugin-chart-table/src/index.ts:19-61`) and
enable server
pagination in the control panel (`server_pagination` and
`server_page_length` controls in
`src/controlPanel.tsx:383-407`).
2. Configure a `row_limit` that is not a multiple of the server page length,
for example
`row_limit = 120` and `server_page_length = 50`. Load data and navigate to
the last server
page via the table pagination UI, which updates `ownState.currentPage`
through
`handleServerPaginationChange` in `src/TableChart.tsx:24-32` by calling
`updateTableOwnState` with `currentPage` set to the selected page.
3. With the chart currently on that last page (so the previous request used
`row_offset =
100` and a capped `row_limit = 20` computed in `buildQuery` at
`src/buildQuery.ts:231-251`), change an external filter (e.g., a dashboard
filter or
cross-filter) so that `queryObject.filters` differs from the cached filters
stored via
`setCachedChanges` (`src/buildQuery.ts:51-54` and the closure in
`cachedBuildQuery` at
`src/buildQuery.ts:124-141`).
4. When the new request is built, `buildQuery` recomputes `moreProps` using
the stale
`ownState.currentPage` (still pointing at the last page) and sets
`moreProps.row_limit =
20` and `moreProps.row_offset = 100` (`src/buildQuery.ts:241-251`). The
subsequent
filter-change block (`src/buildQuery.ts:96-103`) only overrides `row_offset`
back to `0`
and does not recompute `row_limit`, so the first-page query after the filter
change is
sent with `row_limit = 20` instead of the expected full page size `50`,
yielding an
incorrectly short first page.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a655ef05d2394d07996ff2040469c414&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=a655ef05d2394d07996ff2040469c414&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/buildQuery.ts
**Line:** 246:251
**Comment:**
*Logic Error: `row_limit` is now page-dependent, but when cached
filters change the code only resets `row_offset` to 0 and does not recompute
`row_limit`. If the user was on a capped last page (for example
`row_limit=120`, `pageSize=50`, page 3 -> `row_limit=20`), the reset query for
page 1 will still request only 20 rows instead of 50. Recompute `row_limit`
after forcing offset/page reset (or rebuild pagination props using page 0) so
first-page fetch size is correct after filter changes.
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%2F41024&comment_hash=d755a7f95e02d3931c0d812ee6643cc3aa3d11cedb1563560175b8213444565a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41024&comment_hash=d755a7f95e02d3931c0d812ee6643cc3aa3d11cedb1563560175b8213444565a&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]
