Skip to content

Add query results pagination support - #942

Open
Dharshini-RS03 wants to merge 7 commits into
libredb:mainfrom
Dharshini-RS03:query-results-pagination
Open

Dharshini-RS03 wants to merge 7 commits into
libredb:mainfrom
Dharshini-RS03:query-results-pagination

Conversation

@Dharshini-RS03

Copy link
Copy Markdown
Contributor

Description

Adds query results pagination support for supported database providers. Generated table queries no longer impose a preview limit where the pagination infrastructure can handle it, allowing subsequent result pages to load correctly.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test addition or update

Related Issue

Closes #816

Changes Made

  • Added result pagination capability support for providers that support offset-based pagination.
  • Updated query preparation and API pagination handling so subsequent pages use the correct limit and offset.
  • Updated the results grid to show Load More only when result pagination is supported.
  • Updated generated table queries so the pagination system controls the result limit.
  • Added and updated unit, component, API, and integration tests for pagination behavior.

Testing

  • I have tested this locally
  • I have added/updated tests
  • All existing tests pass

Checklist

  • My code follows the project's code style guidelines
  • I have performed a self-review of my code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Additional Notes

The existing pagination infrastructure was reused rather than rebuilt. Pagination remains unavailable for providers that do not support the required offset behavior.

@cevheri

cevheri commented Sep 17, 2026

Copy link
Copy Markdown
Member

rebase upstream, and can you fix CI error

@cevheri cevheri added enhancement New feature or request loop:needs-info Maintainer-loop task blocked on human-reviewed clarification labels Sep 17, 2026
@Dharshini-RS03

Copy link
Copy Markdown
Contributor Author

Sure, I’ll rebase my branch onto the latest upstream/main, investigate the CI failures, and push the fixes. Thanks!

@Dharshini-RS03
Dharshini-RS03 force-pushed the query-results-pagination branch from 218407f to b9055b5 Compare September 17, 2026 13:04
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Dharshini-RS03

Copy link
Copy Markdown
Contributor Author

Updated the implementation based on the review feedback for #816.

  • Added pagination capability gating for supported providers.
  • Removed the default preview limit from generated starter queries where pagination is supported.
  • Fixed API hasMore handling.
  • Updated Load More logic to use the result pagination limit.
  • Added and updated tests for pagination, provider capabilities, and failure handling.
  • Rebased the branch with the latest upstream/main.
  • Fixed the CI test expectations and formatting issues.

All CI checks are now passing.

@cevheri cevheri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, the backbone of this is right and you found the seam the design asks for. Four things are correct and need no further work: hasMore now requires prepared.wasLimited in src/app/api/db/query/route.ts:139, both handleLoadMore copies reuse result.pagination.limit instead of the hardcoded 500, supportsResultPagination follows the supportsInlineRowEdit precedent in src/lib/db/types.ts and is gated on === true in the UI, and the preview cap has left the generated SQL text.

I ran the branch against a PostgreSQL table of 2000 rows to see the result on screen. Opening the table generates SELECT * FROM public.orders;, the grid shows 500 rows with the AUTO-LIMITED badge, Load More appends 500 at a time to 2000, and one further click returns an empty page and retires the control. So the mechanism works. What follows is where it does not yet match the design in #816, with the acceptance criterion for each item.

1. The preview page size has to travel as the limit execution option

The design removes the bound from the SQL text and sends 50 as an execution option. This PR does the first half only, so the first page is now DEFAULT_QUERY_LIMIT, 500 rows, on every engine. That is a tenfold change to what a click on a table costs, and it also makes the "Select Top 50" label at src/components/schema-explorer/TableItem.tsx:114 untrue.

The seam is handleTableClick in src/hooks/use-tab-manager.ts:330, whose executeQueryFn parameter is typed (query: string, tabId: string) => void and therefore cannot carry options today. It is called at src/workspace/StudioWorkspace.tsx:416 with queryExec.executeQuery, which already accepts an options argument.

