Conversation
The suite was 302 green tests that could not detect any of the correctness defects present in the engine, because nearly every assertion is a predicate::str::contains substring match. A test asserting "Engineering,3" passes even when three extra wrong rows are emitted, and passes regardless of row order. Adds three things: - Exact-output helpers in tests/helpers (assert_query, assert_query_fails, run_sql). These compare the COMPLETE stdout: every row, in order. - tests/golden/: 79 characterization tests pinning behaviour that is correct today. This is the semantic oracle for upcoming refactors, most immediately the direct sqlparser 0.36 -> 0.62 jump, which has no bisection handle. Includes new customers/purchases fixtures with unmatched rows on both sides, because the existing users/orders fixtures cannot exercise outer-join NULL-fill at all: every user has an order and every order has a user, so LEFT/RIGHT/FULL all degenerate to INNER there. - tests/defects/: 27 #[ignore]d tests encoding CORRECT behaviour for each confirmed defect (WHERE ignored on GROUP BY, multi-column GROUP BY collapsing to one key, ORDER BY/LIMIT dropped on GROUP BY and JOIN, arithmetic projections returning NULL under LIMIT, COUNT(DISTINCT) ignoring DISTINCT, HAVING binding to the wrong aggregate, NULL comparisons erroring instead of yielding UNKNOWN, and the expression gaps in projections and WHERE). All 27 fail today. The red list shrinks as each is fixed. Also adds tests/data/nullable.csv: no existing fixture had an empty field, so nothing in the suite exercised a NULL flowing into a comparison. 339 passing, 27 ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two pieces of pre-work for the sqlparser 0.36 -> 0.62 upgrade. src/join.rs was never compiled: it is not declared as a module in either lib.rs or main.rs, left behind by the VM execution engine migration. It contained a JoinExecutor whose "inner join" ignored its ON condition outright (// TODO: Evaluate ON condition here) and a From<&JoinOperator> impl whose catch-all mapped unknown join types to INNER. Deleting it is a no-op for the build, confirmed by a clean compile. The live JoinOperator matches in compiler_join.rs both had catch-all arms. That matters because newer sqlparser parses a plain `LEFT JOIN` as JoinOperator::Left rather than LeftOuter, reserving LeftOuter for the explicit `LEFT OUTER JOIN` spelling. Behind a `_` arm that rename is invisible to the compiler and shows up only as failing tests -- and the suite uses the about-to-change spellings far more than the stable ones (10 LEFT JOIN, 10 RIGHT JOIN, 8 bare JOIN, versus 3 each of the OUTER forms). Both sites now route through one `classify_join` helper with an exhaustive match and no catch-all, so a renamed or added variant is a compile error that names the site. Also folds the two copies of the join-kind destructuring into a single place, which the compiler unification will need anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A direct jump across 26 nominally-breaking releases: 147 compile errors,
resolved to 0 with the full suite green and the defect red list unchanged at
27 still-failing.
The AST is well firewalled -- engine.rs and table.rs never mention sqlparser,
so nothing below the bytecode boundary moved. The churn was concentrated in
compiler.rs (113 of the 147 errors).
Adds src/vm/ast_compat.rs, a thin layer of accessors that adapt AST *shape*
without adapting meaning. Rather than spread the new structure over ~150 call
sites -- most of which the planned compiler unification will delete anyway --
the compiler reads through func_args, group_by_exprs, query_order_by,
query_limit, query_offset, delete_from, insert_target, assignment_column,
create_table_options and friends. Anything requiring a semantic decision stays
in the compiler proper.
Shape changes handled:
- Expr::Value now wraps ValueWithSpan. Fixed by destructuring at the binding
site (`Expr::Value(ValueWithSpan { value, .. })`) so inner code is untouched.
- Function::args became FunctionArguments; DISTINCT moved into the argument
list as a DuplicateTreatment.
- Query::order_by is Option<OrderBy>; limit/offset merged into limit_clause.
- Insert/Delete/Update/CreateTable/AlterTable/Truncate became struct variants.
AlterTable now carries multiple operations and Truncate multiple targets;
both are rejected explicitly beyond the single-item case sqawk supports.
- Expr::Case merged conditions and results into CaseWhen, which makes the old
"conditions and results must have same length" check unrepresentable.
- ObjectName holds ObjectNamePart rather than Ident.
- SelectItem gained ExprWithAliases, SetOperator gained Minus (a synonym for
EXCEPT, so it shares that arm), SqlOption and AssignmentTarget became enums.
Two behaviour points worth calling out:
SUBSTR(x, 1, 3) now parses as Expr::Substring rather than as a function call,
so it stopped working in WHERE entirely and lost its column name in SELECT.
This is the two-expression-compiler problem in miniature: the projection path
had a Substring arm and the WHERE path did not. Extracted one compile_substring
used by both. Caught by three tests.
GROUP BY ALL and ORDER BY ALL carry no key list, so a naive accessor would
report them as "no GROUP BY"/"no ORDER BY" and silently drop the clause.
Both are now rejected explicitly, with tests.
Also adds six error-path tests. The suite previously had only two tests that
asserted a failure at all, so nothing pinned which inputs must be REJECTED --
exactly the blind spot an AST upgrade can slip through.
345 passing, 27 ignored, clippy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
VmEngine::init inferred the register-file size as max(p1, p2, p3) over the instruction stream, capped at 10000. That is wrong in both directions: - p1/p2/p3 hold literals as often as register numbers. `SELECT ... WHERE salary > 999999999` compiles to an Integer instruction whose p1 is the literal, so the file was sized to the 10000 cap for a program using a handful of registers. - Conversely a program legitimately needing more than 10000 registers got a short file, and Column errors on a short file instead of growing it, even though set_register grows lazily. Program now carries register_count, set from the compiler's high-water mark, and the engine uses it. Hand-assembled programs in the VM unit tests carry no count, so the old scan remains as a fallback when register_count is 0. This is a prerequisite for the compiler unification: a single recursive expression compiler allocates one register per expression node with no reuse, so it allocates strictly more than the current per-path code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a Label API (label / resolve / emit_jump_to / finish_labels) so jump
destinations are symbolic rather than computed as offsets from program.len()
counting instructions not yet emitted. Unresolved labels fail the compile
rather than leaving a placeholder target. This is a prerequisite for the
staged SELECT pipeline, where any reordering silently invalidates hand-counted
offsets -- and the failure mode is a wrong answer, not a crash.
Converting the fragile sites immediately turned up two live bugs.
1. LIMIT counted rows scanned, not rows returned, whenever WHERE was present.
compile_select_with_limit targeted the WHERE-false jump at
`program.len() + 2`, assuming ResultRow and Next were adjacent. With a
LIMIT they are not -- a DecrJumpZero sits between them -- so a row failing
the filter jumped onto the limit counter and decremented it:
SELECT name FROM employees WHERE department='Engineering' LIMIT 5
-> Alice, Charlie (Frank missing)
SELECT name FROM employees WHERE department='Sales' LIMIT 5
-> David (Grace missing)
With OFFSET instead, the same jump landed on ResultRow, emitting rows that
had failed the filter. The jump now targets a label bound to Next.
2. AND could return true when both operands were false.
The inline AND in compile_binary_comparison initialized its result register
AFTER the short-circuit test on the left operand, so a left-false row
jumped over the initialization. Registers persist across loop iterations,
so the register still held the previous row's verdict:
SELECT name FROM employees WHERE salary > 60000 AND department='Engineering'
-> Alice, Bob, Charlie, Frank (Bob is Marketing/55000: fails both)
SELECT name FROM employees WHERE department='Engineering' AND salary > 60000
-> every row in the table
Only visible when a row passing both conditions precedes a row failing the
left one, which is why two-numeric-comparison cases looked fine. OR already
initialized before its short-circuit and was unaffected. AND now does the
same, and uses labels.
Both bugs were found while auditing the generated golden tests, one of which
had captured the broken AND as expected output. Corrected, and the audit also
moved multi-statement output to the red list: `SELECT a; SELECT b` prints
every statement's rows under the LAST statement's header.
New regression tests cover WHERE x LIMIT/OFFSET, which the suite had never
exercised together.
349 passing, 28 ignored, clippy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three additions to the 76-opcode set, all executable and unit-tested, none emitted by the compiler yet. They are the primitives the staged SELECT pipeline needs. Not (P2 = !P1, NULL-preserving) makes a bare `NOT <expr>` expressible -- there is currently no arm for it anywhere -- and replaces the separate hand-rolled inversions behind NOT LIKE, NOT IN and IS NOT NULL. It propagates NULL deliberately: NOT UNKNOWN is UNKNOWN, not true. Its notion of truthiness is shared with IfZ so the two cannot disagree. Compare (pairwise-compare two register vectors) and Jump (three-way branch on the result) are SQLite's OP_Compare/OP_Jump. Together they make multi-column key comparison two instructions regardless of key width. The GROUP BY group-change check is currently a single Ne against the first key column, which is why multi-column GROUP BY collapses to one key; a chain of Ne plus an OR-chain would be ~4 instructions per row per column instead. Compare deliberately uses INTERNAL ordering, where NULL compares equal to NULL, rather than SQL equality. Grouping and sorting need NULL keys to group together, while SQL `=` must yield UNKNOWN for NULL. Keeping them in separate opcodes is what will let three-valued logic land on Eq later without breaking GROUP BY over a nullable key -- the ordering constraint called out in the plan. Five unit tests cover truthy/falsy negation, NULL propagation, three-way branching, detection of a difference in a non-first key column, and NULL grouping with NULL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Introduces NameCtx and code_expr, the analogue of SQLite's sqlite3ExprCode(): one recursive expression compiler used from every position an expression can appear in, so a node supported in one position is supported in all of them. sqawk had two. compile_expr had no Identifier arm at all; compile_projection_expr and resolve_column_expr understood only bare columns and a fixed list of single-argument functions, reporting anything else as "Only column references supported in SELECT". That split is why CASE and CAST worked in WHERE but not in a SELECT list, and why the sqlparser upgrade broke SUBSTR in WHERE while leaving SUBSTR in a projection working. The root cause of the drift was structural: both compilers threaded a bare (table, cursor_idx) pair, which cannot express a join, an alias, or a post-aggregation scope. NameCtx replaces it with the set of visible FROM sources, each carrying its cursor, table and refname. Landed as a shim so no call site changes: compile_where_operand now delegates to code_expr, and the not-yet-migrated arms fall back to the original operand compiler (renamed compile_operand_legacy). Arms migrate out of it as the pipeline work proceeds; routing through the front door first means every caller picks up new capability immediately. compile_projection_expr is now a delegation rather than a parallel implementation. Its `_` arm tried resolve_column_expr and, on failure, emitted NO INSTRUCTION AT ALL, leaving the destination register at its default NULL -- which is why `SELECT name, salary*2 FROM employees LIMIT 3` returned NULLs while the same query without LIMIT returned correct values. code_expr holds the opposite invariant: on Ok(reg), code has been emitted that defines reg on every path, and there is no arm that returns Ok without emitting. Six red-list defects now pass and are no longer ignored: - arithmetic projection returning NULL under LIMIT - alias-qualified columns in a single-table WHERE (FROM t e WHERE e.age > 30), which had no CompoundIdentifier arm at all - case-insensitive column resolution, where SELECT NAME worked but WHERE NAME = 'x' did not, because one path lowercased and find_column_index compared exactly - the || string concatenation operator, which sqlparser always produced and the compiler simply had no arm for - comparisons as projected values - NOT in WHERE, previously unsupported in any position; the NOT-forms that did work each hand-rolled their own inversion Ambiguous unqualified columns across multiple FROM sources are now an error rather than a silent pick of the first match, which would make the result depend on FROM order. 451 passing, 22 ignored (was 28), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`UPDATE t SET x = x + 1` was rejected with "Unsupported expression type: Identifier". The assignment RHS was compiled by compile_expr, the context-free expression compiler that has no Identifier arm at all, so no column could appear there. Assignments now compile through code_expr in the row's scope. Reading a column emits a Column op against the cursor, which yields the ORIGINAL row value -- the required semantics, since every RHS in a statement is evaluated against the row as it was before any of that statement's assignments. Verified through writeback rather than a trailing SELECT: `UPDATE ...; SELECT ...` in a single invocation emits the header but none of the SELECT's rows, while `SELECT; SELECT` works. That is a separate pre-existing defect, now on the red list as update_then_select_emits_the_select. Adds run_with_write and assert_after_write helpers, since observing what a DML statement did otherwise requires --write. Also removes extract_substr_args, which lost its only caller when compile_projection_expr became a delegation. 453 passing, 22 ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
COUNT(DISTINCT department) returned 8 on an 8-row table with 4 departments: the compiler never read the DISTINCT marker off the function call, so the aggregate saw every row. DISTINCT is carried as a flag bit (AGG_DISTINCT) OR-ed into the AggStep/AggFinal function-type word rather than as five additional constants, so it composes with every aggregate instead of being special-cased for COUNT. The accumulator gains a seen-set that is only allocated when the flag is present, so the ordinary path allocates nothing. NULLs are not tracked, since every aggregate already skips them. Verified across the set on a fixture of 10,10,20,30,30: SUM=100 SUM(DISTINCT)=60 COUNT=5 COUNT(DISTINCT)=3 MIN(DISTINCT)=10 MAX(DISTINCT)=30 AVG(DISTINCT)=20 455 passing, 21 ignored (was 22), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes the third parallel expression compiler, the one embedded in compile_table_scan's projection loop, and replaces the two allowlists that decided whether a projection "needs expression compilation". Those allowlists named nine expression kinds (Function, Trim, Substring, Position, Overlay, BinaryOp, UnaryOp, Ceil, Floor) and omitted CASE, CAST, IS NULL, LIKE, BETWEEN, IN and subqueries. Anything omitted fell through to the column-index path and was rejected with "Only column references supported in SELECT" -- despite the expression compiler handling it perfectly well. projection_needs_expr now asks the closed question instead: is this item something other than a plain column reference? An allowlist has to be updated whenever an expression kind is added; that inversion cannot drift. SQL draws no distinction between an expression used as a filter and the same expression used as a projected value. sqawk did, differently in each of three compilers, in both directions: - CASE, CAST, ||, IS NULL, LIKE, BETWEEN and IN worked in WHERE but not in a SELECT list. - TRIM, CEIL and FLOOR worked in a SELECT list but not in WHERE -- the mirror image, and the same shape as the SUBSTR breakage the sqlparser upgrade exposed. Both directions now go through code_expr. Trim/Ceil/Floor gain real arms; predicate-shaped nodes used as values route to the condition compiler and copy their result into the destination register. Eight golden tests pin both positions for the same node kinds so they cannot diverge again. 463 passing, 21 ignored, clippy clean. compiler.rs is down ~110 lines despite the added capability. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two silent wrong-answer defects on the grouped path. WHERE was ignored on every grouped query. compile_select_with_group_by never read select.selection, so `SELECT department, COUNT(*) FROM employees WHERE salary > 60000 GROUP BY department` returned all four departments with unfiltered counts instead of the two that qualify. Rows are now filtered before reaching the sorter, so an excluded row never reaches an accumulator and a group whose every row is filtered out disappears entirely rather than showing a stale count. Group-change detection compared only the first key column -- a single Ne against column 0 -- so `GROUP BY department, role` grouped on department alone, returning 4 rows instead of 6 and reporting whichever role happened to sort first. It now uses Compare/Jump over the whole key vector: two instructions regardless of key width, and NULL compares equal to NULL so NULL keys group together instead of starting a new group on every row. Adds emit_jump_to_three for the three-way Jump, so its three destinations are symbolic like every other branch. 373 passing, 18 ignored (was 20), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in the HAVING operand compiler, all of the same shape: it identified things by too little information and silently used the wrong one. An aggregate was matched on function type alone, ignoring its argument. With two aggregates of the same kind -- `SELECT department, SUM(salary), SUM(age) ... HAVING SUM(age) > 60` -- HAVING bound to SUM(salary), whichever came first in the projection, and filtered on entirely the wrong column. MIN against MAX bound correctly, which is why this stayed hidden. Matching is now on both function type and resolved argument column, and an aggregate in HAVING that is not in the SELECT list is reported as such. A GROUP BY key referenced in HAVING always copied the FIRST key register whatever name was written, so with a multi-column GROUP BY any reference to a later key silently filtered on the first. The name is now resolved to its position in the key vector, and a column that is not a group key is an error rather than a wrong answer. A literal operand handled Value::Number only and emitted no instruction for anything else, so `HAVING department = 'Sales'` compared the group key against an uninitialized register and matched nothing. Literals now go through the shared expression compiler, which cannot return without emitting. 472 passing, 17 ignored (was 18), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GROUP BY, plain aggregates and explicit joins all emit rows directly rather than through the cursor-based sorter, and all three silently discarded ORDER BY and LIMIT. For the grouped paths `query` was in scope at the call site and simply never passed on, under a comment claiming "GROUP BY with ORDER BY/LIMIT handles these internally". For joins the code said it outright: "ORDER BY support for JOINs requires post-processing which isn't fully implemented. For now, compile the JOIN and skip ORDER BY". So `GROUP BY department ORDER BY department DESC` came back ascending, and `GROUP BY department LIMIT 1` returned every group. Adds a SortResults opcode in the same family as Distinct and Limit: it runs over the finished result set, so it does not care how the rows were produced. That is what makes one implementation serve grouped output and joins alike, neither of which can feed the cursor-based sorter. ORDER BY keys are matched against the SELECT list, by alias or by rendered expression, so `ORDER BY COUNT(*)` and `ORDER BY department` both resolve; a key that is not projected is an error rather than a dropped clause, since after aggregation no other column still exists. Also fixes the index-tracking bookkeeping: post-processing Limit trimmed self.results without trimming result_row_indices, so the debug validator reported a mismatch on every limited query. Both SortResults and Limit now keep the two in step. 482 passing, 13 ignored (was 17), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The worst defect for a tool whose input is CSV: comparing against an empty field raised a runtime error and aborted the query. SELECT name FROM nullable WHERE score > 60 -> VM execution error: Cannot compare incompatible types: Null and Integer(60) Empty fields are ubiquitous in real CSV, so sqawk failed on exactly the data it exists to process. Comparisons now follow SQL three-valued logic: any NULL operand yields UNKNOWN, represented as a NULL result register. IfZ already treated a NULL register as zero, so every WHERE, HAVING and ON site rejects an UNKNOWN row with no control-flow change at all -- the reason this landed without a single test moving. NULL = NULL was true and is now UNKNOWN. Grouping and sorting need the opposite, NULL equal to NULL so NULL keys group together, which is exactly why the Compare opcode added earlier carries its own internal ordering instead of sharing this path. That separation is what made this change safe; landing it before Compare/Jump would have merged every NULL-keyed group into its predecessor. Mixed string/number comparison also errored, so `WHERE salary > '60000'` was unusable. Operands are now coerced when the string parses as a number and compared as text otherwise. This is the awk-shaped choice rather than the SQLite one, for two reasons: a CSV column is untyped text whose type sqawk infers per CELL, so one column can hold both; and arithmetic already coerced this way, so `x + '1'` and `x > '1'` now agree instead of disagreeing. Integer-to-integer comparison still goes through i64 rather than f64, so values above 2^53 stay exact. Replaces compare_registers and compare_registers_eq with a single sql_compare. Verified that outer-join NULL fill, NULL handling in aggregates (COUNT(*)=5 vs COUNT(score)=4) and NULL ordering are all unaffected. 490 passing, 11 ignored (was 13), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The GROUP BY result block was laid out positionally as [group keys.., aggregates..] while the schema was built from the SELECT list, so the two disagreed whenever the projection was not written keys-first: SELECT COUNT(*), department FROM employees GROUP BY department COUNT,department Engineering,3 <- values transposed relative to the header The same mismatch produced a phantom trailing column named col1 for `SELECT COUNT(*) ... GROUP BY department`, where the group key is not projected at all. Output now goes through an explicit plan mapping each SELECT item to the group key or aggregate it names, so header and value order agree by construction and an unprojected key contributes no column. A projected column that is neither a group key nor inside an aggregate is now rejected -- it has no single value per group, so any answer would have been arbitrary. Also folds the two copy-pasted flush blocks into one emit_group_flush. They were near-identical (group-change flush and final-group flush), which is how HAVING came to be applied slightly differently in each, and the duplication is what let the layout bug exist in two places at once. 495 passing, 9 ignored (was 11), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Join registers are laid out left-table columns then right-table columns, and the raw block was emitted as-is while the schema was built from the SELECT list. Whenever the projection named the tables in any other order the two disagreed: SELECT orders.id, users.name FROM users JOIN orders ON ... orders.id,users.name John,101 <- values transposed relative to the header resolve_join_projection now also returns where each projected column lives as (side, index), and rows are gathered into that order before ResultRow. Applies to the NULL-filled rows of outer joins too, which must follow the same order. Landing this broke every outer join, which is worth recording because it is exactly the failure the label work was meant to prevent. Three jumps in the join compiler were hand-counted offsets -- one carrying the comment "IfPos (current), NullRow (+1), ResultRow (+2), Next outer (+3)" -- and the gather step made a result row take more than one instruction to emit. The +3 then landed inside the block it was supposed to skip, so matched rows emitted a spurious NULL row as well. Converted all three to labels. The goldens caught it immediately and named the six affected cases. 500 passing, 8 ignored (was 9), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sorter payload held only the output columns, and the sort spec mapped each ORDER BY key to a position among them. So a key had to be a selected column: SELECT name FROM employees ORDER BY salary DESC -> Invalid SQL query: ORDER BY column must be in SELECT list and an expression key was rejected outright with "Complex ORDER BY expressions not supported". The payload is now [sort keys.., output columns..]. A key becomes just another value computed per row -- through the shared expression compiler, so any expression works -- and the drain emits only the output half, leaving the keys internal. Both restrictions disappear together, and `ORDER BY salary * -1` and `ORDER BY LENGTH(name) DESC, name ASC` now work. Also converts the sorter loop's remaining hand-counted jump to a label and moves WHERE ahead of the payload build, so filtered rows never reach the sorter at all. build_sort_spec is deleted; the spec now follows from key position directly and cannot disagree with the payload layout. 510 passing, 6 ignored (was 8), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An aggregate argument is an ordinary expression, and after aggregation an aggregate result is an ordinary value. Neither worked. SUM(salary + age) was rejected with "Only column references supported in SELECT", because the argument went through resolve_column_expr, which yields a column INDEX and so accepts nothing but a bare column. Arguments now compile through the shared expression compiler, so SUM(salary + age) and MAX(LENGTH(name)) both work. SUM(salary) + 1 reported "Unsupported function: SUM". Aggregate detection looked only at the top level of a projection item, so a query with an aggregate inside arithmetic was not routed to the aggregate path at all and fell through to the plain table scan, where SUM is not a known scalar function. Detection is now recursive, aggregates are collected from anywhere within a projection item, and each is finalized into its own register and bound by call text. The surrounding arithmetic is then compiled by the ordinary expression compiler, which resolves the inner aggregate to its register -- no separate post-aggregation compiler needed. Making that work required moving the arithmetic and comparison operators out of the legacy fallback and into code_expr. The fallback path rebuilds a bare single-table context, so it discarded the aggregate bindings: every operand of `SUM(salary) + 1` was compiled in a context that had never heard of the SUM. Delegation is fine for arms that do not care about scope; it is wrong for anything that does. Also fixes the non-grouped aggregate path to emit in projection order, since it now compiles projection items directly rather than assuming one output per aggregate. 516 passing, 4 ignored (was 6), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every statement was folded into a single bytecode program sharing one result buffer and one schema, and pending modifications were applied only after the whole program finished. Two consequences: SELECT name FROM people WHERE age > 30; SELECT COUNT(*) FROM people COUNT Alice Charlie 3 Both statements' rows printed under the LAST statement's header. And a statement could not observe the one before it, so `UPDATE ...; SELECT ...` ran the SELECT against the pre-update table. Statements are now parsed once, then compiled and executed individually, with each statement's modifications applied before the next is compiled. Each gets its own result set, printed with its own header, and each observes the database as the previous statement left it -- which is also what `CREATE TABLE t; INSERT INTO t ...; SELECT * FROM t` requires, since the third statement cannot even compile until the first has taken effect. execute_vm is split into a driver and an apply_modifications function, and SqlExecutor::execute returns one table per statement rather than one merged table. The REPL shows the last statement's result. This surfaced a further defect, now on the red list: UPDATE is implemented as delete-plus-insert and the insert appends, so any row it touches jumps to the end of the table. That is not cosmetic -- with --write the reordering is persisted to the user's file. It stayed hidden because the existing writeback test updates the last row, which is already at the end. 515 passing, 3 ignored (was 4), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UPDATE compiled to DeleteRow followed by InsertRow, and the insert appends, so every row it touched jumped to the end of the table: UPDATE employees SET salary = 1 WHERE id = 1; SELECT id FROM employees -> 2,3,4,5,6,7,8,1 That is not cosmetic. With --write the reordering is persisted, so updating one field silently rewrote the user's CSV in a different row order. It stayed hidden because the existing writeback test updates the last row, which is already at the end, and because multi-statement scripts could not observe their own effects until the previous commit. Adds an UpdateRow opcode and a Replace modification that overwrite the row at its existing index. As a side effect the affected-row count becomes exact: it was inferred by matching per-table insert and delete tallies and calling equal counts an UPDATE, a heuristic that would misreport any statement genuinely mixing both on one table. 525 passing, 2 ignored (was 3), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An aggregate window with PARTITION BY and no ORDER BY has the whole partition as its frame, so every row must see the same value. The implementation accumulates as it streams, so it returned a RUNNING total: SELECT name, SUM(salary) OVER (PARTITION BY department) FROM employees Alice,70000 Charlie,135000 Frank,210000 -- all three should be 210000 With an ORDER BY the frame does grow row by row, so the running total is the correct answer there and is left alone. The rows are already sorted by partition, so each partition's LAST row holds its total. The engine now records where each partition begins as it emits, and a WindowFinalize opcode back-fills each partition from its final row. Recording boundaries rather than re-deriving them from the data matters because the partition key need not be a projected column -- in the query above, department is not selected. One WindowFinalize is emitted per aggregate window column; finalizing only the first left any others showing a running value. 525 passing, 1 ignored (was 2), clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ests rust-version claimed 1.70.0, which was never true and is further from true after the sqlparser upgrade. Verified empirically by building with older toolchains rather than inferring: 1.74 and 1.82 both fail, 1.88 succeeds. The binding constraints are transitive -- `home` via rustyline, and `psm` via sqlparser -> recursive -> stacker -- so reading the direct dependencies' own MSRVs (clap 1.74, csv 1.73) would still have understated it. Now 1.88. generated-icon.png (1.16 MB, referenced by nothing) and .replit were being packaged into every published crate. Both are now excluded; the package is 37 files. tests/tmp/ was checked-in scratch: four .rs files Cargo never compiles, shell scripts, and a committed test-output CSV. Removed. One file in it turned out to be a live fixture for a REPL test, so it moved to tests/data/repl_commands.txt where the other fixtures are. Also removes .sqawk_history, a stray REPL artifact committed despite being gitignored. tsq is installed onto users' PATH by `cargo install sqawk` and had no mention in any documentation and no tests. It is documented in the README now -- what it generates, its options, and that a seed makes runs reproducible -- and has six tests covering determinism (same seed, identical bytes), seed sensitivity, the generated tree, metadata, that sqawk can read what it writes, and that generated orders all reference real customers. Writing those tests surfaced a gap now on the red list: an aggregate over an explicit JOIN is rejected, because the join projection resolver accepts only column references so COUNT(*) never reaches the aggregate path. The README also undersold the tool badly: subqueries, set operations, window functions, DDL, CASE/CAST/COALESCE and || were all implemented and tested but absent from the feature list. CLAUDE.md's architecture notes referenced a src/string_functions.rs that does not exist and a join.rs that was deleted, and described compiler.rs as a monolith; all corrected, along with a note on which test suites carry the correctness contract. 531 passing, 2 ignored, clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doc/sql_reference.md had no mention of subqueries or window functions at all,
despite both being implemented and covered by tests -- grepping it for
"window", "OVER" or "ROW_NUMBER" returned nothing relevant. Subqueries appeared
only as a single `IN (SELECT ...)` row in a table of WHERE operators, with
nothing on scalar, EXISTS or correlated forms.
Adds sections for both, including the window frame rule that this session
changed: without ORDER BY in the OVER clause the frame is the whole partition,
with ORDER BY it grows row by row. Both spellings are shown side by side with
their differing output, since that is the distinction most easily got wrong.
Also documents two things that had no coverage anywhere and that a user of a
CSV tool will hit immediately:
- NULL handling: an empty field is NULL, comparisons follow three-valued
logic, a NULL row satisfies neither `x > 5` nor `x <= 5`, NULL = NULL is
UNKNOWN, aggregates skip NULLs, and ORDER BY sorts NULLs first.
- Type coercion: values are typed per cell, and a mixed number/string
comparison coerces when the string parses as a number. This is awk-like
rather than strict SQL, chosen so that `x + '1'` and `x > '1'` agree.
Unsupported neighbours are stated rather than left to be discovered: derived
tables in FROM, and explicit window frame clauses.
Every example in the new sections was executed before being written down, and
the correlated-subquery and NULL examples were checked for the right answer,
not merely for running without error.
531 passing, 2 ignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cross-platform job ran tests in parallel while both Linux jobs set
RUST_TEST_THREADS=1. The REPL tests spawn the binary as a subprocess and
several tests share fixtures under tests/data/, so macOS and Windows could
flake in a way Linux never showed. Now serialized like the others.
Adds two jobs:
- lint: cargo fmt --all -- --check and cargo clippy --all-targets
-D warnings. Both pass. The tree was fmt-clean before this session and had
drifted, since a good deal of this work was applied by script; restoring it
is part of this commit.
- msrv: cargo check pinned to the declared rust-version. That field claimed
1.70 for a long time while the dependency tree had moved past it, purely
because nothing checked. It checks the library and binaries only, not
--all-targets: rust-version is a promise to consumers, and
dev-dependencies are not part of that promise (the current dev tree needs
considerably more than 1.88).
All three gates were run locally exactly as CI runs them before committing.
531 passing, 2 ignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SELECT COUNT(*) FROM users JOIN orders ON ...` was rejected: the join projection resolver accepts only column references, so the aggregate never reached the aggregate path. The comma-join spelling of the same query already worked, because that path handles aggregates and GROUP BY. Since `A INNER JOIN B ON c` and `FROM A, B WHERE c` mean the same thing, an aggregate query over an explicit inner join is now rewritten to the comma form and handed to the machinery that can already answer it, rather than that machinery being duplicated into the join path. The rewrite is deliberately refused for outer joins. Moving the ON condition into WHERE would discard exactly the NULL-extended rows an outer join exists to produce, turning a LEFT JOIN into an INNER JOIN and quietly returning a smaller count. Those still report the feature as unsupported, which is the right failure: a clear error rather than a plausible wrong number. There is a test pinning that specifically. 540 passing, 1 ignored (was 2), clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SELECT ... FROM (SELECT ...) t` was rejected with "Only simple table scans are supported". There was no TableFactor::Derived handling anywhere, across twenty TableFactor match sites. Rather than teach every one of those sites about subqueries, a derived table is materialized before compilation: the subquery is executed, its result is registered under the alias, and the statement is rewritten to reference that name. The compiler then sees an ordinary table and needs no changes at all. Nested derived tables are materialized innermost-first. This is only possible because statements are now compiled and executed individually -- materializing requires a mutable database at a point where the old single-program design held only a shared borrow. The registration is scoped to the statement that declares it and dropped afterwards, so a later statement cannot reference the alias. An alias that would shadow an existing table is rejected rather than silently masking it, and a subquery with no alias is rejected, since the outer query would have no way to refer to it. Adds Table::set_name so a materialized result reports the name the outer query uses. This was the last item on the red list. tests/defects/ is now entirely green: every defect found in the audit has a test that passes, and the module has turned from a to-do list into regression cover. 544 passing, 0 ignored, clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ompiler
Three more private expression compilers removed, each of which understood a
different subset of SQL:
- the JOIN ON compiler handled exactly one top-level comparison, so
`ON a.x = b.y AND b.z > 1` was rejected;
- its operand compiler handled qualified columns and literals only;
- the multi-table WHERE compiler handled AND, OR and the six comparisons over
qualified columns and literals.
All now go through code_expr with a NameCtx carrying one source per table.
NameCtx already modelled several sources; what was missing was the logical
operators, which lived only in the single-table path. Those are now compiled in
code_expr -- initializing the result register BEFORE the short-circuit test,
which is the ordering the earlier AND defect established. emit_and/emit_or,
which evaluated both sides unconditionally, lose their last callers.
Capability gained rather than just lines removed. All of these were previously
rejected and now work:
ON users.id = orders.user_id AND orders.id > 102
ON users.id = orders.user_id AND UPPER(users.name) = 'JANE'
WHERE users.id = orders.user_id AND orders.id - 100 > 3
WHERE users.id = orders.user_id AND NOT (orders.id = 101)
WHERE users.id = orders.user_id AND date = '2023-01-15' -- unqualified
Outer joins keep their semantics: a LEFT JOIN with a compound ON still emits
the NULL-extended rows, verified by test.
Arms not yet migrated still take a single (table, cursor). With several sources
in scope, that source is now inferred from the column references inside the
expression instead of being refused outright -- a function applies to columns
of one table in practice -- and an expression genuinely spanning two tables
errors rather than resolving against an arbitrary one.
552 passing, clippy clean, fmt clean. compiler_join.rs down from 2994 to 2752.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Multi-table joins load every table's columns into registers before evaluating
join conditions, so a column reference there is a register offset rather than a
Column read from a cursor. That difference in addressing was why the path
carried its own condition and operand compilers -- ones that understood a
single top-level comparison over qualified columns and nothing else.
Src now models both modes: a source has a cursor and, optionally, a base
register holding its already-materialized columns. Resolution returns a
ColumnRef saying which it is, and a single emit site produces either a Column
or a Copy. One expression compiler now serves both loop shapes, and the last
two private operand compilers are gone.
The capability follows: a multi-table join ON clause can now carry a compound
condition.
SELECT users.name, products.name FROM users
JOIN orders ON users.id = orders.user_id
JOIN products ON orders.product_id = products.product_id AND products.price > 200
Verified against the unfiltered join that exactly the rows over the threshold
survive, rather than only that the query runs.
554 passing, clippy clean, fmt clean. compiler_join.rs 2994 -> 2684 across this
and the previous commit, with capability gained rather than lost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a two-level scope to NameCtx -- the query's own sources, plus the
enclosing query's for a correlated subquery -- and routes the last private
expression compiler, compile_correlated_operand, through it. SQL resolves a
name innermost-first, which is exactly this: an unqualified name binds to the
subquery, and one it does not have is looked up outward. Keeping the outer
sources in a separate list preserves that precedence and keeps the ambiguity
check meaningful, since two tables in one FROM are ambiguous but an inner name
shadowing an outer one is not.
Doing that exposed a real hazard, and briefly created it. Function calls
reached the compiler through the single-source fallback, which rebuilt a
context containing only the innermost table. Inside a correlated subquery that
made `UPPER(e.department)` resolve `e.department` against the SUBQUERY table,
so both sides of the comparison became the same expression and every row
matched. Before this change the query errored; for one build it returned all
eight rows instead. An error becoming a wrong answer is the worse direction, so
the fix is the real one rather than a guard: compile_function and its ten
helpers now take a NameCtx, and code_expr has an explicit Function arm passing
the caller's scope through.
SELECT name FROM employees e WHERE EXISTS
(SELECT 1 FROM employees x
WHERE UPPER(x.department) = UPPER(e.department) AND x.age > 40)
now returns David, Grace -- agreeing with the same query written without the
UPPER, which is the property the new test asserts. Wrapping both sides of a
comparison in a function must not change which rows qualify.
Also drops OuterColumnRef::column, which nothing reads now that resolution goes
through NameCtx.
557 passing in debug and release, clippy clean, fmt clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`FROM users u JOIN orders o ON u.id = o.user_id` failed with "Table 'u' not found". Explicit joins resolved column qualifiers against the table name alone and ignored any alias, while a single table and a comma join both honoured one. This is a documentation bug found by running the examples in doc/user_guide.md rather than reading them: all three of its Joins examples use aliases, so the entire section documented syntax the tool rejected. Fixing the code was the right direction rather than rewriting the docs, since aliases already worked in the other two join forms and the documented spelling is ordinary SQL. Aliases now flow into both the join condition and the projection, for two-table and multi-table joins alike. Fixing it also uncovered a regression I introduced when consolidating the join compilers: compile_multi_join_condition was rewritten to look each name up in the database, but its caller passes ALIASES, not table names. Nothing failed at the time because no test anywhere used an aliased join. It now takes the tables directly, and four tests cover aliased inner, left, three-table and comma joins. 465 integration tests passing, clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
compile_join_condition lost its callers when the alias fix routed every site through compile_join_condition_with_refs. Removing it, and folding its doc comment onto the surviving function. Fixes a clippy failure I committed in the previous change: I misread the warning count as clean before committing. The gate now passes again. 561 passing, clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found while verifying a documentation claim rather than by reading code. The SQL reference said explicit frames were unsupported; the query ran anyway, so I checked what it returned: SELECT name, SUM(salary) OVER (ORDER BY salary ROWS BETWEEN 1 PRECEDING AND CURRENT ROW) produced a running total over the whole partition -- byte-identical to the same query with no frame clause. For a two-row sliding window Eve should be 115000 (55000 + 60000); it reported 160000. The frame was parsed and discarded. Now rejected with a clear message. Frames remain unimplemented, but an error is the right failure: the previous behaviour returned a plausible number that was wrong, and matched the shape of the answer closely enough to be believed. 566 passing, clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Auditing the docs meant running every documented command, which turned up three problems in the user guide's REPL section. `.verbose` is documented but does not exist -- the REPL answers "Unknown command". Seven commands that DO exist were undocumented: .cd, .changes, .load, .print, .show, .stats and .version. The command table now matches what .help prints, and says so, since .help is the authoritative list. `.version` reported "Sqawk version 0.1.1" while the crate was at 0.8.0. The string was written out by hand and nothing asserted on it, so it drifted seven minor versions. It now comes from CARGO_PKG_VERSION, which cannot drift, and also reports the sqlparser version -- the single most useful thing to know when a statement is rejected. A test asserts the reported version matches the crate. Every command in the corrected table was executed to confirm it exists and responds. 563 passing, clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by running the user guide's example REPL session instead of reading it. The guide shows a customers/sales join grouped by name, ordered by total descending, limited to 3 -- and sqawk returned the rows in ascending order, ignoring both clauses. `LIMIT 2` on the same query returned three rows. Grouped multi-table queries emit rows directly, like the single-table grouped path, so they now get the same post-processing treatment: sort and limit applied to the finished result set. Ungrouped comma joins had a subtler version. Their scan applies LIMIT as it goes, which is correct only without ORDER BY -- with one it keeps the first N rows and then sorts those, rather than returning the top N. When ORDER BY is present the scan now runs unlimited and both clauses are applied afterwards, in that order. Also corrects two remaining user-guide inaccuracies: an example that used aliases `a` and `b` without ever declaring them, and the claim that sqawk "loads all data into memory" -- on-disk tables are memory-mapped and only copied to the heap when first modified. 566 passing, clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes sqawk’s SQL engine and its test suite as part of an audit-driven correctness push, including a sqlparser upgrade and a rework of VM execution semantics to properly handle multi-statement scripts and more SQL surface area.
Changes:
- Upgrade
sqlparser(and add an AST compatibility layer) while extending the VM/compiler to cover previously-missing SQL behaviors (NULL semantics, ORDER BY over joins/GROUP BY, DISTINCT aggregates, derived tables, etc.). - Change execution to compile/execute per statement and return one result set per statement, fixing multi-statement output and DML-then-SELECT visibility.
- Add substantial regression coverage (new defects suite, exact-output helpers, new fixtures,
tsqbinary tests) and clean out temporary test artifacts; update docs and CI (lint + MSRV jobs).
Reviewed changes
Copilot reviewed 49 out of 51 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tsq/mod.rs | Adds integration tests for the tsq data generator (tree, determinism, sqawk-readability, referential integrity). |
| tests/tmp/test-dialects.rs | Removes temporary/manual parser experiment code. |
| tests/tmp/test_script.sh | Removes temporary/manual REPL script. |
| tests/tmp/test_schema_display.sql | Removes temporary/manual SQL fixture. |
| tests/tmp/test_repl_schema.sh | Removes temporary/manual REPL script. |
| tests/tmp/test_repl_commands.txt | Removes temporary REPL command input file. |
| tests/tmp/test_pattern_matching.txt | Removes temporary REPL command input file. |
| tests/tmp/test_output.csv | Removes temporary output fixture. |
| tests/tmp/test_input.txt | Removes temporary REPL input file. |
| tests/tmp/test_exit_code.txt | Removes temporary REPL input file. |
| tests/tmp/test_data.csv | Removes temporary CSV fixture. |
| tests/tmp/test_changes_command.txt | Removes temporary REPL input file. |
| tests/tmp/README.md | Removes documentation for the removed tmp test directory. |
| tests/tmp/dialect_test.rs | Removes temporary binary stub. |
| tests/tmp/debug_sql.rs | Removes temporary debug utility. |
| tests/tmp/debug_create_table.rs | Removes temporary debug utility. |
| tests/tmp/create_table.sql | Removes temporary SQL file. |
| tests/repl/test_repl_from_file.rs | Switches REPL canned session input to tests/data/ instead of tests/tmp/. |
| tests/repl/commands.rs | Adds a regression test ensuring .version matches the crate version. |
| tests/mod.rs | Registers new test modules (defects, golden, tsq) in the test harness. |
| tests/helpers/mod.rs | Adds exact-output helpers (assert_query, writeback helpers, fixture path helpers). |
| tests/defects/mod.rs | New “red list” regression suite encoding previously-found correctness defects as exact-output tests. |
| tests/data/repeats.csv | Adds fixture for repeated values. |
| tests/data/purchases.csv | Adds fixture for FK/orphan scenarios. |
| tests/data/nullable.csv | Adds fixture to exercise NULL semantics. |
| tests/data/customers.csv | Adds simple fixture for customer-based tests. |
| src/vm/tests.rs | Adds VM opcode tests for Not and Compare/Jump. |
| src/vm/mod.rs | Switches to per-statement compile/execute, adds derived-table materialization, and returns multiple result tables. |
| src/vm/engine.rs | Adds DISTINCT aggregates, UpdateRow, window partition tracking + finalize, register sizing via compiler count, 3VL comparisons, SortResults, and other correctness fixes. |
| src/vm/compiler_window.rs | Adapts to new sqlparser AST shapes and fixes unordered partition aggregate semantics; rejects explicit frames. |
| src/vm/compiler_join.rs | Adds join operator classification, alias-aware name resolution, projection-order emission, label-based jumps, and broader expression support via shared expression compiler. |
| src/vm/compiler_dml.rs | Makes UPDATE RHS expression compilation column-aware and switches UPDATE to in-place row replacement (UpdateRow). |
| src/vm/compiler_ddl.rs | Adapts CREATE TABLE options parsing to new sqlparser AST shapes. |
| src/vm/compiler_aggregate.rs | Improves aggregate detection/binding, DISTINCT support, GROUP BY correctness (WHERE/HAVING/order), and projection order alignment. |
| src/vm/bytecode.rs | Introduces new opcodes (UpdateRow, Not, Compare, Jump, SortResults, WindowFinalize) and aggregate flag constants; adds program register count. |
| src/vm/ast_compat.rs | New compatibility helpers to normalize sqlparser AST changes (args, GROUP BY, ORDER BY, LIMIT/OFFSET, UPDATE targets, options). |
| src/table.rs | Adds table renaming (for derived tables) and in-place row replacement. |
| src/sql_executor.rs | Returns a vector of result tables (one per statement) and adapts REPL result handling accordingly. |
| src/repl.rs | Makes .version dynamic (crate version) and includes sqlparser version string. |
| src/main.rs | Prints each statement’s result set independently (per-statement headers). |
| src/join.rs | Removes old join module (now handled in VM/compiler). |
| README.md | Updates advertised feature set and documents tsq. |
| doc/user_guide.md | Updates repo URL, REPL command docs, and operational guidance around NULL/coercion/multi-statement behavior. |
| doc/sql_reference.md | Expands reference for subqueries, derived tables, window functions, NULL handling, and coercion. |
| doc/database.md | Updates architecture docs to match VM execution model, NULL/coercion semantics, statement execution model, joins, and update behavior. |
| CLAUDE.md | Updates contributor guidance to match new VM architecture and test strategy. |
| Cargo.toml | Raises MSRV and upgrades sqlparser dependency; updates package excludes. |
| Cargo.lock | Updates lockfile for dependency changes from the sqlparser/MSRV bump. |
| .github/workflows/TestingCI.yml | Serializes non-Linux tests, adds lint job, and adds an MSRV check job. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… comment
Three findings from a Copilot review of this branch, all fair.
1. affected_rows kept a stale count (medium).
The count was only overwritten when the new one was non-zero, which
conflated "this statement was not DML" with "this DML matched no rows". So
UPDATE employees SET salary=1 WHERE department='Engineering'; -- 3 rows
UPDATE employees SET salary=2 WHERE department='NoSuchDept'; -- 0 rows
reported 3 rows affected for the second statement. Reproduced before fixing.
The count now follows the statement kind rather than the value: INSERT,
UPDATE and DELETE set it, including to zero, matching the set SQL's
changes() covers. A SELECT or DDL statement leaves the previous count alone
instead of clearing it, which is what the original guard was reaching for.
The REPL also reports zero now. Under `.changes on` a statement matching
nothing previously produced no output at all, leaving it ambiguous whether
it had run -- and making the bug invisible. A test covers both counts.
2. A tsq test used stdout.contains("40"), which also passes on 140 (low).
Fair, and it contradicts guidance in this repo's own CLAUDE.md, added
earlier in this same branch, to prefer exact-output assertions. Now asserts
the complete output is exactly ["COUNT", "40"].
3. WindowFinalize carried a comment describing WindowValue's operands (low).
Worse than reported: the scripted edit that introduced the opcode also
stripped WindowValue's own comment and appended it to WindowFinalize, so one
opcode was undocumented and the other documented wrongly. Both restored,
with WindowFinalize describing its actual operand (a result column index).
567 passing, clippy clean, fmt clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
No description provided.