oli/rdbms-playground
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 2 Wiki Activity

INSERT ... VALUES (...) without a column list: simple/advanced behaviour inconsistent #1

New Issue
Closed
opened 2026-05-28 14:43:34 +01:00 by oliversturm · 2 comments
oliversturm commented 2026-05-28 14:43:34 +01:00 (Migrated from github.com)
Copy Link
Copy Source

Schema for repro: Customers(id serial PRIMARY KEY, Name text, Age int, SerNo serial)

Observed

Mode Line Result
Simple insert into Customers values('Oli',52,3) Rejected. Error appends "(valid as SQL in advanced mode — mode advanced or prefix :)"
Advanced same line Rejected by the engine: missing value for id
Either insert into Customers values (13, 'Oli', 42, 13) (both serials supplied) Succeeds

So the user is told the line will work in advanced mode, switches, and the line still fails.

Why this happens today

  • Simple-mode positional Form B treats serial columns as auto-generated and excludes them from the value count — both PK and non-PK serials must be omitted. ADR-0033 §6 and src/dsl/walker/mod.rs:4127. Supplying a value triggers the count mismatch + the "valid as SQL in advanced mode" hint at src/friendly/strings/en-US.yaml:709 (added per ADR-0033 Amendment 3).
  • Advanced-mode VALUES-only form has no auto-fill. The X4 fix (commit 42306d3, "X4 — advanced-mode SQL INSERT auto-fills omitted non-PK serial (MAX+1)") only covers Form A (column list present); the column-list-less variant goes straight to the engine, which then demands all four values. See plan_autogen_autofill in src/db.rs:8440.

Spec decisions needed

  1. Should simple-mode positional Form B accept user-supplied serial values (override path) or continue to require omission? Today the user is forbidden from setting them even when they want to.
  2. Should advanced-mode VALUES-only form gain the same auto-fill as Form A, so that omitting trailing serial values "just works" the same way?
  3. The "valid as SQL in advanced mode" hint is misleading whenever the line actually still fails in advanced. Should the hint be gated on executability, or removed for this class of error?

Workaround

Supply every column including serials: insert into Customers values (13, 'Oli', 42, 13).

**Schema for repro:** `Customers(id serial PRIMARY KEY, Name text, Age int, SerNo serial)` ### Observed | Mode | Line | Result | |------|------|--------| | Simple | `insert into Customers values('Oli',52,3)` | Rejected. Error appends *"(valid as SQL in advanced mode — `mode advanced` or prefix `:`)"* | | Advanced | same line | Rejected by the engine: missing value for `id` | | Either | `insert into Customers values (13, 'Oli', 42, 13)` (both serials supplied) | Succeeds | So the user is told the line will work in advanced mode, switches, and the line still fails. ### Why this happens today - **Simple-mode positional Form B** treats serial columns as auto-generated and excludes them from the value count — both PK and non-PK serials must be omitted. ADR-0033 §6 and `src/dsl/walker/mod.rs:4127`. Supplying a value triggers the count mismatch + the "valid as SQL in advanced mode" hint at `src/friendly/strings/en-US.yaml:709` (added per ADR-0033 Amendment 3). - **Advanced-mode VALUES-only form** has no auto-fill. The X4 fix (commit `42306d3`, *"X4 — advanced-mode SQL INSERT auto-fills omitted non-PK serial (MAX+1)"*) only covers **Form A** (column list present); the column-list-less variant goes straight to the engine, which then demands all four values. See `plan_autogen_autofill` in `src/db.rs:8440`. ### Spec decisions needed 1. Should simple-mode positional Form B accept user-supplied serial values (override path) or continue to require omission? Today the user is forbidden from setting them even when they want to. 2. Should advanced-mode VALUES-only form gain the same auto-fill as Form A, so that omitting trailing serial values "just works" the same way? 3. The *"valid as SQL in advanced mode"* hint is misleading whenever the line actually still fails in advanced. Should the hint be gated on executability, or removed for this class of error? ### Workaround Supply every column including serials: `insert into Customers values (13, 'Oli', 42, 13)`.
oliversturm commented 2026-05-28 15:21:21 +01:00 (Migrated from github.com)
Copy Link
Copy Source

Resolution (2026-05-28 discussion)

Q1 — simple Form B serial override: keep current restriction. Form A (column list) is the documented override path; preserves the contract that serials are auto-generated.

Q2 — advanced Form B auto-fill: do not add. Advanced mode teaches standard SQL semantics — positional VALUES (...) without a column list requires every column.

Q3 — misleading hint: narrow and reword.

  • Append the hint only when the rejection class is "SQL-only syntax used in simple mode" — not for contract / value-count mismatches on a form that exists in both modes.
  • Reword along the lines of "Trying to write SQL? Switch to advanced mode (mode advanced), or prefix the line with : to run once." Existing string at src/friendly/strings/en-US.yaml:709.

Diagnostic work — the load-bearing piece

The surprise the user hits in both modes is not the restriction itself but the unhelpful error text. PK-serial vs non-PK-serial is not surfaced in user-facing messages — that distinction is an engine implementation detail.

Simple mode — extra values supplied for auto-generated columns. Current error is roughly "Expected 2 values, got 3 (valid as SQL in advanced mode …)". Target:

Expected 2 values for Name, Age — id and SerNo are auto-generated and filled automatically. To set them explicitly, use the column-list form: insert into Customers (id, Name, Age, SerNo) values (...).

Advanced mode — positional INSERT INTO T VALUES (...) missing values for auto-generated columns. Current error is the raw engine NOT-NULL. Target:

Standard SQL INSERT ... VALUES (...) without a column list requires every column. Customers has 4 columns (id, Name, Age, SerNo) — supply all 4, or use the column-list form to omit auto-generated columns: INSERT INTO Customers (Name, Age) VALUES ('Oli', 52).

Wordings above are drafts; final copy gets polished during implementation.

Implementation notes

  • Diagnostic copy lives in src/friendly/strings/en-US.yaml; the simple-mode walker path is around src/dsl/walker/mod.rs:4127 (Form B dispatch) and the advanced-mode INSERT planner is around src/db.rs:8440 (plan_autogen_autofill).
  • The hint-gating change touches the error wrapper that calls also_valid_sql — an error-class signal needs to flow through so only true "SQL-syntax-in-simple-mode" errors get the hint.
  • Likely worth breaking into three sub-tasks once implementation starts (simple-mode diagnostic, advanced-mode diagnostic, hint gate + reword) — each independently testable per the test-first rule.