Acceptance criteria:

  • Opening a table from the object tree returns 50 rows, and the response reports pagination.limit: 50.
  • Load More then fetches 50 more, not 500: page two is the size of page one.
  • A hook test covers the option reaching executeQuery, and the same behaviour holds in the standalone app and in StudioWorkspace, since both render this path.

2. The fallback statement still carries a bound

src/hooks/use-tab-manager.ts:341 still emits `SELECT * FROM ${path.join(".")} LIMIT 50;` when capabilities is absent. The design names this string explicitly. As it stands a tab opened before metadata arrives is still pinned to page one.

Acceptance criterion: that string carries no row bound, and the same execution option from item 1 applies to it.

3. Every provider has to declare the flag, and six do not

Ten providers were set to true and I agree with all ten. Six have no value at all, which the design rules out: the flag is optional in the type only because the interface is published through src/exports/, and every provider in this repo declares it. Each value below comes from that provider's own prepareQuery, which is what the design asks you to read.

Provider Value Why, from its own code
sql/cassandra/index.ts:224 false prepareQuery:355 throws on any SELECT with offset > 0
sql/clickhouse/index.ts:504 true Inherits the sql-base offset form. Its trailing FORMAT / SETTINGS path returns wasLimited: false, which your own hasMore conjunct now covers
sql/search/index.ts:680 see below prepareQuery:827 throws unless this.product.acceptsOffsetClause
document/mongodb.ts:623 false prepareQuery:694 pins offset: 0
keyvalue/redis.ts:158 false prepareQuery:865 pins offset: 0
embedded/libredb.ts:321 false No override, so base-provider.ts:366 never sets wasLimited, and page two can never be offered

