Skip to content

fix(drivers): show pagination for SELECTs with leading SQL comments#213

Open
ymadd wants to merge 1 commit into
TabularisDB:mainfrom
ymadd:fix/pagination-leading-comments
Open

fix(drivers): show pagination for SELECTs with leading SQL comments#213
ymadd wants to merge 1 commit into
TabularisDB:mainfrom
ymadd:fix/pagination-leading-comments

Conversation

@ymadd
Copy link
Copy Markdown
Contributor

@ymadd ymadd commented May 18, 2026

Summary

  • is_select_query now strips leading -- / /* */ comments before checking for SELECT, matching returns_result_set and is_explainable_query. Without this, a SELECT preceded by a header-comment block was routed through the result-set fetch path but bypassed the pagination branch in exec_on_*_conn, returning pagination: None and hiding the UI controls entirely — so a 500-row result looked like a fixed slice with no way to advance.
  • Fixed a latent bug in strip_limit_offset that the classifier change exposed: it tokenised then rejoined with " ", collapsing newlines. For a query like -- header\nSELECT ... LIMIT 50 the rejoined output became -- header SELECT ... LIMIT 50 OFFSET 0, which MySQL/Postgres/SQLite all parse as a single -- comment line — silently no-op. Switched the tokenizer to track byte offsets and made strip_limit_offset slice the original input, preserving comments and whitespace verbatim.
  • Factored the three identical quoted-token scanner branches (', ", `) into a read_quoted helper while refactoring the tokenizer.

Repro (before this PR)

-- ============================================
-- Any header comment block
-- ============================================
SELECT * FROM big_table ORDER BY id;  -- 500+ rows

Result: rows show, but the pagination bar is gone — only the first page is reachable.

After: pagination renders and pages can be navigated as expected.

Test plan

  • cargo test --lib drivers::common — 42 passed, including 4 new regression tests:
    • test_is_select_query_with_leading_comments
    • test_build_paginated_query_with_leading_comments
    • test_build_paginated_query_multiline_comments_with_user_limit
    • test_strip_limit_offset_preserves_leading_comments
  • Manual: paste a comment-headered SELECT returning >page_size rows against MySQL, Postgres, SQLite; verify pagination bar renders and Next/Prev work.

🤖 Generated with Claude Code

`is_select_query` did not strip leading `--` / `/* */` comments before
checking for `SELECT`, while `returns_result_set` and
`is_explainable_query` did. A SELECT preceded by a comment header was
therefore routed to the fetch path but bypassed the pagination branch
in `exec_on_*_conn`, returning `pagination: None` so the UI hid the
controls entirely. Align it with the other helpers.

Fixing the classifier alone exposed a latent bug in `strip_limit_offset`:
it tokenised then rejoined with `" "`, collapsing newlines. For a query
like `-- header\nSELECT ... LIMIT 50` the rejoined output became
`-- header SELECT ... LIMIT 50 OFFSET 0`, which engines parse as one
`--` comment line — silently no-op. Switched the tokenizer to track
byte offsets and made `strip_limit_offset` slice the original input,
preserving comments and whitespace verbatim.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant