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

ADR-0017 implementation: per-cell type-change with override flags

Browse Source 
Replaces the placeholder "trust STRICT" body of do_change_column_type
with the per-cell transformer matrix from ADR-0017. Adds:

- src/type_change.rs: CellOutcome { Clean / Lossy / Incompatible }
  + transform_cell + static_refusal covering every matrix pair
  from §3 (54 unit tests).
- --force-conversion and --dont-convert flags on `change column`
  (mutually exclusive at parse time per §5).
- Refined PK rule (§4.1): refused only when the column has an
  inbound FK and fk_target_type would change. Outbound-FK columns
  still refused outright (§4.2). PK / shortid uniqueness checked
  post-transformation (§4.3).
- Bordered diagnostic tables (lossy / incompatible / collision)
  via the pretty-table renderer (§7) — uses ADR-0016's primitives.
- [client-side] success note (§6) when any cell was rewritten.
- Friendly wrapper for engine-level errors under --dont-convert
  so no engine vocabulary leaks (ADR-0002 user-facing posture).

ADR-0017 §3 + §7 amended in place (with user sign-off): serial->int
added explicitly to the always-clean matrix, and diagnostic rows
identify themselves by PK value(s) rather than positional indices
(SQLite returns rows unordered without ORDER BY, so positional
"row 5" is unaddressable).

