fix(sql): evaluate sequence-backed DEFAULT expressions on typed engines - #303
Open
EnRaiha wants to merge 1 commit into
Open
fix(sql): evaluate sequence-backed DEFAULT expressions on typed engines#303EnRaiha wants to merge 1 commit into
EnRaiha wants to merge 1 commit into
Conversation
`DEFAULT nextval('name')` passed DDL and then silently produced NULL on
every insert: the pure default evaluator does not recognize sequence
accessors, so the expression evaluated to "no value" and the column was
omitted — including primary keys (NodeDB-Lab#294). The sequence machinery itself
exists on the control plane (SequenceRegistry with catalog persistence and
range allocation); this wires the SQL DEFAULT path to it.
- SequenceRegistry is threaded from SharedState through QueryContext into
ConvertContext (for_state / for_state_with_lease and both planning
sites); sub-planner contexts keep None.
- evaluate_sequence_default recognizes DEFAULT nextval('name'), advances
the CP-side registry, and raises a loud PlanError for unknown sequences —
a DDL-accepted DEFAULT never silently vanishes into NULL again.
- The INSERT/UPSERT expanded_rows loop prefers the sequence-aware evaluator
over the pure stateless one.
Verified: document_strict DEFAULT nextval fills distinct ids (1,2,3) across
inserts; unknown sequence raises naming the sequence; uuid defaults
unchanged. Out of scope here (tracked): SELECT nextval/currval evaluation,
kv/columnar-family defaults, schemaless document column defaults (dropped
at catalog adapter — pre-existing), currval/setval session semantics.
EnRaiha
added
sev:2-high
8 tasks
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.
fix(sql): evaluate sequence-backed DEFAULT expressions on typed engines
Partially addresses #294.
Problem
DEFAULT nextval('name')passed DDL validation and then silently produced NULL on every insert: the pure default evaluator does not recognize sequence accessors, so the expression evaluated to "no value" and the column was omitted — including primary keys. The sequence machinery itself exists on the control plane (SequenceRegistry, catalog persistence, range allocation); the SQL DEFAULT path was never wired to it.Fix
SequenceRegistrythreaded fromSharedStatethroughQueryContextintoConvertContext(for_state/for_state_with_leaseand both planning construction sites); sub-planner contexts keepNone.evaluate_sequence_default: recognizesDEFAULT nextval('name'), advances the CP-side registry, and raises a loudPlanErrornaming the sequence when it does not exist — a DDL-accepted DEFAULT never silently vanishes into NULL again.expanded_rowsloop prefers the sequence-aware evaluator over the pure stateless one.Verified
document_strict DEFAULT nextval('seq'), 3 insertsDEFAULT nextval('no_such_seq')insertuuid_v7()default on strictsequence_default_typed-D warnings/ fmtOut of scope (tracked separately)
SELECT nextval/currval/setvalevaluation — still 42883 (unchanged). The accessor registry registration ships together with the SELECT-side fold so the gate never passes a call that folds to NULL.type_convert.rshardcodesdefault: None) — pre-existing, affects UUID defaults too; separate fix.currval/setvalsession semantics.How to test