Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Offset-form limits currently return before reading any rows.
1 open finding
What changed in this PR
Adds MySQL-style LIMIT support to interactive SELECT queries and early termination of INSERT parsing.
Changes:
- Parses count and offset/count LIMIT forms.
- Stops lexing once enough rows are produced.
- Documents supported query grammar and adds parser tests.
| File | Description |
|---|---|
scanner.go |
Adds cancellable lexing and LIMIT token. |
interpreter.go |
Lexes LIMIT clauses. |
query.go |
Models and parses query limits. |
cmd/sqlread/query.go |
Applies limits during execution. |
detail.go |
Supports stopping row parsing. |
query_test.go |
Tests LIMIT parsing and inclusion. |
detail_test.go |
Tests parser cancellation. |
lexitemtype_string.go |
Updates generated token strings. |
README.md |
Documents grammar and LIMIT usage. |
Files not reviewed (1)
- lexitemtype_string.go: Generated file
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently handles parsing, cross-block limits, cancellation, documentation, and relevant regressions.
0 open findings
1 resolved since last review
Files not reviewed (1)
- lexitemtype_string.go: Generated file
🧠 Review effort: Balanced
Comment on lines
+8
to
+10
| // ExecuteQuery reads a SELECT's rows from the dump and passes them to writeRow. | ||
| // It stops after the requested limit or the first error. The caller owns the | ||
| // output destination and must flush it. Outfile is handled by the caller. |
Comment on lines
+266
to
+267
| if d.Type == TIntpLimit { | ||
| return q.parseSelectLimitBuilder(qry) |
donatj
force-pushed
the
fix/query-limits
branch
from
October 11, 2026 17:13
aa64c9a to
8dd3041
Compare
donatj
force-pushed
the
fix/query-limits
branch
from
October 11, 2026 17:27
8dd3041 to
aa64c9a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Large dumps need small, streaming query results. A limited query should return to the prompt after the requested rows. It should not keep scanning the rest of the data block or retain a large queue of decoded data.
This adds
LIMIT countandLIMIT offset, countto user queries. Both values must be non-negative integers. Offsets apply across all INSERT blocks for a table. A zero count reads no row data. The README documents the supported grammar.The branch includes the merged architecture guide from #31. Query execution now follows its boundaries:
ExecuteQuerylives in the library. It streams rows through a callback. The command owns output files and flushing.Semicolon-terminated queries run without a second semicolon or end of input.
Validation:
go test ./... -timeout 30sgo test -race ./... -timeout 60sgo test ./cmd/sqlread -run '^Test(QueryStopsReading|InteractiveQueries)$' -count=10 -timeout 60sgo build ./...go vet ./...go test ./cmd/sqlread -run '^$' -bench '^BenchmarkExecQuery$' -benchmem -benchtime=300ms -count=2On a synthetic 20,001-row INSERT, executor allocations for
LIMIT 10fell from about 64 MB to 3.6 KB per call. Full-scan allocations fell from about 64.8 MB to 0.84 MB. Full scans were slightly faster in these runs. These are allocation measurements on an Apple M1 Max, not peak process memory. They exclude indexing and user-query parsing.The architecture guide still identifies full-scan buffers, index growth, cache loading, and large individual values as areas for further work. This change does not claim to resolve those broader limits.