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

Phase D: insert value list mirrors do_insert's user_cols contract

Browse Source 
Bug: hint at \`insert into Customers values (\` for a Customers
table with id:serial PK suggested typing an integer for \`id\`,
but the dispatch path (\`db::do_insert\`) deliberately doesn't
accept user-supplied values for auto-generated columns in
Form B. The grammar prompted for a value the dispatch would
refuse.

The fix aligns Phase D's \`column_value_list\` dynamic sub-grammar
with do_insert's three forms (ADR-0014 + ADR-0018 §3):

- **Form A** \`insert into <T> (col1, col2, …) values (…)\` —
  user explicitly lists columns. Slot list mirrors that
  selection; serial / shortid columns CAN appear if the user
  lists them.
- **Form B** \`insert into <T> values (…)\` — bare values. Slot
  list = non-auto-generated columns of the table in
  declaration order. Serial / shortid get auto-filled by the
  dispatch; the grammar doesn't prompt for them.
- **Form C** \`insert into <T> (v1, v2, …)\` — bare value list.
  Not affected by this change (column_value_list isn't on this
  path; Form C's literals route through the schemaless
  INSERT_PAREN_LIST).

Implementation:

\`WalkContext.user_listed_columns: Option<Vec<String>>\` — when
\`Some\`, signals Form A; \`None\` is Form B. Populated by walking
the first paren's column-list idents.

\`Node::Ident.writes_user_listed_column: bool\` — new field;
\`true\` on the INSERT_PAREN_ITEM's Ident child. When the
walker matches that ident in Form A, it appends the
schema-canonical column name (case-corrected against the
schema) to user_listed_columns.

\`column_value_list\` factory:
- If user_listed_columns is Some → resolve each name from the
  schema; one typed slot per listed column.
- Else → filter current_table_columns to non-auto-generated;
  one typed slot per remaining column.
- Empty result → fall back to the schemaless value-literal
  list (a serial-only table in Form B has nothing for the
  user to type).

Tests:
- New \`phase_d_insert_form_b_skips_serial_column\` confirms the
  bug: \`insert into Customers values (1, 'Alice')\` against a
  Customers with serial id rejects at parse time (Form B
  expects 1 value for Name, not 2).
- New \`phase_d_insert_form_a_accepts_serial_when_listed\`
  confirms \`insert into Customers (id, Name) values (1, 'Alice')\`
  works.
- New \`phase_d_insert_form_a_filters_to_user_listed_columns\`
  confirms partial Form A (\`(Name) values ('Alice')\`).
- Updated \`phase_d_insert_with_schema_accepts_typed_values_per_column\`
  to match the new Form B contract (2 user-typed values, not 3).
- Updated typed-hint test matrix split into form-B (8 types)
  and form-A (serial / shortid).
- New \`typed_hint_form_b_skips_serial_column_to_generic_or_text_neighbor\`
  pins the fallback behavior for a serial-only table.

For the user: \`insert into Customers values (\` for a Customers
with \`(id:serial, Name:text, Email:text)\` now hints
\`for \`Name\`: Type a quoted string …\` (skipping id entirely)
and accepts exactly 2 values. To set the serial explicitly,
use Form A: \`insert into Customers (id, Name, Email) values
(1, 'Alice', 'a@b.c')\`.

Tests: 851 passing, 0 failing, 1 ignored. Clippy clean.
This commit is contained in:
claude@clouddev1
2026-05-15 18:45:47 +00:00
parent c485189da8
commit b3f1a20652
9 changed files with 249 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/dsl/grammar/app.rs
+3
View File
@@ -52,6 +52,7 @@ const IMPORT_AS_TARGET: Node = Node::Seq(&[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
]);
const IMPORT_AS_TARGET_OPT: Node = Node::Optional(&IMPORT_AS_TARGET);
@@ -76,6 +77,7 @@ const MODE_CHOICES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const MODE_VALUE: Node = Node::Choice(MODE_CHOICES);
@@ -90,6 +92,7 @@ const MESSAGES_CHOICES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const MESSAGES_VALUE: Node = Node::Choice(MESSAGES_CHOICES);
src/dsl/grammar/data.rs
+13
View File
@@ -35,6 +35,7 @@ const TABLE_NAME_EXISTING: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
/// Table-name slot variant that populates
@@ -48,6 +49,7 @@ const TABLE_NAME_INSERT: Node = Node::Ident {
highlight_override: None,
writes_table: true,
writes_column: false,
writes_user_listed_column: false,
};
// `value_literal` — null / true / false / number / string. The
@@ -108,6 +110,14 @@ const INSERT_PAREN_ITEM_CHOICES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
// Form A signal: when the user lists explicit columns
// in `insert into <T> (col1, col2, …)`, the walker
// appends each matched name to
// `WalkContext::user_listed_columns`. The inner
// `values (…)` slot list then mirrors that user
// selection instead of the auto-filtered default
// (ADR-0024 §Phase D §column_value_list).
writes_user_listed_column: true,
},
];
const INSERT_PAREN_ITEM: Node = Node::Choice(INSERT_PAREN_ITEM_CHOICES);
@@ -173,6 +183,7 @@ const TABLE_NAME_WRITES: Node = Node::Ident {
highlight_override: None,
writes_table: true,
writes_column: false,
writes_user_listed_column: false,
};
/// Column-name slot in `set col = …` — resolves the column's
@@ -185,6 +196,7 @@ const SET_COLUMN: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: true,
writes_user_listed_column: false,
};
/// Column-name slot in `where col = …` — same writes-column
@@ -196,6 +208,7 @@ const FILTER_COLUMN: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: true,
writes_user_listed_column: false,
};
/// Value slot resolved at walk time from
src/dsl/grammar/ddl.rs
+17
View File
@@ -31,6 +31,7 @@ const TABLE_NAME_NEW: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
const TABLE_NAME_EXISTING: Node = Node::Ident {
@@ -40,6 +41,7 @@ const TABLE_NAME_EXISTING: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
const COLUMN_NAME: Node = Node::Ident {
@@ -49,6 +51,7 @@ const COLUMN_NAME: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
const COLUMN_NAME_NEW: Node = Node::Ident {
@@ -58,6 +61,7 @@ const COLUMN_NAME_NEW: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
const RELATIONSHIP_NAME: Node = Node::Ident {
@@ -67,6 +71,7 @@ const RELATIONSHIP_NAME: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
const RELATIONSHIP_NAME_NEW: Node = Node::Ident {
@@ -76,6 +81,7 @@ const RELATIONSHIP_NAME_NEW: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
// `[to]` and `[table]` connectives.
@@ -120,6 +126,7 @@ const DR_PARENT_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
Node::Punct('.'),
Node::Ident {
@@ -129,6 +136,7 @@ const DR_PARENT_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const DR_PARENT: Node = Node::Seq(DR_PARENT_NODES);
@@ -141,6 +149,7 @@ const DR_CHILD_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
Node::Punct('.'),
Node::Ident {
@@ -150,6 +159,7 @@ const DR_CHILD_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const DR_CHILD: Node = Node::Seq(DR_CHILD_NODES);
@@ -210,6 +220,7 @@ const AR_PARENT_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
Node::Punct('.'),
Node::Ident {
@@ -219,6 +230,7 @@ const AR_PARENT_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const AR_PARENT: Node = Node::Seq(AR_PARENT_NODES);
@@ -231,6 +243,7 @@ const AR_CHILD_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
Node::Punct('.'),
Node::Ident {
@@ -240,6 +253,7 @@ const AR_CHILD_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const AR_CHILD: Node = Node::Seq(AR_CHILD_NODES);
@@ -293,6 +307,7 @@ const RENAME_COLUMN_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const RENAME_COLUMN: Node = Node::Seq(RENAME_COLUMN_NODES);
@@ -627,6 +642,7 @@ const COL_SPEC_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
Node::Punct(':'),
Node::Ident {
@@ -636,6 +652,7 @@ const COL_SPEC_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
const COL_SPEC: Node = Node::Seq(COL_SPEC_NODES);
src/dsl/grammar/mod.rs
+8
View File
@@ -233,6 +233,14 @@ pub enum Node {
highlight_override: Option<HighlightClass>,
writes_table: bool,
writes_column: bool,
/// Append the matched text to
/// `WalkContext::user_listed_columns` (Phase D). Used by
/// the `insert into <T> (col1, col2, …)` column-list
/// idents — when the walker sees these, the form is
/// "Form A" and the inner values slot list mirrors the
/// user's explicit selection instead of the
/// auto-filtered schema default.
writes_user_listed_column: bool,
},
/// A number literal. The optional `validator` runs against
/// the matched text (used by Phase D value slots to enforce
src/dsl/grammar/shared.rs
+50 -8
View File
@@ -5,6 +5,7 @@
//! actions; Phase D extends with `where_clause`,
//! `column_value_list`, and the typed value slots.
use crate::completion::TableColumn;
use crate::dsl::grammar::{
IdentSource, IdentValidator, Node, NumberValidator, ValidationError, Word,
};
@@ -51,6 +52,7 @@ pub const TYPE_SLOT: Node = Node::Ident {
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
};
// --- Qualified column reference (`<Table>.<Column>`) --------------
@@ -63,6 +65,7 @@ const QUALIFIED_COLUMN_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
Node::Punct('.'),
Node::Ident {
@@ -72,6 +75,7 @@ const QUALIFIED_COLUMN_NODES: &[Node] = &[
highlight_override: None,
writes_table: false,
writes_column: false,
writes_user_listed_column: false,
},
];
pub const QUALIFIED_COLUMN: Node = Node::Seq(QUALIFIED_COLUMN_NODES);
@@ -373,18 +377,56 @@ pub fn current_column_value(ctx: &WalkContext) -> Node {
/// `Repeated(VALUE_LITERAL, ',', 1)` shape so existing
/// callers/tests continue to work.
pub fn column_value_list(ctx: &WalkContext) -> Node {
let Some(cols) = ctx.current_table_columns.as_ref() else {
let Some(table_cols) = ctx.current_table_columns.as_ref() else {
return FALLBACK_VALUE_LIST;
};
if cols.is_empty() {
if table_cols.is_empty() {
return FALLBACK_VALUE_LIST;
}
// Build a Seq of typed slots interleaved with commas.
// Each slot embeds its column name so the hint resolver
// can mention the column by name ("for `Email`: Type a
// quoted string …").
let mut children: Vec<Node> = Vec::with_capacity(cols.len() * 2);
for (i, col) in cols.iter().enumerate() {
// Three dispatch shapes (ADR-0024 §Phase D §column_value_list,
// matching `db::do_insert`'s user_cols logic):
//
// 1. Form A — user listed explicit columns
// (`insert into T (col1, col2, …) values (…)`): one slot
// per listed column, in the user's order, types resolved
// from the schema.
// 2. Form B — bare values keyword
// (`insert into T values (…)`): one slot per non-auto-
// generated column of T, in declaration order. Serial /
// shortid columns are skipped because the dispatch path
// auto-fills them (ADR-0018 §3).
// 3. Schemaless / fallback: the generic value-literal list.
let target_cols: Vec<&TableColumn> = ctx.user_listed_columns.as_ref().map_or_else(
|| {
// Form B — exclude auto-generated columns.
table_cols
.iter()
.filter(|c| !matches!(c.user_type, Type::Serial | Type::ShortId))
.collect()
},
|user_listed| {
// Form A — resolve each listed name from the schema.
// Names the schema doesn't know about silently drop;
// the bind-time path catches unknown columns.
user_listed
.iter()
.filter_map(|name| {
table_cols
.iter()
.find(|c| c.name.eq_ignore_ascii_case(name))
})
.collect()
},
);
if target_cols.is_empty() {
return FALLBACK_VALUE_LIST;
}
// Build a Seq of typed slots interleaved with commas. Each
// slot embeds its column name so the hint resolver can
// mention the column by name ("for `Email`: Type a quoted
// string …").
let mut children: Vec<Node> = Vec::with_capacity(target_cols.len() * 2);
for (i, col) in target_cols.iter().enumerate() {
if i > 0 {
children.push(Node::Punct(','));
}