Conversation
8 tasks
EnRaiha
force-pushed
the
fix/issue294-seq-defaults
branch
4 times, most recently
from
September 7, 2026 23:19
c36e498 to
29a2350
Compare
EnRaiha
added a commit
to EnRaiha/nodedb
that referenced
this pull request
Sep 8, 2026
Grand plan issue NodeDB-Lab#294 (PR NodeDB-Lab#303): register nextval/currval/setval in the function registry (A1), guard const-fold and row-scope evaluation with a typed FeatureNotSupported error (A2/A3), and carry it end-to-end as SQLSTATE 0A000 (A5/A6). - nodedb-query: EvalError::FeatureNotSupported; eval_function dispatch arm so accessors can never fall through to the geo fallback's silent NULL. - nodedb-sql: registry entries (plan-time gate + arity/typing); SqlError variant; const_fold exhaustive arm classifies the fold path loud. - error plumbing: Error/ErrorDetails/ErrorCode(1208)/msgpack tag 81/ envelope + data-plane wire variants, pgwire + gateway + DDL sqlstate maps -> 0A000; NodeDbError::feature_not_supported builder. - Executor side-channels that collapsed ANY EvalError to 22012 now discriminate variants (provider/kv/doc scans, grouping sets, aggregate HAVING, cold filter) - division keeps 22012, accessors surface 0A000. - Wire coverage (8 cases): FROM-less SELECT, SELECT list (columnar, document), WHERE/ORDER BY/VALUES on kv, INSERT..SELECT, derived-table constants (also pins issue NodeDB-Lab#295's mod(5,0) 22012 class), DEFAULT nextval regression across engines (11 existing cases stay green). kv-engine SELECT projection expressions are not evaluated (pre-existing: SELECT 1 + 1 FROM kv returns an empty column); documented in-test.
EnRaiha
added a commit
to EnRaiha/nodedb
that referenced
this pull request
Sep 8, 2026
…tval in the function registry so the plan gate admits and types them, guard const-fold and row-scope evaluation with a typed FeatureNotSupported error, and carry it end-to-end as SQLSTATE 0A000. - nodedb-query: EvalError::FeatureNotSupported; eval_function dispatch arm so accessors can never fall through to the geo fallback's silent NULL. - nodedb-sql: registry entries (plan-time gate + arity/typing); SqlError variant; const_fold exhaustive arm classifies the fold path loud. - error plumbing: Error/ErrorDetails/ErrorCode(1208)/msgpack tag 81/ envelope + data-plane wire variants, pgwire + gateway + DDL sqlstate maps -> 0A000; NodeDbError::feature_not_supported builder. - Executor side-channels that collapsed ANY EvalError to 22012 now discriminate variants (provider/kv/doc scans, grouping sets, aggregate HAVING, cold filter) - division keeps 22012, accessors surface 0A000. - Wire coverage (8 cases): FROM-less SELECT, SELECT list (columnar, document), WHERE/ORDER BY/VALUES on kv, INSERT..SELECT, derived-table constants (also pins issue NodeDB-Lab#295's mod(5,0) 22012 class), DEFAULT nextval regression across engines (11 existing cases stay green). kv-engine SELECT projection expressions are not evaluated (pre-existing: SELECT 1 + 1 FROM kv returns an empty column); documented in-test.
EnRaiha
force-pushed
the
fix/issue294-seq-defaults
branch
from
September 8, 2026 13:42
078bfe9 to
2d6f477
Compare
This was referenced Sep 8, 2026
EnRaiha
force-pushed
the
fix/issue294-seq-defaults
branch
from
September 9, 2026 12:09
2d6f477 to
5f23c5b
Compare
DEFAULT expressions were honored only where an engine happened to wire them: uuid() worked on strict, silently vanished on schemaless document, kv, and columnar; sequence defaults (nextval) did not run anywhere — the pure planner evaluator does not know the accessors, and every engine path swallowed "no value" into a missing column. NodeDB-Lab#294's repro: DDL accepted `DEFAULT nextval('s')`, every insert silently committed NULL (including primary keys). One defect at two layers, fixed together: 1. Storage: the catalog adapter's schemaless branch discarded the DEFAULT clause even though `stored.fields` carries the full DDL constraint text. UUID-class defaults on the document engine now materialize. 2. Evaluation: one shared per-row expander (`expand_row_defaults`) serves INSERT, UPSERT, the columnar batch encoder and the kv converter — replacing the per-path pure-evaluator swallows. Sequence accessors are classified by ONE canonical parser (`sequence_accessor`, byte-safe, robust-parsing-gate clean) shared between planner and convert: - nextval('name') advances the CP-side registry per row; - currval/setval in a DEFAULT raise loudly (no per-row meaning); - accessor-shaped but malformed (nextval('')) raises loudly; - unknown sequences raise naming them. A DDL-accepted DEFAULT never silently becomes NULL. Mechanics: SequenceRegistry threaded SharedState -> QueryContext -> ConvertContext; SqlPlan::KvInsert carries key_column + sequence_defaults (kv planner skips them; converter fills key slot + mirrored value map so scans read the defaulted key back); rows.rs and the doc-family paths share the same expander; nodedb_value_to_sql moved into value/convert. Verified: wire sequence_default_all_engines 8 tests (strict/doc/kv/ columnar/doc-UPSERT nextval fills; uuid sentinel; currval loud; malformed loud; unknown-seq loud) + sequence_default_typed 3; regression sentinels green (kv_column_defaults, dml_returning, not-null gate). fmt clean; clippy -D warnings clean; robust-parsing + calvin gates clean; sql 864 + types 688. Flow also verified by instrumented run (SEQDBG logs): doc nextval filled Integer(1),(2); currval reached the loud branch; uuid_v7 went through the pure evaluator. Not included here (tracked separately): SELECT/currval/setval evaluation — accessors stay unregistered so the gate keeps raising 42883 loudly; the registry registration ships together with the SELECT-side fold so no commit ever leaves a registered call folding to NULL. Partially addresses NodeDB-Lab#294.
Register nextval, currval and setval so the planner admits and types them, then make every evaluation that escapes the DEFAULT path fail loudly: the scalar evaluator raises a typed FeatureNotSupported error, const folding classifies it exhaustively, and the error plumbing maps it to SQLSTATE 0A000 end to end. Executor lanes that collapsed any evaluation error to division-by-zero now discriminate variants, so the typed code survives to the client. Adds the expression-context wire coverage, a parser corpus locking the tolerant byte-safe accessor recogniser, and a static gate script that keeps the DEFAULT-plane seam honest.
EnRaiha
force-pushed
the
fix/issue294-seq-defaults
branch
from
September 9, 2026 20:40
2d57c41 to
6a46e9a
Compare
kv scans now carry the SELECT projection list and computed columns like the other engines, so expression projections evaluate per row instead of surfacing as NULL; plain projections stay full-row so clone-source merges keep the keys their tombstone suppression needs. Sequence accessors in constant contexts (FROM-less SELECT, VALUES cells) evaluate through the control-plane registry at plan time, in expression order; folded accessor plans are never admitted to the physical-plan cache, EXPLAIN does not advance the registry, and row-scope contexts keep raising 0A000 instead of evaluating. Executor lanes that collapsed evaluation errors to generic classes now propagate typed SQLSTATEs end to end, and the tokio worker stack is raised to 16 MiB so deep DDL planning stops dying with a silent segfault. Documents the shipped behaviour in a sequences guide.
EnRaiha
force-pushed
the
fix/issue294-seq-defaults
branch
from
September 9, 2026 21:13
6a46e9a to
2222d2a
Compare
An insert whose DEFAULT names a missing sequence used to surface as a
generic plan error (42601); the registry already carries the missing
name. A sequence lookup that misses now becomes a typed UndefinedSequence
error rendered as SQLSTATE 42704 (undefined_object), matching
PostgreSQL's class for nextval('missing'). Strict and kv converter paths
both discriminate the registry miss; all other registry failures keep the
plan-error class.
…ng them Outer projections and aggregate/group-key arguments over a constant derived table used to fold silently: the CTE body materialized as bare value rows, the response shaper found no computed alias and emitted NULL, and aggregates over the body scanned an empty collection. Computed projection expressions now ride on the materialized-row scans as computed columns and evaluate per row (division raises 22012, sequence accessors raise 0A000); aggregates whose input is a non-Scan body lower it to a ProviderScan sub-plan so the accumulator receives the rows and evaluates its arguments and group keys against them. Window functions over a constant derived table are still folded silently; that path needs window evaluation in the row post-processor and is tracked separately.
8 tasks
EnRaiha
marked this pull request as draft
September 10, 2026 00:44
… them per row The derived-table tail now transports window function specs from the outer query all the way to the materialized-row scan, which evaluates each spec per partition after computed columns. Partition, ordering, and argument errors fail the query instead of answering NULL. The subquery fold and validation walks now cover window expressions, so catalog casts inside window clauses fold and validate like every other expression on the wrapper.
The copy orchestrators scanned every source with the document materialize scan, which reads the sparse store. A KV source has no rows there, so a non-empty kv source silently copied zero rows and reported INSERT 0 0. resolve_copy_spec now resolves the source collection's engine, the orchestrators route each page through the engine-appropriate scan, and the KV materialize scan folds the transaction overlay the same way the document scan does, so in-transaction copies read their own writes. Engines with no copy scan yet (columnar) refuse loudly.
inline_cte merges an outer SELECT's constraints onto a derived body when that body is itself a Scan, and it copied the BODY's (empty) window list in place of the OUTER's — so SUM/row_number/rank over FROM (SELECT * FROM c) silently answered NULL on every row. The outer window specs now run after the body's own, matching the post-processor branch, and two positive-value tests lock in the carriage and the results.
EnRaiha
marked this pull request as ready for review
September 10, 2026 06:24
Add missing and fields to three ProviderScan constructors in nodedb-cluster-tests: - shuffle_aggregate_cross_node.rs - shuffle_consume_cross_node.rs - shuffle_produce_cross_node.rs These fixtures were missing the fields added for window-over-derived support in NodeDB-Lab#303.
Member
|
Superseded by #316, which landed the sequence-accessor and DEFAULT work on #316 covers, from this PR's scope:
It also closes #298 and #313, and adds the volatility model that keeps a non-deterministic DEFAULT from freezing into a cached plan. Two parts of this PR are not covered by #316 and stay open as their own issues:
Closing to clear the conflicting branch, not to discard those. Thanks for the work here — the split you documented on #294 mapped the surface accurately, and the DEFAULT classification in particular matched where the defect actually was. |
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.
Scope
Sequence accessors (
nextval/currval/setval) end to end:Per-row DEFAULTs on every engine —
DEFAULT nextval('s')advances the control-plane registry once per inserted row on kv, columnar, document and strict collections; malformed orcurrval/setvalDEFAULTs are rejected loudly instead of silently NULLing.Loud classification everywhere else — an accessor in any row-scope context (SELECT list, WHERE, ORDER BY, GROUP BY, HAVING, UPDATE SET, JOIN ON, INSERT..SELECT) raises SQLSTATE 0A000 with a self-describing message; the registry entries, scalar-evaluator guard, exhaustive const-fold classification and end-to-end error plumbing (error code, msgpack tag, envelope + data-plane wire, pgwire/gateway maps) carry the typed code to the client.
Real evaluation in constant contexts — FROM-less
SELECT nextval('s')and explicitVALUEScells evaluate through the registry at plan time, in expression order;setvalsets and returns; a missing sequence raises a plan error naming it. Folded accessor plans are never admitted to the physical-plan cache, andEXPLAINdoes not advance the registry.kv SELECT projections evaluate — kv scans carry the SELECT projection list and computed columns like the other engines, so scalar expressions evaluate per row (
SELECT 1 + 1 FROM kvno longer returns an empty column); plain projections stay full-row so clone-source merges keep the keys tombstone suppression needs.Typed error lanes — executor paths that collapsed evaluation errors to division-by-zero or internal error now discriminate variants, so division keeps 22012 and accessors surface 0A000 through every scan/aggregate/join/routing path.
Runtime headroom — tokio worker stack raised to 16 MiB; deep DDL planning no longer dies with a silent SIGSEGV.
Typed unknown-sequence DEFAULTs — a DEFAULT naming a missing sequence
raises SQLSTATE 42704 (undefined_object), matching PostgreSQL; other
registry failures keep the plan-error class.
Window functions over derived tables —
SUM(n) OVER (...),row_number() OVER (...), and every other window verb over aFROM (SELECT ...)body evaluate per row through the materialized-rowscan instead of silently answering NULL (issue Expression errors fold silently over constant derived tables — 22012 raised on collection scans but not on FROM (SELECT ...) #295 window form). Window
specs survive CTE inlining for Scan-bodied derived tables, and subquery
fold + validation walk window expressions so catalog casts inside
PARTITION BY / ORDER BY fold and validate.
kv INSERT..SELECT sources —
INSERT INTO <target> SELECT ... FROM <kv source>copies rows through the KV materialize scan instead ofsilently reporting
INSERT 0 0(issue INSERT..SELECT with a kv-engine source silently NULLs expression cells #311). The copy orchestratorroutes the source scan by engine, in-transaction copies read their own
writes, and columnar sources refuse loudly until a columnar copy scan
exists.
Rebased on current main (includes the primary-key NOT NULL / row-identity
change #310): DEFAULT expansion runs before identity resolution, so a
sequence-defaulted primary key is filled first and the NOT NULL
enforcement only rejects keys that are still NULL after defaults.
Tests
Wire suites: per-engine DEFAULT, expression-context (FROM-less, SELECT
list, WHERE, ORDER BY, VALUES, INSERT..SELECT, derived-table constants),
constant-context (advance, currval, setval, EXPLAIN, missing-sequence
errors), kv projection, a 60-case engine x accessor x context matrix
asserting loud 0A000, window-over-derived positive-value (SUM,
row_number) and division-by-zero, and kv INSERT..SELECT (autocommit and
in-transaction staged). Verified against the #310 suites on the merged
tree: sql_primary_key_nullability, sql_null_predicate_parity,
write_verdict_sqlstate, plus the division-by-zero and clone regressions.
Known boundaries
Row-scope sequence accessors stay loud by design until per-row evaluation lands (future work). The remaining #295 forms —
VALUES-derived and UNION-derived bodies plus the outer-expression case —
are tracked separately. index-DDL stack overflow (#312) is fixed here.