feat(sql): evaluate sequence accessors in constant contexts and kv projections - #307
Closed
EnRaiha wants to merge 15 commits into
Closed
feat(sql): evaluate sequence accessors in constant contexts and kv projections#307EnRaiha wants to merge 15 commits into
EnRaiha wants to merge 15 commits into
Conversation
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.
…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.
…ce accessors inside the DEFAULT plane (static contract gate, exit 0 verified), plus a parser corpus locking sequence_accessor()'s tolerant, byte-safe recognition semantics: case, quotes, whitespace, unicode, casts, non-call shapes, and two-arg tolerance that resolves loud later.
KvOp::Scan carried neither the SELECT projection list nor computed columns, so expression projections over kv collections were never evaluated: the column surfaced as NULL at response shaping (SELECT 1 + 1 FROM kv returned an empty column), and sequence accessors could not raise their typed 0A000 from that path either. - nodedb-physical: KvOp::Scan gains projection + computed_columns fields (same wire format as DocumentOp::Scan / ColumnarOp::Scan). - sql_plan_convert scan/core.rs: the kv branch now forwards the SELECT projection list and serialized computed columns. - kv scan handler: decodes computed columns and applies projection + computed per row after sorting, via the same evaluator as the document scan path; eval errors surface as their typed code (accessors -> 0A000). - Constructor sites (clone rewrite, RESP/native builders, weighted pick, full exchange scan) updated to pass the new fields. - Wire coverage (3 cases): scalar expressions evaluate per row over kv (upper, 1+1), nextval/currval in a kv SELECT list raise 0A000, plain stored-column projections are unchanged. issue NodeDB-Lab#294
SELECT nextval('s') without a FROM clause and explicit VALUES cells fold
at plan time inside nodedb-sql, which has no sequence registry; they
previously raised the loud 0A000 reserved for row-scope escapes. The
control plane now installs a registry hook for the duration of one
planning call, so each constant accessor evaluates exactly once, in
expression order:
- nodedb-sql const-fold gains a thread-local, nesting-safe sequence
evaluator hook (default absent: behaviour unchanged); registered
accessors with fully folded arguments consult it before classification.
- QueryContext planning (plain and parameterized paths) installs a guard
over its Arc<SequenceRegistry>, translating folded Value arguments to
nextval/currval/setval calls and errors back to plan errors.
- Wire coverage: FROM-less nextval advances and currval reads back in the
same statement; consecutive statements continue the sequence; VALUES
cells advance per row; setval returns its value; a missing sequence
raises a plan error naming it (same class as the DEFAULT path).
- Row-scope contexts over a table (WHERE, ORDER BY, SELECT list) keep
raising 0A000; expression-context suite updated: constant-context cases
now expect the registry-miss plan error instead of 0A000.
issue NodeDB-Lab#294
ConstantResult and Insert plans are cache-eligible, so a plan that folded nextval/currval/setval at plan time would be admitted to the physical-plan cache and replay the same frozen literal on every subsequent execution. - The const-fold hook now records whether it produced a value (shared flag between the TLS slot and the guard); planning forces the eligibility to DataDependent when the hook ran, so accessor statements re-plan and keep advancing. - Unit coverage: hook folds a value and marks the guard used; dropping the guard restores the loud 0A000 classification; hook errors surface as Unsupported with the sequence name. - Wire coverage: two accessors in one statement advance in expression order (1, 2); nextval continues after setval (42). issue NodeDB-Lab#294
Combinatorial wire suite (engine x accessor x expression context) exposed three remaining lanes that collapsed evaluation errors to the wrong class: - streaming aggregate over docs wrapped ANY error as Internal (XX000): HAVING/group-key accessors now keep their typed code (0A000), division keeps 22012. - seven pgwire routing sites hardcoded XX000 for task errors; they now map through the numeric code table, so typed codes survive to the client. - columnar-read scan filter collapsed EvalError to 22012; it now discriminates like the other scan paths (accessors -> 0A000). - gateway and pgwire error maps gained the FeatureNotSupported arms for typed and RemoteTyped errors (division-by-zero typed lane fixed as well). Adds sequence_matrix wire suite: 60 row-scope combinations (4 engines x 3 accessors x SELECT list/WHERE/ORDER BY/GROUP BY/HAVING) assert loud 0A000; JOIN ON and INSERT..SELECT contexts assert 0A000; constant contexts evaluate per engine (VALUES advance per row). Known follow-up (pre-existing, unrelated to accessors): INSERT..SELECT with a kv-engine source drops expression projections in the copy_rows column-map (silent NULL); documented in-test. issue NodeDB-Lab#294
- PlanningPurpose::Metadata (EXPLAIN, catalog inspection) installs a dummy constant-eval hook: the accessor folds to a placeholder literal and the registry is never advanced, matching PostgreSQL (EXPLAIN plans without executing). - Wire coverage: UPDATE SET and ON CONFLICT DO UPDATE with an accessor raise 0A000 (row-scope stays loud, no once-folded value silently applied to every row); EXPLAIN SELECT nextval(...) succeeds and the following SELECT still returns 1. issue NodeDB-Lab#294
- inproc/test initializers (test_kv, test_kv_advanced, test_kv_scan_budget, txn_buffering classify test) now pass the new projection + computed columns fields (workspace all-targets all-features compiles clean). - restore duplicated #[test] / missing attribute in const_fold tests (CI -D warnings). - drop needless borrow across the pgwire routing error-map sites (clippy -D warnings).
Documents the shipped behaviour of sequence accessors: CREATE/DROP/SHOW SEQUENCE, per-row DEFAULT nextval on every engine, real evaluation in constant contexts (FROM-less SELECT, VALUES, setval), EXPLAIN side-effect-free planning, non-cacheable folded plans, loud 0A000 for row-scope contexts, and the error-class table. Cross-listed from the docs README.
…ocal tgrep index The executor-side projection stripped rows to the SELECT list before the control plane merged clone-source reads, removing the primary key the tombstone suppression needs (clone_write_suppresses_source_row). Project only when computed columns exist; plain projections stay full-row like before the kv change. Also add .tgrep/ to .gitignore (local search index).
) The worker threads carry the whole planning -> dispatch -> execution pipeline synchronously; the DDL/index path nests deep enough to exhaust tokio's 2 MiB default and the process died with a silent SIGSEGV (CREATE VECTOR/SORTED INDEX dropped the connection; inproc tests aborted with 'tokio-rt-worker has overflowed its stack'). 16 MiB restores headroom; tokio reserves the space virtually and commits pages on demand. Verified: index DDL and the previously-aborting suites pass without RUST_MIN_STACK.
8 tasks
Contributor
Author
|
Superseded by #303 — the change set is merged into it as a single commit. |
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.
Problem
Two gaps kept sequence accessors and scalar expressions out of SELECT
evaluation:
KvOp::Scancarried neitherthe projection list nor computed columns, so expression projections
over kv collections were never evaluated: the column surfaced as NULL
at response shaping (
SELECT 1 + 1 FROM kvreturned an empty column),and accessors in a kv SELECT list could not raise their typed error.
SELECT nextval('s')without a FROM clause and explicit
VALUES (nextval('s'))cells foldat plan time inside
nodedb-sql, which has no sequence registry; theyraised the 0A000 reserved for row-scope escapes instead of evaluating.
Fix
KvOp::Scangainsprojectionandcomputed_columns(same wire formatas
DocumentOp::Scan/ColumnarOp::Scan); the scan converter forwardsthem and the kv scan handler applies projection + computed per row after
sorting, through the evaluator the document path uses. Evaluation errors
surface with their typed code (division ->
22012, accessors ->0A000).nodedb-sqlgains a nesting-safe, thread-localsequence-evaluator hook (default absent: behaviour unchanged). The
control plane installs it around each planning call over its sequence
registry, so
nextval/currval/setvalwith fully folded argumentsevaluate exactly once, in expression order, and registry errors surface
as plan errors naming the sequence (same class as the DEFAULT path).
rows) keep raising 0A000.
Tests
Wire coverage: kv scalar projections evaluate per row (
upper(v),1 + 1); accessors in a kv SELECT list raise 0A000; plain projectionsunchanged. Constant contexts: FROM-less
nextvaladvances andcurrvalreads back in-statement; consecutive statements continue the sequence;
VALUES cells advance per row;
setvalreturns its value; missingsequence raises a plan error naming it. All prior sequence suites stay
green (DEFAULT across engines, expression-context classification).
Part 2 of issue #294 (see the issue for the split). The branch history
includes Part 1 commits until the parts merge; the final diff against main
will contain this change alone.
issue #294 (Part 2), #295
Cache safety
A plan that folded a stateful accessor at plan time must not be admitted
to the physical-plan cache: the folded literal would replay the same value
forever. The fold hook records whether it produced a value, and planning
forces such statements to re-plan on every execution (
DataDependenteligibility). The parameterized path does not admit plans to the cache at
all.
Semantics boundaries
UPDATE SET,ON CONFLICT DO UPDATE,DELETEWHERE,
RETURNING, window functions) keep raising0A000— noonce-folded value is silently applied per row. Per-row evaluation for
SELECT-list projections over tables is a follow-up.
EXPLAINplans without advancing the registry (metadata-purpose planningfolds the accessor to a placeholder), matching PostgreSQL.
INSERT..SELECTwith akv-engine source drops expression projections in the copy_rows
column-map (silent NULL) — tracked in INSERT..SELECT with a kv-engine source silently NULLs expression cells #311. Per-row SELECT-list
evaluation is tracked in Evaluate sequence accessors per row in SELECT-list contexts (0A000 today) #314; the 42704 promotion for unknown-sequence
DEFAULTs in Unknown sequence in DEFAULT raises 42601 instead of 42704 #313.