Skip to content

fix: resolve double LIMIT bug in SQL pagination wrapping#154

Merged
debba merged 1 commit intoTabularisDB:mainfrom
midasism:fix/mcp-sql-limit-bug
May 4, 2026
Merged

fix: resolve double LIMIT bug in SQL pagination wrapping#154
debba merged 1 commit intoTabularisDB:mainfrom
midasism:fix/mcp-sql-limit-bug

Conversation

@midasism
Copy link
Copy Markdown
Contributor

@midasism midasism commented Apr 28, 2026

Summary

  • Fix a bug where extract_order_by/remove_order_by used naive rfind("ORDER BY") that captured trailing LIMIT/OFFSET clauses, producing double LIMIT syntax errors when the pagination layer appended its own LIMIT.
  • Replace per-driver helpers with a shared, quote-and-bracket-aware SQL tokenizer (extract_trailing_clauses + build_paginated_query) in drivers/common.rs that correctly separates ORDER BY, LIMIT, and OFFSET from the base query.
  • MCP run_query now accepts an optional limit parameter and respects user-specified LIMIT clauses (takes min(user_limit, page_size)).

Bug details

When a user (or AI agent via MCP) ran a query like:

SELECT * FROM tapp_appointment_message_event_limit ORDER BY id LIMIT 10

The old code produced invalid SQL:

SELECT * FROM (SELECT * FROM tapp_appointment_message_event_limit) as data_wrapper ORDER BY id LIMIT 10 LIMIT 101 OFFSET 0

The double LIMIT 10 LIMIT 101 is a syntax error on all database engines.

Changes

File Change
src-tauri/src/drivers/common.rs Add shared tokenize_sql, extract_trailing_clauses, build_paginated_query + 18 unit tests
src-tauri/src/drivers/postgres/mod.rs Remove old helpers, use shared functions
src-tauri/src/drivers/mysql/mod.rs Same
src-tauri/src/drivers/sqlite/mod.rs Same
src-tauri/src/mcp/mod.rs Add optional limit param to run_query, use configurable max_rows

Test plan

  • 29 unit tests pass (cargo test --lib drivers::common::tests) covering:
    • Basic queries without trailing clauses
    • ORDER BY only / LIMIT only / both / all three
    • Table names containing "limit" substring (e.g. tapp_appointment_message_event_limit)
    • Quoted identifiers and string literals with SQL keywords
    • Subqueries with inner ORDER BY
    • Multi-column ORDER BY
    • Trailing semicolons
    • build_paginated_query with user limit smaller/larger than page_size
    • Pagination page 2+ offset calculation
    • SQLite (no alias) vs PG/MySQL (with alias) variants
  • Full cargo test --lib passes (115/117 pass; 2 pre-existing failures in updater::tests unrelated to this change)
  • cargo build succeeds with no warnings

The `strip_limit_offset` and `extract_user_limit` helpers used naive
`rfind("LIMIT")` / `rfind("OFFSET")` which matched the keyword as a
substring of table names (e.g. `tapp_appointment_message_event_limit`),
string literals, or quoted identifiers — producing corrupted pagination
queries.

Replace the raw string search with a quote-and-bracket-aware SQL
tokenizer (`tokenize_sql`) that treats single-quoted strings,
double-quoted identifiers, backtick-quoted identifiers, and
parenthesized groups as opaque tokens. `strip_limit_offset` and
`extract_user_limit` now scan backward over tokens so only standalone
`LIMIT` / `OFFSET` keywords are matched.

Also update MCP `run_query` tool:
- Accept an optional `limit` parameter (default 100)
- Document that user LIMIT clauses in the SQL are respected

Add 11 new test cases covering table names containing "limit", quoted
identifiers, string literals with SQL keywords, and subqueries.

Made-with: Cursor
@midasism midasism force-pushed the fix/mcp-sql-limit-bug branch from b9d81bf to c4d2298 Compare April 28, 2026 08:19
@debba
Copy link
Copy Markdown
Collaborator

debba commented Apr 30, 2026

Thanks for PR, I will check it ASAP

@debba
Copy link
Copy Markdown
Collaborator

debba commented May 4, 2026

i will merge it, thanks

@debba debba merged commit f7a3d96 into TabularisDB:main May 4, 2026
1 check passed
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.

2 participants