Tests: 449 -> 517 (+68). Clippy clean with nursery lints.
This commit is contained in:
claude@clouddev1
2026-05-08 13:21:07 +00:00
parent 545cbf5c0e
commit 00947b928c
12 changed files with 2598 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/adr/0017-column-type-change-compatibility.md
+48 -23
View File
@@ -108,6 +108,7 @@ statically refused (carried over from B2/C2). Anything ↔
| `int` / `serial` | `real` | widening; precision caveat for ¦v¦ > 2⁵³ noted in docs but not policed |
| `int` / `serial` | `decimal` | exact decimal representation |
| `int` / `serial` | `text` | stringify |
| `serial` | `int` | identity at the storage class level (both store as INTEGER); drops the auto-increment metadata. The canonical PK conversion enabled by §4.1's `fk_target_type`-aware refinement. |
| `bool` | `int` | 0/1 |
| `bool` | `real` | 0.0/1.0 |
| `bool` | `decimal` | "0"/"1" |
@@ -359,14 +360,14 @@ and these conversion diagnostics.
Cannot change `T.col` from real to int: 50 row(s) would
discard information.
┌─────┬───────┬─────┬───────────────────────────────────┐
│ Row │ From │ To │ Reason │
├─────┼───────┼─────┼───────────────────────────────────┤
│ 5 │ 3.14 │ 3 │ truncated; would discard 0.14 │
│ 12 │ 2.71 │ 2 │ truncated; would discard 0.71 │
│ 18 │ 1.5 │ 1 │ truncated; would discard 0.5 │
│ … │ … │ … │ … and 47 more │
└─────┴───────┴─────┴───────────────────────────────────┘
┌─────────┬───────┬─────┬───────────────────────────────────┐
│ id (PK) │ From │ To │ Reason │
├─────────┼───────┼─────┼───────────────────────────────────┤
│ 5 │ 3.14 │ 3 │ truncated; would discard 0.14 │
│ 12 │ 2.71 │ 2 │ truncated; would discard 0.71 │
│ 18 │ 1.5 │ 1 │ truncated; would discard 0.5 │
│ … │ … │ … │ … and 47 more │
└─────────┴───────┴─────┴───────────────────────────────────┘
if you want to execute this conversion in spite of the
problems, re-run with `--force-conversion`.
@@ -378,13 +379,13 @@ problems, re-run with `--force-conversion`.
Cannot change `T.col` from text to int: 3 row(s) cannot
be converted.
┌─────┬───────┬───────────────────────┐
│ Row │ Value │ Reason │
├─────┼───────┼───────────────────────┤
│ 3 │ abc │ not a valid int │
│ 7 │ x42 │ not a valid int │
│ 12 │ │ not a valid int │
└─────┴───────┴───────────────────────┘
┌─────────┬───────┬───────────────────────┐
│ id (PK) │ Value │ Reason │
├─────────┼───────┼───────────────────────┤
│ 3 │ abc │ not a valid int │
│ 7 │ x42 │ not a valid int │
│ 12 │ │ not a valid int │
└─────────┴───────┴───────────────────────┘
```
The trailing `--force-conversion` hint is omitted for
@@ -397,11 +398,11 @@ forward-look — would re-introduce one).
Cannot change `T.col` from real to int: 1 collision(s)
would violate uniqueness.
┌─────────┬─────────────────┬──────────────────┐
│ Becomes │ Source rows │ Source values │
├─────────┼─────────────────┼──────────────────┤
│ 3 │ row 5, row 12 │ 3.14, 3.7 │
└─────────┴─────────────────┴──────────────────┘
┌─────────┬──────────────────┬──────────────────┐
│ Becomes │ Source rows (id) │ Source values │
├─────────┼──────────────────┼──────────────────┤
│ 3 │ 5, 12 │ 3.14, 3.7 │
└─────────┴──────────────────┴──────────────────┘
```
#### Common rules
@@ -411,9 +412,33 @@ would violate uniqueness.
literal text "and N more" inside the row is rendered
inside the table — not as a footer line. Keeps the
bordered shape intact.
- Row indices are 1-based to match how learners count
rows; the tool counts internally from 0.
- Numeric "Row" / "Becomes" columns inherit numeric
- Rows are identified by their **primary-key value(s)**, not
by positional indices. SQLite returns rows in unspecified
order without `ORDER BY`, so a positional "row 5" would
not be reproducible or addressable by the user. The PK is
the natural row identifier in a relational setting and is
what the user would type in a `where` clause to find or
fix the offending cell.
- Single PK: rendered as one column whose header is the
PK column name with a trailing `(PK)` marker
(e.g. `id (PK)`); cells carry the raw PK value with no
`column=` prefix. The marker appears once per table, in
the header.
- Compound PK: one column per PK component, each header
annotated `(PK)` (e.g. `a (PK)`, `b (PK)`); cells carry
the raw component values.
- Uniqueness-collision tables list the colliding rows'
PK values comma-separated inside a single `Source rows`
cell whose header carries the PK column name(s) in
parentheses (e.g. `Source rows (id)` or `Source rows
(a, b)`). Compound-PK source rows render as tuples:
`(1,2), (1,3)`.
- The change-column command always operates on a table
with at least one PK column (every `create table` in v1
produces a PK; the AST permits PK-less tables, but no
grammar produces one today). If a PK-less surface ever
lands, this section will be revisited.
- Numeric PK and "Becomes" columns inherit numeric
right-alignment from ADR-0016 §2.
- Cells that would render multi-line content (for `text →`
conversions where source values contain newlines) honour
src/app.rs
+43 -1
View File
@@ -13,7 +13,8 @@ use tracing::{trace, warn};
use crate::action::Action;
use crate::db::{
CascadeEffect, DataResult, DeleteResult, InsertResult, TableDescription, UpdateResult,
CascadeEffect, ChangeColumnTypeResult, DataResult, DeleteResult, InsertResult,
TableDescription, UpdateResult,
};
use crate::dsl::{Command, ParseError, parse_command};
use crate::event::AppEvent;
@@ -312,6 +313,10 @@ impl App {
self.handle_dsl_delete_success(&command, &result);
Vec::new()
}
AppEvent::DslChangeColumnSucceeded { command, result } => {
self.handle_dsl_change_column_success(&command, result);
Vec::new()
}
AppEvent::DslFailed { command, error } => {
self.handle_dsl_failure(&command, &error);
Vec::new()
@@ -761,6 +766,43 @@ impl App {
}
}
fn handle_dsl_change_column_success(
&mut self,
command: &Command,
result: ChangeColumnTypeResult,
) {
let summary = format!("[ok] {} {}", command.verb(), command.display_subject());
self.note_system(summary);
if let Some(note) = result.client_side {
// ADR-0017 §6: pedagogical hook telling the learner
// "the tool did this for you; raw SQL would need a
// CAST or application-level code." Lossy variant
// adds the lossy count when --force-conversion was
// used.
let line = if note.lossy > 0 {
format!(
"[client-side] {n} row(s) transformed; {l} of those discarded \
information (lossy). In raw SQL this would need an explicit \
`CAST` or application-level code.",
n = note.transformed,
l = note.lossy
)
} else {
format!(
"[client-side] {n} row(s) were transformed before being stored. \
In raw SQL this would need an explicit `CAST` or \
application-level code.",
n = note.transformed
)
};
self.note_system(line);
}
for line in crate::output_render::render_structure(&result.description) {
self.note_system(line);
}
self.current_table = Some(result.description);
}
fn handle_dsl_delete_success(&mut self, command: &Command, result: &DeleteResult) {
self.note_system(format!(
"[ok] {} {}",
src/db.rs
+1180 -94
View File
File diff suppressed because it is too large Load Diff
src/dsl/command.rs
+22 -5
View File
@@ -63,15 +63,17 @@ pub enum Command {
},
/// Change a column's type. Implemented via the
/// rebuild-table primitive (ADR-0013) since SQLite's
/// ALTER TABLE does not support type changes. Refused
/// for PK columns and for columns involved in a declared
/// relationship — those would require cascading FK
/// type updates the v1 surface deliberately doesn't
/// expose.
/// ALTER TABLE does not support type changes.
///
/// Per ADR-0017 the actual conversion model is a per-cell
/// dry-run against a curated transformer matrix; the two
/// optional flags carried in `mode` let the user opt into
/// lossy conversions or skip the client-side layer entirely.
ChangeColumnType {
table: String,
column: String,
ty: Type,
mode: ChangeColumnMode,
},
/// Establish a 1:n relationship: parent_table.parent_column
/// is the primary-key side; child_table.child_column is the
@@ -125,6 +127,21 @@ pub enum Command {
},
}
/// Conversion mode for `change column …` (ADR-0017 §5).
///
/// `Default` runs the per-cell dry-run and refuses on lossy or
/// incompatible cells. `ForceConversion` accepts lossy cells but
/// still refuses incompatibles. `DontConvert` skips the entire
/// client-side layer and lets the database's STRICT typing
/// decide. `ForceConversion` and `DontConvert` are mutually
/// exclusive at the grammar level.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ChangeColumnMode {
Default,
ForceConversion,
DontConvert,
}
/// How an UPDATE / DELETE selects which rows to operate on.
/// `Where` is the default safe form. `AllRows` is the explicit
/// `--all-rows` flag opt-in for unfiltered operations.
src/dsl/mod.rs
+3 -1
View File
@@ -17,7 +17,9 @@ pub mod types;
pub mod value;
pub use action::ReferentialAction;
pub use command::{ColumnSpec, Command, RelationshipSelector, RowFilter};
pub use command::{
ChangeColumnMode, ColumnSpec, Command, RelationshipSelector, RowFilter,
};
pub use parser::{ParseError, parse_command};
pub use types::Type;
pub use value::Value;
src/dsl/parser.rs
+104 -5
View File
@@ -14,7 +14,9 @@ use chumsky::error::RichReason;
use chumsky::prelude::*;
use crate::dsl::action::ReferentialAction;
use crate::dsl::command::{ColumnSpec, Command, RelationshipSelector, RowFilter};
use crate::dsl::command::{
ChangeColumnMode, ColumnSpec, Command, RelationshipSelector, RowFilter,
};
use crate::dsl::types::Type;
use crate::dsl::value::Value;
@@ -172,9 +174,10 @@ fn command_parser<'a>()
.then(identifier())
.map(|((table, old), new)| Command::RenameColumn { table, old, new });
// `change column [in] [table] <T>: <col> (<newtype>)`.
// Same shape as `add column` — the column-and-type
// tuple in parens — but the verb is `change`.
// `change column [in] [table] <T>: <col> (<newtype>) [flags]`
// where `flags` is at most one of `--force-conversion` /
// `--dont-convert` (mutually exclusive at parse time per
// ADR-0017 §5).
let change_column = keyword_ci("change")
.ignore_then(keyword_ci("column"))
.ignore_then(optional_keyword("in"))
@@ -185,7 +188,13 @@ fn command_parser<'a>()
.then_ignore(just('(').padded())
.then(type_keyword())
.then_ignore(just(')').padded())
.map(|((table, column), ty)| Command::ChangeColumnType { table, column, ty });
.then(change_column_flags())
.map(|(((table, column), ty), mode)| Command::ChangeColumnType {
table,
column,
ty,
mode,
});
let add_relationship = add_relationship_parser();
let drop_relationship = drop_relationship_parser();
@@ -555,6 +564,33 @@ fn create_fk_flag<'a>()
.map(|opt| opt.is_some())
}
/// Optional flags for `change column …` (ADR-0017 §5).
/// Allows zero or one of the two mutually-exclusive flags;
/// emits a custom parse error if both are present, naming both
/// flags so the user knows what the conflict is.
fn change_column_flags<'a>()
-> impl Parser<'a, &'a str, ChangeColumnMode, extra::Err<Rich<'a, char>>> + Clone {
let force = just("--force-conversion")
.padded()
.to(ChangeColumnMode::ForceConversion);
let dont = just("--dont-convert")
.padded()
.to(ChangeColumnMode::DontConvert);
choice((force, dont))
.repeated()
.collect::<Vec<_>>()
.try_map(|flags, span| match flags.as_slice() {
[] => Ok(ChangeColumnMode::Default),
[single] => Ok(*single),
_ => Err(Rich::custom(
span,
"`--force-conversion` and `--dont-convert` are mutually \
exclusive — pick one."
.to_string(),
)),
})
}
/// Parse the optional `with pk [<spec>]` clause that may follow
/// `create table <Name>`. Returns the list of (name, type) pairs
/// that form the primary key. An absent clause returns an empty
@@ -870,6 +906,7 @@ mod tests {
table: "Customers".to_string(),
column: "Score".to_string(),
ty: Type::Int,
mode: ChangeColumnMode::Default,
}
);
}
@@ -882,6 +919,7 @@ mod tests {
table: "Customers".to_string(),
column: "Score".to_string(),
ty: Type::Real,
mode: ChangeColumnMode::Default,
}
);
}
@@ -894,10 +932,71 @@ mod tests {
table: "Customers".to_string(),
column: "Score".to_string(),
ty: Type::Text,
mode: ChangeColumnMode::Default,
}
);
}
#[test]
fn change_column_with_force_conversion_flag() {
assert_eq!(
ok("change column Customers: Score (int) --force-conversion"),
Command::ChangeColumnType {
table: "Customers".to_string(),
column: "Score".to_string(),
ty: Type::Int,
mode: ChangeColumnMode::ForceConversion,
}
);
}
#[test]
fn change_column_with_dont_convert_flag() {
assert_eq!(
ok("change column Customers: Score (int) --dont-convert"),
Command::ChangeColumnType {
table: "Customers".to_string(),
column: "Score".to_string(),
ty: Type::Int,
mode: ChangeColumnMode::DontConvert,
}
);
}
#[test]
fn change_column_rejects_both_flags() {
let e = err(
"change column Customers: Score (int) --force-conversion --dont-convert",
);
match e {
ParseError::Invalid { message, .. } => {
assert!(
message.contains("--force-conversion")
&& message.contains("--dont-convert"),
"expected both flag names in error: {message}"
);
assert!(
message.contains("mutually exclusive") || message.contains("pick one"),
"{message}"
);
}
ParseError::Empty => panic!("unexpected empty error"),
}
}
#[test]
fn change_column_rejects_both_flags_in_either_order() {
// Both orderings — and same-flag-twice — should reject
// with a uniform "pick one" signal.
let e = err("change column T: c (int) --dont-convert --force-conversion");
match e {
ParseError::Invalid { message, .. } => {
assert!(message.contains("mutually exclusive"), "{message}");
}
ParseError::Empty => panic!("unexpected empty error"),
}
}
#[test]
fn drop_table_simple() {
assert_eq!(
src/dsl/value.rs
+2 -2
View File
@@ -208,7 +208,7 @@ impl Value {
}
}
fn validate_date(s: &str) -> Result<(), String> {
pub(crate) fn validate_date(s: &str) -> Result<(), String> {
// Expect YYYY-MM-DD: 10 chars, two dashes at fixed positions.
let bytes = s.as_bytes();
if bytes.len() != 10 || bytes[4] != b'-' || bytes[7] != b'-' {
@@ -231,7 +231,7 @@ fn validate_date(s: &str) -> Result<(), String> {
Ok(())
}
fn validate_datetime(s: &str) -> Result<(), String> {
pub(crate) fn validate_datetime(s: &str) -> Result<(), String> {
// Minimum: YYYY-MM-DDTHH:MM:SS = 19 chars. Allow optional
// fractional seconds (.fff) and optional Z or ±HH:MM offset.
if s.len() < 19 {
src/event.rs
+11 -1
View File
@@ -7,7 +7,10 @@
use crossterm::event::KeyEvent;
use crate::db::{DataResult, DeleteResult, InsertResult, TableDescription, UpdateResult};
use crate::db::{
ChangeColumnTypeResult, DataResult, DeleteResult, InsertResult, TableDescription,
UpdateResult,
};
use crate::dsl::Command;
#[derive(Debug, Clone)]
@@ -39,6 +42,13 @@ pub enum AppEvent {
command: Command,
result: DeleteResult,
},
/// A `change column …` succeeded. `result` carries both the
/// post-rebuild description (for the auto-show) and the
/// optional `[client-side]` note (ADR-0017 §6).
DslChangeColumnSucceeded {
command: Command,
result: ChangeColumnTypeResult,
},
/// A DSL command failed. `error` is already a friendly
/// message produced via `DbError::friendly_message`.
DslFailed {
src/lib.rs
+1
View File
@@ -19,4 +19,5 @@ pub mod persistence;
pub mod project;
pub mod runtime;
pub mod theme;
pub mod type_change;
pub mod ui;
src/output_render.rs
+28 -1
View File
@@ -136,11 +136,38 @@ pub fn render_structure(desc: &TableDescription) -> Vec<String> {
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum Alignment {
pub enum Alignment {
Left,
Right,
}
/// Per-column alignment hint for [`render_diagnostic_table`].
#[must_use]
pub const fn numeric_alignment_for(ty: Type) -> Alignment {
alignment_for(Some(ty))
}
/// Render an arbitrary bordered table from headers + rows +
/// per-column alignments. Used for change-column diagnostic
/// output (ADR-0017 §7) — anything tabular goes through the
/// pretty-table renderer.
///
/// Cell contents are sanitised the same way as
/// [`render_data_table`]: newlines and control characters
/// become visible markers (display-only).
#[must_use]
pub fn render_diagnostic_table(
headers: &[String],
rows: &[Vec<String>],
alignments: &[Alignment],
) -> Vec<String> {
let body: Vec<Vec<String>> = rows
.iter()
.map(|r| r.iter().map(|c| sanitize_cell(c)).collect())
.collect();
render_table(headers, &body, alignments)
}
/// Map a user-facing type to its column alignment per
/// ADR-0016 §2. `None` (foreign-attached, no metadata)
/// falls back to Left.
src/runtime.rs
+15 -4
View File
@@ -29,7 +29,8 @@ use crate::action::Action;
use crate::app::App;
use crate::cli::Args;
use crate::db::{
DataResult, Database, DbError, DeleteResult, InsertResult, TableDescription, UpdateResult,
ChangeColumnTypeResult, DataResult, Database, DbError, DeleteResult, InsertResult,
TableDescription, UpdateResult,
};
use crate::dsl::Command;
use crate::event::AppEvent;
@@ -946,6 +947,10 @@ fn spawn_dsl_dispatch(
command: command.clone(),
result,
},
Ok(CommandOutcome::ChangeColumn(result)) => AppEvent::DslChangeColumnSucceeded {
command: command.clone(),
result,
},
Err(DbError::PersistenceFatal {
operation,
path,
@@ -982,6 +987,7 @@ enum CommandOutcome {
Insert(InsertResult),
Update(UpdateResult),
Delete(DeleteResult),
ChangeColumn(ChangeColumnTypeResult),
}
/// Execute a parsed user command and return either a typed
@@ -1019,10 +1025,15 @@ async fn execute_command_typed(
.rename_column(table, old, new, src)
.await
.map(|d| CommandOutcome::Schema(Some(d))),
Command::ChangeColumnType { table, column, ty } => database
.change_column_type(table, column, ty, src)
Command::ChangeColumnType {
table,
column,
ty,
mode,
} => database
.change_column_type(table, column, ty, mode, src)
.await
.map(|d| CommandOutcome::Schema(Some(d))),
.map(CommandOutcome::ChangeColumn),
Command::AddRelationship {
name,
parent_table,
src/type_change.rs
+1141
View File
File diff suppressed because it is too large Load Diff