Skip to content

feat: query pagination#1010

Merged
nextchamp-saqib merged 15 commits intodevelopfrom
backend-pagination
Mar 29, 2026
Merged

feat: query pagination#1010
nextchamp-saqib merged 15 commits intodevelopfrom
backend-pagination

Conversation

@nextchamp-saqib
Copy link
Copy Markdown
Member

@nextchamp-saqib nextchamp-saqib commented Mar 28, 2026

Backend changes:

  • execute_ibis_query now uses page/page_size instead of a flat limit, computing LIMIT … OFFSET … for the requested page
  • The limit operation in the pipeline no longer applies SQL LIMIT (pagination handles it)
  • A bug fix for contains/not_contains filters on numeric columns (cast to string first)

Frontend – query execution:

  • query.ts tracks currentPage/pageSize state and sends them to the server
  • Execution-token pattern prevents stale out-of-order responses from overwriting newer results
  • addLimit removed; charts pass page_size directly to execute()

Frontend – data table:

  • New usePagination composable supporting both client-side and server-side paging
  • Smart filter mode: client-side when full result fits on one page, server-side (via adhocFilters) when paginated
  • New DataTableFooter component with lazy row-count loading
  • Prettier upgraded from 2.x → 3.x

- Keep TableChart mounted during execution (remove !loading guard) to
  prevent filter state loss and UI flash
- Add parseFilterString/matchesFilter helpers shared by client-side and
  server-side filter paths
- QueryDataTable falls back to client-side filtering when all rows fit
  on one page; pushes adhoc filters to backend otherwise
- Fix multi-column filter bug (rules were overwritten per iteration)
- Add execution token to discard stale paginated responses
- Show subtle tbody opacity dim while filtering instead of full progress bar
- Cast numeric columns to string before applying contains/not_contains in ibis
@nextchamp-saqib nextchamp-saqib marked this pull request as ready for review March 29, 2026 16:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end pagination for Insights query execution, shifting “limit” semantics from a pipeline operation to explicit page / page_size parameters, and updates the frontend data table to support both client-side and server-side paging/filtering.

Changes:

  • Backend: execute_ibis_query now applies LIMIT/OFFSET via page + page_size, and fixes contains/not_contains for numeric columns by casting to string.
  • Frontend: query execution tracks currentPage/pageSize, prevents stale responses via execution tokens, and introduces shared pagination + a new footer component.
  • Tooling/UI: removes query-builder limit usage for charts, adds inline filter parsing/matching helpers, upgrades Prettier and config.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
insights/insights/doctype/insights_query_v3/insights_query_v3.py Passes page/page_size into query execution and updates related query helpers.
insights/insights/doctype/insights_data_source_v3/ibis_utils.py Implements SQL paging in execute_ibis_query, disables pipeline limit op, fixes numeric contains filters.
frontend/src2/query/query.ts Tracks pagination state, sends it to backend, adds execution-token stale-response protection.
frontend/src2/query/helpers.ts Adds inline column filter parsing/matching utilities.
frontend/src2/query/components/QueryDataTable.vue Wires DataTable to server paging + server-side adhoc filters when needed.
frontend/src2/components/DataTable.vue Introduces shared usePagination, server filter hook, new footer, and filter input improvements.
frontend/src2/composables/usePagination.ts New composable supporting client and server pagination modes.
frontend/src2/components/DataTableFooter.vue New footer UI with paging controls and lazy count loading.
frontend/src2/components/LazyTextInput.vue Adds prefix/suffix slots and Prettier-3 formatting.
frontend/src2/charts/chart.ts Removes limit operation injection; passes chart limit as page_size to execute.
frontend/src2/charts/components/ChartRenderer.vue Adjusts Table chart rendering condition.
frontend/src2/dashboard/dashboard.ts Types filtersByQuery as AdhocFilters.
frontend/src2/dashboard/Filter.vue Template typing/formatting updates (Prettier 3).
frontend/src2/query/components/ColumnFilterValueSelector.vue Trailing comma formatting (Prettier 3).
frontend/package.json Bumps Prettier to 3.1.0.
frontend/yarn.lock Updates lockfile for Prettier change.
frontend/.prettierrc.json Updates config for Prettier 3 (tabWidth/trailingComma).
Comments suppressed due to low confidence (1)

insights/insights/doctype/insights_query_v3/insights_query_v3.py:188

  • The response currently returns sql: ibis.to_sql(ibis_query), but pagination is applied inside execute_ibis_query by creating a new limited query. This means the SQL shown in the UI/debug tools can differ from what actually executed (missing the LIMIT/OFFSET for the requested page). Consider returning the computed SQL from execute_ibis_query (or recomputing SQL after applying pagination) and using that in the response.
        results, time_taken = execute_ibis_query(
            ibis_query,
            page=page,
            page_size=page_size,
            force=force,
            cache_expiry=60 * 10,
            reference_doctype=self.doctype,
            reference_name=self.name,
        )
        results = results.to_dict(orient="records")

        columns = get_columns_from_schema(ibis_query.schema())
        return {
            "sql": ibis.to_sql(ibis_query),
            "columns": columns,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src2/query/components/QueryDataTable.vue Outdated
Comment thread frontend/src2/components/DataTable.vue
Comment thread insights/insights/doctype/insights_data_source_v3/ibis_utils.py Outdated
Comment thread insights/insights/doctype/insights_data_source_v3/ibis_utils.py Outdated
…value

Synthesizing totalRowCount from previewRowCount caused pagination to
incorrectly treat partial pages as the final page in server-paged mode.
usePagination already falls back to rowCount < pageSize when totalRowCount
is undefined, so returning undefined is sufficient and correct.
Returning self.query unconditionally caused older saved queries/charts
that contain a limit operation to silently produce unbounded result sets.
apply_limit now applies query.limit(n) as before; execute_ibis_query
then paginates on top of that via its own LIMIT/OFFSET, which is the
correct layering (dataset size capped by limit op, page sliced within).
Whitelisted endpoints receive values as strings; page was used raw,
causing TypeError on string multiplication and potential negative
offsets. Both params are now cast to int with a safe fallback and
clamped to valid ranges before computing LIMIT/OFFSET. The page_size
guard is also removed so 0/None is treated as the default (100) rather
than bypassing pagination entirely.
@nextchamp-saqib nextchamp-saqib merged commit d9b678b into develop Mar 29, 2026
4 checks passed
@mergify mergify bot deleted the backend-pagination branch March 29, 2026 16:44
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 3.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants