Merge 'Add ColDef struct to make schema::Column creation more ergonomic' from Preston Thorpe

RE: #3970
That Column::new having 14 boolean arguments was not great.
Also this removes the unneeded `parent_cols: Vec<String>` from
`ResolvedFkRef`

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #3973
This commit is contained in:
Jussi Saurio
2025-11-17 09:17:56 +02:00
committed by GitHub
10 changed files with 152 additions and 187 deletions

View File

@@ -2254,7 +2254,7 @@ mod tests {
use super::*;
use crate::incremental::dbsp::Delta;
use crate::incremental::operator::{FilterOperator, FilterPredicate};
use crate::schema::{BTreeTable, Column as SchemaColumn, Schema, Type};
use crate::schema::{BTreeTable, ColDef, Column as SchemaColumn, Schema, Type};
use crate::storage::pager::CreateBTreeFlags;
use crate::translate::logical::{ColumnInfo, LogicalPlanBuilder, LogicalSchema};
use crate::util::IOExt;
@@ -2278,11 +2278,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_text(
Some("name".to_string()),
@@ -2320,11 +2321,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_text(
Some("product_name".to_string()),
@@ -2362,11 +2364,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_integer(
Some("user_id".to_string()),
@@ -2406,11 +2409,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_text(
Some("name".to_string()),
@@ -2440,11 +2444,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_integer(
Some("customer_id".to_string()),
@@ -2484,11 +2489,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_text(
Some("name".to_string()),

View File

@@ -1383,7 +1383,7 @@ impl IncrementalView {
#[cfg(test)]
mod tests {
use super::*;
use crate::schema::{BTreeTable, Column as SchemaColumn, Schema, Type};
use crate::schema::{BTreeTable, ColDef, Column as SchemaColumn, Schema, Type};
use std::sync::Arc;
use turso_parser::ast;
use turso_parser::parser::Parser;
@@ -1404,11 +1404,13 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
unique: false,
hidden: false,
},
),
SchemaColumn::new_default_text(Some("name".to_string()), "TEXT".to_string(), None),
],
@@ -1431,11 +1433,13 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
unique: false,
hidden: false,
},
),
SchemaColumn::new(
Some("customer_id".to_string()),
@@ -1443,11 +1447,7 @@ mod tests {
None,
Type::Integer,
None,
false,
false,
false,
false,
false,
ColDef::default(),
),
SchemaColumn::new_default_integer(
Some("total".to_string()),
@@ -1474,11 +1474,13 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
unique: false,
hidden: false,
},
),
SchemaColumn::new_default_text(Some("name".to_string()), "TEXT".to_string(), None),
SchemaColumn::new(
@@ -1487,11 +1489,7 @@ mod tests {
None,
Type::Real,
None,
false,
false,
false,
false,
false,
ColDef::default(),
),
],
has_rowid: true,
@@ -1513,11 +1511,7 @@ mod tests {
None,
Type::Text,
None,
false,
false,
false,
false,
false,
ColDef::default(),
),
SchemaColumn::new_default_integer(
Some("level".to_string()),

View File

@@ -973,7 +973,6 @@ impl Schema {
out.push(ResolvedFkRef {
child_table: Arc::clone(&child),
fk: Arc::clone(fk),
parent_cols,
child_cols,
child_pos,
parent_pos,
@@ -1089,7 +1088,6 @@ impl Schema {
out.push(ResolvedFkRef {
child_table: Arc::clone(&child),
fk: Arc::clone(fk),
parent_cols,
child_cols,
child_pos,
parent_pos,
@@ -1836,11 +1834,15 @@ pub fn create_table(tbl_name: &str, body: &CreateTableBody, root_page: i64) -> R
default,
ty,
collation,
primary_key,
typename_exactly_integer && primary_key && !primary_key_desc_columns_constraint,
notnull,
unique,
false,
ColDef {
primary_key,
rowid_alias: typename_exactly_integer
&& primary_key
&& !primary_key_desc_columns_constraint,
notnull,
unique,
hidden: false,
},
));
}
if options.contains(TableOptions::WITHOUT_ROWID) {
@@ -2000,9 +2002,7 @@ pub struct ResolvedFkRef {
pub fk: Arc<ForeignKey>,
/// Resolved, normalized column names.
pub parent_cols: Vec<String>,
pub child_cols: Vec<String>,
/// Column positions in the child/parent tables (pos_in_table)
pub child_pos: Vec<usize>,
pub parent_pos: Vec<usize>,
@@ -2071,6 +2071,15 @@ pub struct Column {
raw: u16,
}
#[derive(Default)]
pub struct ColDef {
pub primary_key: bool,
pub rowid_alias: bool,
pub notnull: bool,
pub unique: bool,
pub hidden: bool,
}
// flags
const F_PRIMARY_KEY: u16 = 1;
const F_ROWID_ALIAS: u16 = 2;
@@ -2088,25 +2097,14 @@ impl Column {
pub fn affinity(&self) -> Affinity {
Affinity::affinity(&self.ty_str)
}
pub const fn new_default_text(
pub fn new_default_text(
name: Option<String>,
ty_str: String,
default: Option<Box<Expr>>,
) -> Self {
Self::new(
name,
ty_str,
default,
Type::Text,
None,
false,
false,
false,
false,
false,
)
Self::new(name, ty_str, default, Type::Text, None, ColDef::default())
}
pub const fn new_default_integer(
pub fn new_default_integer(
name: Option<String>,
ty_str: String,
default: Option<Box<Expr>>,
@@ -2117,45 +2115,36 @@ impl Column {
default,
Type::Integer,
None,
false,
false,
false,
false,
false,
ColDef::default(),
)
}
#[inline]
#[allow(clippy::too_many_arguments)]
pub const fn new(
name: Option<String>,
ty_str: String,
default: Option<Box<Expr>>,
ty: Type,
col: Option<CollationSeq>,
primary_key: bool,
rowid_alias: bool,
notnull: bool,
unique: bool,
hidden: bool,
coldef: ColDef,
) -> Self {
let mut raw = 0u16;
raw |= (ty as u16) << TYPE_SHIFT;
if let Some(c) = col {
raw |= (c as u16) << COLL_SHIFT;
}
if primary_key {
if coldef.primary_key {
raw |= F_PRIMARY_KEY
}
if rowid_alias {
if coldef.rowid_alias {
raw |= F_ROWID_ALIAS
}
if notnull {
if coldef.notnull {
raw |= F_NOTNULL
}
if unique {
if coldef.unique {
raw |= F_UNIQUE
}
if hidden {
if coldef.hidden {
raw |= F_HIDDEN
}
Self {
@@ -2305,11 +2294,13 @@ impl From<&ColumnDefinition> for Column {
default,
ty,
collation,
primary_key,
primary_key && matches!(ty, Type::Integer),
notnull,
unique,
hidden,
ColDef {
primary_key,
rowid_alias: primary_key && matches!(ty, Type::Integer),
notnull,
unique,
hidden,
},
)
}
}

View File

@@ -152,7 +152,7 @@ mod tests {
use turso_parser::ast::{Literal, Name, Operator, TableInternalId, UnaryOperator};
use crate::{
schema::{BTreeTable, Column, Table, Type},
schema::{BTreeTable, ColDef, Column, Table, Type},
translate::plan::{ColumnUsedMask, IterationDirection, JoinedTable, Operation, Scan},
};
@@ -374,11 +374,7 @@ mod tests {
None,
Type::Text,
collation,
false,
false,
false,
false,
false,
ColDef::default(),
)],
unique_sets: vec![],
foreign_keys: vec![],
@@ -417,11 +413,7 @@ mod tests {
None,
Type::Text,
left,
false,
false,
false,
false,
false,
ColDef::default(),
)],
unique_sets: vec![],
foreign_keys: vec![],
@@ -451,11 +443,7 @@ mod tests {
None,
Type::Text,
right,
false,
false,
false,
false,
false,
ColDef::default(),
)],
unique_sets: vec![],
foreign_keys: vec![],
@@ -492,11 +480,13 @@ mod tests {
None,
Type::Integer,
collation,
true,
true,
false,
true,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: false,
unique: true,
hidden: false,
},
)],
unique_sets: vec![],
foreign_keys: vec![],

View File

@@ -7,7 +7,7 @@ use turso_parser::ast::{
use crate::error::{
SQLITE_CONSTRAINT_NOTNULL, SQLITE_CONSTRAINT_PRIMARYKEY, SQLITE_CONSTRAINT_UNIQUE,
};
use crate::schema::{self, BTreeTable, Index, ResolvedFkRef, Table};
use crate::schema::{self, BTreeTable, ColDef, Index, ResolvedFkRef, Table};
use crate::translate::emitter::{
emit_cdc_insns, emit_cdc_patch_record, prepare_cdc_if_necessary, OperationMode,
};
@@ -1282,11 +1282,13 @@ pub const ROWID_COLUMN: Column = Column::new(
None, // default
schema::Type::Integer,
None,
true, // primary key
true, // rowid alias
true, // notnull
false, // unique
false, // hidden
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
hidden: false,
unique: false,
},
);
/// Represents how a table should be populated during an INSERT.

View File

@@ -2378,7 +2378,7 @@ impl<'a> LogicalPlanBuilder<'a> {
#[cfg(test)]
mod tests {
use super::*;
use crate::schema::{BTreeTable, Column as SchemaColumn, Schema, Type};
use crate::schema::{BTreeTable, ColDef, Column as SchemaColumn, Schema, Type};
use turso_parser::parser::Parser;
fn create_test_schema() -> Schema {
@@ -2397,11 +2397,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_text(Some("name".to_string()), "TEXT".to_string(), None),
SchemaColumn::new_default_integer(
@@ -2432,11 +2433,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_integer(
Some("user_id".to_string()),
@@ -2454,11 +2456,7 @@ mod tests {
None,
Type::Real,
None,
false,
false,
false,
false,
false,
ColDef::default(),
),
],
has_rowid: true,
@@ -2483,11 +2481,12 @@ mod tests {
None,
Type::Integer,
None,
true,
true,
true,
false,
false,
ColDef {
primary_key: true,
rowid_alias: true,
notnull: true,
..Default::default()
},
),
SchemaColumn::new_default_text(Some("name".to_string()), "TEXT".to_string(), None),
SchemaColumn::new(
@@ -2496,11 +2495,7 @@ mod tests {
None,
Type::Real,
None,
false,
false,
false,
false,
false,
ColDef::default(),
),
SchemaColumn::new_default_integer(
Some("product_id".to_string()),

View File

@@ -507,7 +507,7 @@ mod tests {
use super::*;
use crate::{
schema::{BTreeTable, Column, Index, IndexColumn, Table, Type},
schema::{BTreeTable, ColDef, Column, Index, IndexColumn, Table, Type},
translate::{
optimizer::{
access_method::AccessMethodParams,
@@ -1676,11 +1676,11 @@ mod tests {
None,
c.ty,
None,
false,
c.is_rowid_alias,
false,
false,
false,
ColDef {
primary_key: false,
rowid_alias: c.is_rowid_alias,
..Default::default()
},
)
}
fn _create_column_of_type(name: &str, ty: Type) -> Column {

View File

@@ -5,7 +5,7 @@ use turso_parser::ast::{
use crate::{
function::AggFunc,
schema::{BTreeTable, Column, FromClauseSubquery, Index, Schema, Table},
schema::{BTreeTable, ColDef, Column, FromClauseSubquery, Index, Schema, Table},
translate::{
collate::get_collseq_from_expr, emitter::UpdateRowSource,
optimizer::constraints::SeekRangeConstraint,
@@ -921,11 +921,7 @@ impl JoinedTable {
None,
Type::Blob, // FIXME: infer proper type
None,
false,
false,
false,
false,
false,
ColDef::default(),
)
})
.collect::<Vec<_>>();

View File

@@ -2,7 +2,9 @@ use std::sync::Arc;
use crate::ast;
use crate::ext::VTabImpl;
use crate::schema::{create_table, BTreeTable, Column, Table, Type, RESERVED_TABLE_PREFIXES};
use crate::schema::{
create_table, BTreeTable, ColDef, Column, Table, Type, RESERVED_TABLE_PREFIXES,
};
use crate::storage::pager::CreateBTreeFlags;
use crate::translate::emitter::{
emit_cdc_full_record, emit_cdc_insns, prepare_cdc_if_necessary, OperationMode, Resolver,
@@ -848,11 +850,7 @@ pub fn translate_drop_table(
None,
Type::Integer,
None,
false,
false,
false,
false,
false,
ColDef::default(),
)],
is_strict: false,
unique_sets: vec![],

View File

@@ -1,6 +1,7 @@
#![allow(unused)]
use crate::incremental::view::IncrementalView;
use crate::numeric::StrToF64;
use crate::schema::ColDef;
use crate::translate::emitter::TransactionMode;
use crate::translate::expr::{walk_expr_mut, WalkControl};
use crate::translate::plan::JoinedTable;
@@ -1387,11 +1388,7 @@ pub fn extract_view_columns(
None,
table_column.ty(),
table_column.collation_opt(),
false,
false,
false,
false,
false,
ColDef::default(),
),
});
}
@@ -1437,11 +1434,7 @@ pub fn extract_view_columns(
None,
table_column.ty(),
table_column.collation_opt(),
false,
false,
false,
false,
false,
ColDef::default(),
),
});
}