CLI, benchmark harness and a worked example - #5
Merged
Merged
Conversation
The shell's command handling is separate from the terminal loop, so every command is tested by feeding it a line and reading what comes back — no pty, no stdin, no mocking. The loop itself is thin enough to be excluded from coverage rather than tested through a fake terminal. The benchmark measures the optimiser instead of asserting it: every query runs twice against the same data in the same process, once on the bound plan and once on the optimised one, and the run fails if the two disagree on the number of rows. On 500k rows a selective range comes back 155x faster because the zone maps let it read 3 row groups out of 62. Fixed a real bug found by running the example script: a LEFT JOIN with a non-equality in its ON clause applied that condition after deciding what matched, so a preserved row whose only candidate failed the condition vanished instead of coming back NULL-padded — and so did rows that never had a candidate. Matching is now settled before padding. Regression tests cover the left and full cases at both the operator and the SQL level. Also fixed: a comment line before a statement was swallowing it, because the script reader buffered the comment and then treated the next line as a continuation.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
DataType's Display wrote its name with write_str, which silently ignores a
format width, so the {:<10} in the column listing did nothing. Padding through
Formatter::pad makes the width work, and the nullability column now pads too.
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.
The part you can actually run.
Shell
Command handling lives in
shell.rs, separate from the terminal loop, so every command is tested by handing it a line and reading the string it returns — no pty, no stdin, no mocking.repl.rsis the leftover: read a line, print what comes back, and it is excluded from coverage rather than tested through a fake terminal.\dt,\d <table>(columns plus the zone-map statistics),\import(CSV with inferred types),\save(write to.qfcand start reading from it),\attach,\explain,\timing.Benchmark
It measures the optimiser rather than asserting it. Every query runs twice against the same data in the same process — once on the bound plan, once on the optimised one — after a warm-up pass so the comparison is not timing the file cache. The run fails outright if the two plans disagree on the row count, because a flattering speedup from a wrong answer is worse than no number at all.
On 500k rows in 8192-row row groups:
The three-figure numbers are all zone-map pruning; the 4.5x on a full aggregate is projection pushdown alone, reading one column chunk per row group instead of four.
queryforge benchregenerates the table.A bug the example script found
examples/tour.sqlwalks through the feature set on ten rows of orders. Running it surfaced a genuine defect:returned one row instead of five. The join was applying the ON clause's non-equality part after deciding which probe rows had matched, so a preserved row whose only candidate failed the condition disappeared rather than coming back NULL-padded — and so did every customer with no orders at all, since
NULL = 'cancelled'is unknown.The fix settles matching before padding: candidates are gathered, the residual condition filters them, and only then is it decided which probe rows went unmatched. Regression tests cover the left and full cases at the operator level and through SQL. The existing 43 end-to-end tests did not catch this — every join test either had no residual condition or was an inner join — which is the argument for having an example you actually run.
Also fixed: a comment line above a statement swallowed it, because the script reader buffered the comment and then read the next line as a continuation.
Verification. 39 CLI tests. Workspace line coverage is 96.8%, with the optimiser's rewriting machinery brought up to 92% by tests that call it directly.