## Resolution (2026-05-28 discussion) **Q1 — simple Form B serial override:** keep current restriction. Form A (column list) is the documented override path; preserves the contract that serials are auto-generated. **Q2 — advanced Form B auto-fill:** do not add. Advanced mode teaches standard SQL semantics — positional `VALUES (...)` without a column list requires every column. **Q3 — misleading hint:** narrow *and* reword. - Append the hint only when the rejection class is *"SQL-only syntax used in simple mode"* — not for contract / value-count mismatches on a form that exists in both modes. - Reword along the lines of *"Trying to write SQL? Switch to advanced mode (`mode advanced`), or prefix the line with `:` to run once."* Existing string at `src/friendly/strings/en-US.yaml:709`. ### Diagnostic work — the load-bearing piece The surprise the user hits in both modes is not the restriction itself but the unhelpful error text. PK-serial vs non-PK-serial is **not** surfaced in user-facing messages — that distinction is an engine implementation detail. **Simple mode** — extra values supplied for auto-generated columns. Current error is roughly *"Expected 2 values, got 3 (valid as SQL in advanced mode …)"*. Target: > Expected 2 values for `Name`, `Age` — `id` and `SerNo` are auto-generated and filled automatically. To set them explicitly, use the column-list form: `insert into Customers (id, Name, Age, SerNo) values (...)`. **Advanced mode** — positional `INSERT INTO T VALUES (...)` missing values for auto-generated columns. Current error is the raw engine NOT-NULL. Target: > Standard SQL `INSERT ... VALUES (...)` without a column list requires every column. `Customers` has 4 columns (`id`, `Name`, `Age`, `SerNo`) — supply all 4, or use the column-list form to omit auto-generated columns: `INSERT INTO Customers (Name, Age) VALUES ('Oli', 52)`. Wordings above are drafts; final copy gets polished during implementation. ### Implementation notes - Diagnostic copy lives in `src/friendly/strings/en-US.yaml`; the simple-mode walker path is around `src/dsl/walker/mod.rs:4127` (Form B dispatch) and the advanced-mode INSERT planner is around `src/db.rs:8440` (`plan_autogen_autofill`). - The hint-gating change touches the error wrapper that calls `also_valid_sql` — an error-class signal needs to flow through so only true *"SQL-syntax-in-simple-mode"* errors get the hint. - Likely worth breaking into three sub-tasks once implementation starts (simple-mode diagnostic, advanced-mode diagnostic, hint gate + reword) — each independently testable per the test-first rule.
oliversturm commented 2026-05-28 17:42:33 +01:00 (Migrated from github.com)
Copy Link
Copy Source

Resolved in commit c12ed1d.

Three layered fixes plus ADR-0033 Amendment 5:

  1. Walker emits a new diagnostic.insert_arity_mismatch_form_b ERROR for the no-column-list (Form B) case (previously deferred per dml_insert_arity_diagnostics's own doc-comment). The [ERR] validity indicator now lights up at typing time.

  2. advanced_alternative_note now reads input_verdict_in_mode directly — the "would parse in advanced" gate becomes "would be valid in advanced" in the ADR-0027 sense (verdict None). Any future static check added to the verdict pipeline participates automatically.

  3. Teaching notes for the submit path:

    • simple-mode: insert.form_b_extra_values_note (under-, in-window, over-supply)
    • advanced-mode pre-flight: insert.form_b_positional_count_mismatch_note

Pointer wording reworked to "trying to write SQL? switch with mode advanced, or prefix : to run once".

ADR-0033 Amendment 5 documents the gate refinement with an explicit definition of "valid". docs/requirements.md H1a cites the three new pedagogical strings.

Tests added: 8 (4 walker + 4 app). Full suite 2031 passed / 0 failed / 0 unexpected skips. Clippy clean.

Resolved in commit c12ed1d. Three layered fixes plus ADR-0033 Amendment 5: 1. Walker emits a new `diagnostic.insert_arity_mismatch_form_b` ERROR for the no-column-list (Form B) case (previously deferred per `dml_insert_arity_diagnostics`'s own doc-comment). The `[ERR]` validity indicator now lights up at typing time. 2. `advanced_alternative_note` now reads `input_verdict_in_mode` directly — the "would parse in advanced" gate becomes "would be valid in advanced" in the ADR-0027 sense (verdict `None`). Any future static check added to the verdict pipeline participates automatically. 3. Teaching notes for the submit path: - simple-mode: `insert.form_b_extra_values_note` (under-, in-window, over-supply) - advanced-mode pre-flight: `insert.form_b_positional_count_mismatch_note` Pointer wording reworked to *"trying to write SQL? switch with `mode advanced`, or prefix `:` to run once"*. ADR-0033 Amendment 5 documents the gate refinement with an explicit definition of "valid". `docs/requirements.md` H1a cites the three new pedagogical strings. Tests added: 8 (4 walker + 4 app). Full suite 2031 passed / 0 failed / 0 unexpected skips. Clippy clean.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No labels bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
claude-clouddev1 oli (Oli Sturm)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: oli/rdbms-playground#1