The search provider is the one that cannot take a literal. One class serves two type-ids (#424), and the two products differ exactly here: acceptsOffsetClause is false for Elasticsearch at search/index.ts:291 and true for OpenSearch at search/index.ts:300. So the declaration has to be supportsResultPagination: this.product.acceptsOffsetClause. Writing true there would offer a control that makes Elasticsearch throw; writing false would hide a control OpenSearch can serve.

Acceptance criteria:

  • supportsResultPagination appears in every getCapabilities() under src/lib/db/providers/, each with a one-line comment giving the reason, in the style of the flags around it.
  • tests/integration/db/<type-id>-provider.test.ts asserts the value for its own type-id, including one assertion per product for elasticsearch and opensearch.

4. The regression this leaves behind

Items 1 and 3 together produce a loss on three engines. ClickHouse, Elasticsearch and OpenSearch fall into the default branch of generateTableQuery, so they lost their LIMIT 50, and with no flag declared they show no Load More. Their table preview goes from 50 rows to 500 and gains nothing in return. Cassandra avoided this only because a port 9042 branch was added to keep its LIMIT 50.

Acceptance criterion: for every type-id, opening a table either offers the preview page size with a working Load More, or keeps a bound in the generated statement. No engine is left with a larger preview and no control.

5. The unordered-pages notice is missing

The design accepts that an unordered query can return overlapping or skipped pages, and requires that the grid say so once rather than let it pass silently, next to the existing wasLimited badge at src/components/results-grid/StatsBar.tsx:92. That file is untouched in this PR.

Acceptance criteria:

  • When pagination is offered and the executed statement has no ORDER BY, the grid states the condition once, beside AUTO-LIMITED.
  • It is stated once, not per page, and it is a statement of fact and not a warning that blocks anything.
  • A component test covers both the ordered and the unordered case.

6. Docs and tests have to move with the code

CLAUDE.md requires code, docs and tests in step in the same PR, and this diff changes code only.

  • src/lib/db/types.ts:247 has no docblock, while every flag around it does.
  • The capability table in docs/ADDING_A_PROVIDER.md and the per provider capability tables in docs/providers/*.md do not mention the new flag.
  • The generated statement is documented with its old bound in docs/providers/elasticsearch.md:580-586, docs/providers/opensearch.md:606, docs/providers/couchbase.md:276, docs/providers/druid.md:886 and docs/API_DOCS.md:443, :483, :520. All of those are now wrong.
  • The ClickHouse test named for #264, "the trailing LIMIT is the last clause, so a user-appended FORMAT stays legal", was deleted. Please re-aim it at whatever still carries that invariant rather than removing the guard.

7. One item to confirm rather than change

The design asks that a failed page behave the same in both hooks and that neither advances currentOffset on failure. Reading the branch, use-query-adapter.ts raises a "Load More Error" toast and use-query-execution.ts reaches its own error toast through executeQuery, and neither advances the offset. Please confirm that on both paths and say so in the PR, so it is measured rather than assumed.

Out of scope, so please do not add it here

Automatic fetch on scroll stays out, as #816 says, and the manual control stays. The footer button's own placement and styling is a separate concern that predates this work (StatsBar.tsx, from #810), and I will open its own issue. Nothing about it belongs in this PR.

@cevheri

cevheri commented Sep 17, 2026

Copy link
Copy Markdown
Member

One optional extra on top of the review above. This is a UI change rather than a correctness one, so treat it as a nice to have: take it if you want, and it will not hold the PR up either way.

The Load More footer (LoadMoreFooter at src/components/results-grid/StatsBar.tsx:200, rendered at src/components/ResultsGrid.tsx:819) does not sit well in this UI. Measured on a 1920x1080 window it takes 57px of a 460px results panel, it appears and disappears with hasMore so the grid height jumps by that much between queries, and it is a centered control in a strip of its own while every other bar in this app is justify-between in text-xs font-mono. It also states a second time something the grid already says: StatsBar.tsx:78 renders (more available) right beside the row count.

So if you want to fold the two together: make that (more available) the control itself, reading something like 500 rows · load 500 more, and drop the footer and its render site.

Acceptance criteria:

  • No chrome below the grid, and the grid keeps the same height whether or not another page is available.
  • The control lives in the stats strip, in the same text-xs font-mono language as its neighbours, and appears under the same condition as today: supportsResultPagination === true && pagination.hasMore && onLoadMore.
  • It carries the same disabled and in-flight state that isLoadingMore gives the button today.
  • The load-more-footer tests in tests/components/ResultsGrid.test.tsx move to the new control rather than being deleted, including the case that asserts it is absent when result pagination is unsupported.

@Dharshini-RS03

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I went through all the requested changes for #816 and I understand what needs to be updated.

I’ll make the changes to pass the 50-row preview limit as an execution option, update the fallback query, add the pagination capability for all the required providers, and make sure ClickHouse, Elasticsearch, and OpenSearch still keep the 50-row preview with Load More.

I’ll also add the unordered-results message, update the related docs and tests, and verify the failed-page behavior in both query execution paths.

As requested, I’ll keep automatic infinite scroll out of this PR.

I’ll work on these changes and run the relevant tests before updating the PR.

@cevheri

cevheri commented Sep 18, 2026

Copy link
Copy Markdown
Member

Thanks for the detailed review. I went through all the requested changes for #816 and I understand what needs to be updated.

I’ll make the changes to pass the 50-row preview limit as an execution option, update the fallback query, add the pagination capability for all the required providers, and make sure ClickHouse, Elasticsearch, and OpenSearch still keep the 50-row preview with Load More.

I’ll also add the unordered-results message, update the related docs and tests, and verify the failed-page behavior in both query execution paths.

As requested, I’ll keep automatic infinite scroll out of this PR.

I’ll work on these changes and run the relevant tests before updating the PR.

thanks for explanation

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

Labels

enhancement New feature or request loop:needs-info Maintainer-loop task blocked on human-reviewed clarification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Add pagination and infinite scroll to query results

2 participants