Skip to content

fix: nine-issue batch — sequences, kv scans, windows, graph DSL, and one instant conversion - #324

Closed
EnRaiha wants to merge 14 commits into
mainfrom
pr/open-issues-20260914
Closed

EnRaiha wants to merge 14 commits into
mainfrom
pr/open-issues-20260914

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes nine open issues (#295, #296, #297, #305, #306, #311, #314, #317,
#318) across SQL planning and execution, the kv engine, the graph DSL,
sequences, and the timeseries read path. 26 commits over 124cc53a6;
verified at fdba3e378. A full local CI-parity run over this exact tree is
green (see Validation).

Also fixes two silent engine defects on the same paths:

  • kv scan projectionsSELECT upper(v) FROM kv answered NULL: KvOp::Scan carries the projection list and computed columns and evaluates them per row.
  • materialized-sum pages — a multi-row INSERT into a source whose balance target is cross-shard timed out waiting for Calvin completion: such a page is un-batched to the per-row PointInsert shape (which settles correctly); purely co-resident pages keep their single page.

Behaviour changes

Path Before After
SELECT, RETURNING, native stream, NDJSON body with nextval(...) NULL one allocated value per output row
Window over a derived table NULL cell evaluated over the materialized rows, in the window's own order
Timeseries read after a join unit depended on the path milliseconds throughout, converted once at the boundary
INSERT with more than one row one tag per row one INSERT 0 n
currval(...) after a stamped nextval(...) not yet defined the last allocated value
CONVERT without the source key column minted new row identities refused with 42601

Compatibility

Pre-1.0, no migration:

  • The response shaper gained a sequence-stamper parameter and every response path threads the session's sequence values. Internal to the crate.
  • A CONVERT that omits the source primary key fails instead of minting identities.
  • A multi-row INSERT into a materialized-sum source with a cross-shard target plans as per-row writes (the page shape is reserved for co-resident targets). Internal to the planner.

Validation

Check Result
cargo fmt --all -- --check clean
cargo clippy --workspace --all-targets --all-features --profile ci -- -D warnings clean
static gates (scripts/ci/*) 11/11 pass
cargo deny check advisories, bans, licenses, sources all ok (lockfile bumps rustls to 0.23.45 for RUSTSEC-2026-0285)
cargo nextest run --workspace --all-features (fast suite) 16,169 passed, 0 failed (5 flaky passed on retry, 3 leaky)
cargo nextest run -p nodedb-cluster-tests --all-features 363 passed, 4 skipped (the CI-profile exclusions)
cargo check --target wasm32-unknown-unknown trio clean
fuzz targets (scripts/ci/*) 6/6 clean

Local parity reaches everything except CodeQL, release packaging, and secret
scanning, which need GitHub-hosted runners; those report as not run until the
PR CI does them.

Note on the test environment

nextest needs RUST_MIN_STACK=33554432, the value
.github/workflows/test.yml sets. On the 2 MiB default, two kv surrogate
tests (kv_incr_on_fresh_key_persists_a_real_surrogate,
kv_transfer_persists_two_distinct_surrogates) abort with a
tokio-rt-worker stack overflow — on clean main as well, so that gap
predates this branch. Four cluster tests excluded by .config/nextest.toml
from the CI profile (CI-only Calvin-completion hangs) remain excluded here.

Closes #295, closes #296, closes #297, closes #305, closes #306, closes #311, closes #314, closes #317, closes #318

The data-group replay gate aborted after a hardcoded 60s. A restart whose
last log entry is an election no-op can sit one index short of
commit_index for that whole budget, then fail startup.

- add [server] data_group_recovery_timeout_ms, default 60000
- pass it through await_cluster_ready into await_data_group_recovery
Each value tuple became its own PointInsert task. One statement emitted
INSERT 0 1 per row, and drivers read rowcount 1.

- accumulate plain rows in insert/convert.rs and emit one BatchInsert
- keep per-row tasks for ON CONFLICT DO NOTHING and CRDT rows
kv writes reported no affected count, so pgwire emitted a bare OK.

- report affected = 1 in execute_kv_insert and execute_kv_put
- map KvOp::Insert, KvOp::Put, KvOp::BatchPut to the INSERT tag
ARRAY[0.1, 0.2, 0.3] folds its literals to Decimal. extract_vector_floats
kept Float and Integer, dropped the rest, and reported "got 0 elements"
as XX000.

- accept Float, Integer, Decimal, and numeric String elements
- reject a non-numeric element by index and type
- refuse non-finite values
The binary_tuple wrap sites turned every non-UnknownStrictField error into
a Serialization error. A BadRequest surfaced as XX000 with no SQLSTATE.

- preserve BadRequest at the four wrap sites
The document materializer scanned every source. A kv source read an empty
document store, so INSERT ... SELECT copied no rows and reported
INSERT 0 0.

- route on the catalog engine in clone_materializer/auto_source.rs
- kv normalizes the materialize-scan page to the copy entry shape
- refuse columnar, timeseries, spatial, array by name
Derived-relation inference listed declared columns only. The response
layer appends a synthetic distance cell, so s.distance failed 42703 on a
closed schema.

- append distance when ORDER BY carries vector_distance(...)
KvOp::Scan carried no projection list and no computed columns. Every
expression projection over a kv source shaped to NULL.

- add projection and computed_columns to KvOp::Scan
- evaluate both in the kv scan handler; keep plain projections full-row
- update all KvOp::Scan constructors
- add the kv_select_expressions wire case
SELECT x/0 FROM (SELECT 1 AS x) s answered one NULL row. The same
expression over a collection raised 22012.

- evaluate computed columns per row on the materialized-row scan
- lower aggregates over a non-Scan body to a ProviderScan sub-plan
- carry window specs through the post-processor
- add the derived_expression_errors wire case
The projection list named an unaliased expression by its lowercased SQL
text; the window spec used the verbatim text. The alias match never fired
and the projection shaped to NULL.

- add unaliased_projection_alias and use it at every naming site
Partitions were consumed in arrival order. row_number numbered by
arrival and the rank family compared wrong neighbours whenever the window
ORDER BY differed from the query's.

- sort each partition by the spec's order keys
- evaluate keys on singleton partitions so ordering errors still fail
CREATE TYPEGUARD answered 42P01 on an existing collection. The handlers
read DatabaseId::DEFAULT while collections live under the session
database.

- thread database_id through the seven handlers and validate_typeguard
CONVERT rebuilt the strict schema from the column list and dropped the
source key. Every later insert failed "no resolved primary key".

- mark the source key column in the converted schema
- refuse a column list that omits it with 42601
A guard DEFAULT or VALUE naming another column became a strict column
DEFAULT. That DEFAULT evaluates with no row in scope, so every insert
raised UnevaluableDefault.

- add default_expr_references_columns in nodedb-sql
- refuse the guard at CONVERT, naming field and clause
@EnRaiha

EnRaiha commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Superseded: settling locally before opening for review.

@EnRaiha EnRaiha closed this Sep 14, 2026
@EnRaiha
EnRaiha deleted the pr/open-issues-20260914 branch September 14, 2026 03:58
@EnRaiha EnRaiha changed the title fix: resolve issues #295, #297, #305, #306, #311, #318 — SQL, kv, and strict-format correctness fix: nine-issue batch — sequences, kv scans, windows, graph DSL, and one instant conversion Sep 15, 2026
@EnRaiha

EnRaiha commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #325, which carries the settled branch at fdba3e3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment