Skip to content

feat(sql): evaluate sequence accessors in constant contexts and kv projections - #307

Closed
EnRaiha wants to merge 15 commits into
NodeDB-Lab:mainfrom
EnRaiha:fix/issue294-select-seq
Closed

feat(sql): evaluate sequence accessors in constant contexts and kv projections#307
EnRaiha wants to merge 15 commits into
NodeDB-Lab:mainfrom
EnRaiha:fix/issue294-select-seq

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Two gaps kept sequence accessors and scalar expressions out of SELECT
evaluation:

  1. kv scans dropped SELECT projectionsKvOp::Scan carried neither
    the 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 accessors in a kv SELECT list could not raise their typed error.
  2. Constant contexts had no registry accessSELECT nextval('s')
    without a FROM clause and explicit VALUES (nextval('s')) cells fold
    at plan time inside nodedb-sql, which has no sequence registry; they
    raised the 0A000 reserved for row-scope escapes instead of evaluating.

Fix

  • KvOp::Scan gains projection and computed_columns (same wire format
    as DocumentOp::Scan / ColumnarOp::Scan); the scan converter forwards
    them 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).
  • Constant folding in nodedb-sql gains a nesting-safe, thread-local
    sequence-evaluator hook (default absent: behaviour unchanged). The
    control plane installs it around each planning call over its sequence
    registry, so nextval/currval/setval with fully folded arguments
    evaluate exactly once, in expression order, and registry errors surface
    as plan errors naming the sequence (same class as the DEFAULT path).
  • Row-scope contexts over a table (WHERE, ORDER BY, SELECT list over
    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 projections
unchanged. Constant contexts: FROM-less nextval advances and currval
reads back in-statement; consecutive statements continue the sequence;
VALUES cells advance per row; setval returns its value; missing
sequence 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 (DataDependent
eligibility). The parameterized path does not admit plans to the cache at
all.

Semantics boundaries

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
Copilot AI lite review requested due to automatic review settings September 8, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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
@EnRaiha EnRaiha changed the title fix(sql): evaluate SELECT projection expressions on kv scans feat(sql): evaluate sequence accessors in constant contexts and kv projections Sep 8, 2026
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
@EnRaiha EnRaiha added the run-ci Opt this PR into the full test suite; re-add to force a re-run label Sep 8, 2026
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.
@EnRaiha EnRaiha added area:sql Parser, planner, SQL semantics engine:kv Key-Value engine type:feature New capability or behavior change labels Sep 9, 2026
@EnRaiha

EnRaiha commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #303 — the change set is merged into it as a single commit.

@EnRaiha EnRaiha closed this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:sql Parser, planner, SQL semantics engine:kv Key-Value engine run-ci Opt this PR into the full test suite; re-add to force a re-run type:feature New capability or behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants