fix: PartiQL read correctness, the two missing batch member fields, and the measurement-gated perf pair - #190
Merged
Merged
Conversation
`"table"."index"` tokenises as three tokens and the table name parser took one, leaving the `.` and the index name for the next clause to parse against. A SELECT lost its WHERE, an UPDATE its SET, and a DELETE reported requiring a WHERE clause it plainly had. The qualifier is now parsed and carried. UPDATE and DELETE reject it during execution and INSERT at parse, reporting the table name's position the way DynamoDB does. Every Statement variant is now non_exhaustive, so a caller matching one exhaustively needs a `..`.
A qualified SELECT scanned the base table, so it returned items the index does not contain and charged the read to the wrong arm. It now resolves the index, reads through it, and reports capacity against that index with the table arm at zero, the same shape Query and Scan already produce. An unknown index name is rejected, without the index name in the message: Query and Scan append it and this surface does not. A strongly consistent read of a GSI is rejected too, again with its own wording. The continuation token carries the base table key, so rows sharing an index key are no longer skipped, and it is bound to the index that minted it. Measured against eu-west-2 on 2026-08-15.
…rapper DynamoDB wraps a malformed statement in "Statement wasn't well formed, can't be processed: " and reports a handful of rejections on its own terms. Every parse failure went through the wrapper here, so a three-part name, an empty path component and a qualifier on an INSERT all came back double-enveloped. The parser now says which envelope its rejection takes, rather than leaving each of the three call sites to guess from the message text.
A GSI rejects a projection naming an attribute it does not project, and an LSI rejects a filter on one. The two sides really are asymmetric on AWS: a GSI filter on an unprojected attribute matches nothing rather than failing, because the attribute is absent from every entry. An LSI serves a projection naming an unprojected attribute by reading the base table. That reach-back is not implemented here, so such a projection still comes back empty, and it is the one case in this area still diverging.
The rejection was applied to LSIs and not GSIs, which fitted the two cases measured at the time and was the wrong reading of them. Both kinds reject a filter on an attribute the index does not carry, and only when the read is keyed on the index partition key; an unkeyed read is a scan and matches nothing instead. Four further cases settle it. An unkeyed LSI filter is accepted, a keyed GSI filter is rejected, and an unkeyed filter with two conditions is accepted, which rules out the index kind and the condition count in turn. The message says "Secondary index" on both kinds. The projection rule is a genuine GSI/LSI split and is unchanged.
PartiQL had its own comparison handling strings, numbers and booleans, with a catch-all answering every other type false for = and true for <>. So a predicate on a set, list, map, binary or null never matched whatever the values were, and since the same code gates UPDATE and DELETE, a write conditioned on one of those types could never fire. The condition-expression engine already compares all of them, including sets without regard to order. A capture found the two surfaces agree on every type on real DynamoDB, so PartiQL now shares that implementation rather than growing a second copy of it, and the two operator enums become one.
Four changes, all from review of the preceding commits. An unterminated quoted name panicked the parser: `SELECT * FROM "` sliced a one-character string from index 1, and the release profile aborts on panic, so one malformed statement took the process down. The tokeniser now rejects it as the syntax error it is. That fault predates the index qualifier, which added a second way to reach it. A continuation token stripped of its base table key but carrying a valid fingerprint ended an index walk after one row rather than being rejected. The two halves are now checked separately. Token positions are computed when the one message that reports them fires, rather than building a line and column for every character of every statement to serve a case that almost never happens. The projection rule lived twice, once in `build_index_item` and once in the PartiQL read path. It now lives in `IndexDef` and both call it, which is the same fault the predicate work was fixing a few commits earlier. `ParseError` derives `Error` like `DynoxideError` rather than only `Display`.
The changelog's Notes section still said a SELECT against an index scans the base table and drops its WHERE clause, and the compatibility summary still called the index arm a known read-side gap. Both were true when written and are the opposite of what the code now does. The remaining read-side gap is restated as what it actually is: a PartiQL read with no key condition is charged on the rows it returns where DynamoDB charges a flat figure for the scan, and that applies to a base table read as much as an index one.
A second capture round, prompted by review, settled five behaviours the first one had not measured. Three of them the code already had right; two it did not. An IN on the index partition key counts as keyed, so an unprojected filter beside it is rejected. The read still scans, because an IN cannot be pushed down as a single key, but the rejection follows the shape of the key condition rather than what the read does with it. An index key reached through OR does not count, and AWS accepts an unprojected filter there. An index-qualified SELECT inside a transaction is rejected outright rather than served and charged to the wrong arm. Confirmed unchanged: the empty-component rejection belongs on the table half as well as the index half; the unprojected-filter rejection covers IS MISSING and BEGINS_WITH, not just equality; and ordering on binary compares the bytes. Two divergences are now pinned by tests rather than left implicit. Ordering between mismatched types answers false where DynamoDB rejects the statement, and a parenthesised WHERE clause is a parse error here and valid on DynamoDB. Both predate this work.
Both were listed as supported and neither was. The clause parser was a flat OR of ANDs with no notion of parentheses, so even `WHERE (a='1')` was a parse error, and `NOT` was recognised only as part of `NOT EXISTS` and `NOT BEGINS_WITH`. DynamoDB parses all of it. The clause is now read into a tree, `NOT` is driven down to the leaves by De Morgan, and the result is flattened back into the OR of ANDs the executor and the key pushdown already read, so nothing below the parser changes. `NOT` over `BETWEEN` becomes a disjunction and over `IN` a conjunction, neither of which fits a single condition, and `NOT CONTAINS` gains the variant it needed. Flattening a clause distributes AND over OR, so an alternation nested deeply enough is rejected as too complex rather than expanded. Also rejects an ordering comparison whose operand has no ordering. DynamoDB orders S, N and B, rejects `<`, `<=`, `>`, `>=` and `BETWEEN` against anything else, and does it before resolving the table. dynoxide answered no rows. An earlier note called this a type mismatch; it is not, and `S < N` is accepted on both sides. Captured eu-west-2 2026-08-15.
Three things found while working in this code and previously left standing. A batch SELECT must name a single item. One that does not resolve to a primary key, or that names an index, is now rejected against itself while the rest of the batch runs. Both shapes carry the same message on DynamoDB, so an index-qualified batch read is unreachable even when it does name the key. An LSI serves a projection naming an attribute it does not carry by reading the base item, which a GSI cannot do and rejects instead. dynoxide returned rows of empty objects. The base reads land on the table arm at read granularity apiece, leaving the index arm to cover the index read: three rows served this way report total 2, table 1.5, lsi 0.5. A batch member whose statement ran and failed echoes its table, where one rejected before it ran does not. ConditionalCheckFailed and DuplicateItem carry it; a ValidationError does not, which is what an invalid RETURNING variant is. Captured eu-west-2 2026-08-15. The batch round now matches on 13 of 15, with both remaining cases waiting on a member's ConsistentRead.
…ilure BatchStatementRequest carried Statement and Parameters alone, so a member setting either of DynamoDB's other two fields was parsed as though it had not. ConsistentRead is per member and sets the rate that member's read is charged at. A keyed batch SELECT costs 0.5 without it and 1 with it, and a batch mixing the two sums both rates rather than taking one mode. It does not change which rows come back, because every read against SQLite is already consistent. ReturnValuesOnConditionCheckFailure is accepted and inert, which is what DynamoDB does with it: a member whose condition fails returns the same response whatever the option says and never the item. The same option on a TransactWriteItems ConditionCheck does return it, which is what rules out a bad measurement rather than a real inertness. Deserialising it means a client setting it meets a field dynoxide knows rather than one it drops. The batch round now matches on all fifteen cases.
The fan-out builds the projected entry to store it, and the capacity calculation built the same projection again to measure it. On a table with two indexes an overwrite made six projections where four would do, and an insert four where two would do, whether or not anyone asked for the figures. The fan-out now builds it once and hands it on. Measured with iai-callgrind, which is deterministic where the wall-clock suite is not: an overwrite against two indexes drops 2.6%, an insert against one drops 3.7%, both on instruction counts. Adds the benchmark that shows it. The suite could not: it only inserted fresh keys against a single-index table, which is the one shape with no old image to rebuild and half as much of it. Adds counters behind a feature for the questions in this area that are about how many times something happens rather than how long it takes; they compile to nothing when the feature is off.
…urned DynamoDB sizes a read before the WHERE clause and before the projection, so a SELECT matching one row costs what a SELECT matching every row costs, and asking for one attribute costs what asking for all of them costs. Scan and Query already did this; the PartiQL executor summed the rows it was about to hand back, which under-reported any filtered or projected read. The window now carries the bytes it read alongside the rows that matched. Confirmed against eu-west-2: the filter cases agree on ten fixtures and the projection cases on six. Scan's own behaviour was correct and had no test holding it there, so this covers both.
The counters are process-wide and the harness runs tests in parallel, so one test's fixture setup landed inside another's measurement. An insert into a two-index table read as 8 entries built against a true figure of 2, which puts an insert above an overwrite and inverts the answer. Each test now holds a lock for its whole body, setup included, because building a fixture table reads metadata too. Three parallel runs now agree with each other and with a single-threaded run.
The index fan-out called index_write_units for every index on every write, and sizing needs the old image projected on top of the new one the write already built. ReturnConsumedCapacity defaults to NONE, where the response builders throw those maps away, so the default write paid for a figure nobody could see. The maintenance helpers now take the caller's ReturnConsumedCapacity and skip the sizing when it asks for nothing. They take the mode itself rather than a boolean, so a call site is right by forwarding a field it already holds. TTL sweeps pass None, having no caller to report to. On an overwrite against a table with two indexes, entries built goes from four to two, which is what an insert costs. Asking for INDEXES still builds four, so nothing is lost where the figures are wanted. The gate sits in the same per-index loop as the write operations, so a version placed one line out would stop maintaining indexes rather than stop measuring them, and no capacity test would catch it because they all run in a mode that does the work. The new tests read the indexes back through a query instead; four of the six fail against that mistake.
…tement The duplicate-target check loaded a table's metadata and parsed its key schema to build a (table, pk, sk) tuple, then threw both away; the executor resolved the same table again a moment later, and sizing a failed member resolved it a third time. A 25-statement batch performed 50 metadata loads, or 75 when the members failed. The preparation pass now keeps what it resolved and hands it to the executor. It keys that by table rather than by statement, because the metadata and the key schema are per table and only the item key is per statement, so a batch against one table resolves it once however many statements it carries. The failure path reuses the target the preparation pass already holds. The same change applies to ExecuteTransaction, which had the identical shape. A 25-statement batch goes from 51 metadata loads and 51 key schema parses to one of each. That matters most on the wasm backend, where every load crosses the bridge to a JS worker and nothing caches the result, but the key schema is a JSON parse and was paid on both backends. Keying by table is the part that could go quietly wrong: a lookup returning another table's entry would run a statement against the wrong key schema, and the existing multi-table coverage would not catch it because those tables are keyed alike. The new tests mix a hash-only table with a composite one.
A statement nested deeper than the parser can walk aborted the process. Around 250 nested parentheses, or a few thousand leading NOTs, overflowed the stack, and the release profile aborts on panic, so one statement of about a kilobyte took the host down. Nested list and map literals reached the same end by a second route that predates this branch. Both descents now carry a depth budget and reject past it, the way an over-complex clause was already rejected. A negated comparison dropped rows the attribute is missing from. NOT a='x' answered on rows holding an a and skipped the rest, because negation flipped the operator and <> is false on a missing path, while NOT CONTAINS wrapped instead and kept them. The two disagreed with each other on the same data. Negation now wraps in every form, so a row the comparison cannot answer for is a row the negation matches. A batch SELECT naming a table that does not exist reported that it must specify a primary key in the where clause, which it had. The check that reads a member's target cannot tell a missing key from a missing table, and only the reads went through it, so the same batch reported the missing table correctly for a write and misleadingly for a read. An LSI read that reaches back to the base table is charged on the bytes each base read moved rather than a flat half unit apiece. The flat figure came from one capture of three small rows and holds up to 4KB. Captured again in eu-west-2 on 17 August across item sizes: the same three rows at 9KB apiece cost 4.5 rather than 1.5, and 9 under ConsistentRead. Also drops statement_target and the prepared-statement type it fed, which lost their last caller when the batch pass moved to resolving per table.
…s in CI The compatibility summary gained NOT, parenthesised grouping and the index qualifier when they landed; the MCP tool descriptions did not, and that schema is what an agent reads at the moment it writes a statement. They now say the same thing, along with the two batch member fields. ExecutePartiql also gains ConsistentRead, which was harmless to omit until it started deciding both the rate a read is charged at and whether a GSI-qualified select is rejected. The counter harness had no CI leg, so the evidence for the two performance changes on this branch would have rotted without anything noticing. Two counter tests printed their figures and asserted nothing, and two index tests asserted only that a row was absent, which a write that never populated the index also satisfies.
…rstand GetItem, PutItem, UpdateItem, DeleteItem, Query and Scan each check the value against the enum during their own deserialisation. The three PartiQL surfaces derive their requests, so they had nowhere to put the check and took anything. A typo read as NONE and reported nothing, which was easy to miss when that was all it decided. It now also decides whether a write sizes its indexes, so the same typo quietly skips that too. The check moves into one place rather than being spelled out a seventh time. Also records on ResolvedTable that a batch resolves a table before any of its statements run, so require_table no longer runs per statement and a table altered part way through a batch is served from the snapshot taken at the start of it.
Contributor
Criterion Benchmark ResultsBaseline is the per-benchmark median of the last 5 stored runs, so one unusually fast or slow runner cannot skew the comparison. The range column is the spread across those runs.
Runs in the baseline
|
…e on the clause it flattens to A clause could be made expensive in two ways the depth budget does not reach. Flattening an alternation built the whole intermediate before testing the 256-group cap, so around 925KB of statement peaked at gigabytes. Flattening a conjunction cloned conditions with only the group count bounding it, and a 4000-element negated IN crossed with eight negated pairs took about 27 seconds of CPU from 32KB of text. The alternation now tests as it goes, the conjunction skips the clone when a term is a single group, and a budget bounds the conditions a clause can materialise as well as the groups. The single-item requirement on UPDATE and DELETE went back to reading the flattened clause. Keying it on whether the text contained an OR was wrong both ways round: NOT (NOT pk='a' AND NOT pk='b') names two items and was accepted, updating one of them, while pk='a' AND NOT (v='x' OR v='y') names one and was refused for an OR it flattens away. Every group must now pin the same key, and the clause as written only chooses the wording so an OR still reads as one. An LSI reach-back happens before the filter rather than after it, so the table arm covers every row walked. Captured eu-west-2 2026-08-17: three rows of 9KB cost 4.5 whether the filter keeps all of them or none. A replayed transaction is checked for an unrecognised ReturnConsumedCapacity before the idempotency cache answers it, rather than only on the first call.
…rser Six operations each spelled out the ReturnConsumedCapacity enum and its rejection message, and the three PartiQL surfaces had a ninth copy in a shared helper. All nine now ask the helper, and the ones that collect several failures still report the envelope they did before. The parser caps nesting at 64 levels, and three places still advertised parenthesised grouping to any depth, including the tool schema an agent reads before it writes a statement. AGENTS.md listed four CI feature configurations where there are now five.
Contributor
Criterion Benchmark Results
|
Contributor
Criterion Benchmark Results
|
This was referenced Aug 18, 2026
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
1 participant
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.
What this changes
SELECT * FROM "table"."index"threw the index away and scanned the base table, so it returned rows the index does not hold. Fixing that turned up a run of problems in the same code: predicates that never matched on sets, lists, maps, binary or null; aWHEREclause that could not parse the grouping andNOTthe docs already promised; two batch member fields dropped on the floor; and reads charged on what they returned rather than what they read.A statement could also take the process down, three ways. Around 250 nested parentheses overflowed the stack, and the release profile aborts on panic. Flattening a wide clause was unbounded in two more: 32KB of text burned about half a minute of CPU, and 925KB peaked at gigabytes before the cap rejected it. All three are bounded now.
Every figure here is captured against eu-west-2.
Two performance items land with it, both measured before they were taken. Index writes are sized only when someone asked for the figure, which halves the projections an overwrite does against a two-index table. And a batch resolves each table once instead of twice per statement, taking a 25-statement batch from 51 metadata loads to one. That second one matters most on wasm, where every load crosses the bridge to a JS worker.
Breaking changes
Eight, all in the changelog. Two behavioural: a batch or transaction may no longer mix reads and writes or name one item twice, and the PartiQL surfaces now reject a
ReturnConsumedCapacityoutside the enum. Six are Rust API only, around the parser's public types,WhereClause,WhereConditionandexecute_page. The wire API is untouched, so this is a minor bump.Closes #179, #186, #183, #184, #185, #182.
Checklist
Tests added or updated
cargo fmt --checkandcargo clippy -- -D warningspass locallyCHANGELOG.mdupdated if this is a user-visible changeLinked issue, discussion, or a short note explaining the motivation
I agree my contribution is licensed under the project's terms (MIT License and Apache License, Version 2.0)
DynamoDB compatibility note- nothing here introduces a divergence; every change moves towards DynamoDB.