From 8155480ca12a77a7adedc1dfdde13774f8327c21 Mon Sep 17 00:00:00 2001 From: kould Date: Mon, 30 Mar 2026 01:24:08 +0800 Subject: [PATCH] fix: keep added columns after existing schema order --- src/catalog/table.rs | 53 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/src/catalog/table.rs b/src/catalog/table.rs index be86eca3..dd7eb0be 100644 --- a/src/catalog/table.rs +++ b/src/catalog/table.rs @@ -116,7 +116,11 @@ impl TableCatalog { if self.column_idxs.contains_key(col.name()) { return Err(DatabaseError::DuplicateColumn(col.name().to_string())); } - let col_id = generator.generate().unwrap(); + let max_existing_id = self.columns.keys().max().copied(); + let mut col_id = generator.generate().unwrap(); + while max_existing_id.is_some_and(|max_id| col_id <= max_id) { + col_id = generator.generate().unwrap(); + } col.summary_mut().relation = ColumnRelation::Table { column_id: col_id, @@ -292,6 +296,7 @@ mod tests { use super::*; use crate::catalog::ColumnDesc; use crate::types::LogicalType; + use ulid::Generator; #[test] // | a (Int32) | b (Bool) | @@ -328,4 +333,50 @@ mod tests { assert_eq!(column_catalog.name(), "b"); assert_eq!(*column_catalog.datatype(), LogicalType::Boolean,); } + + #[test] + fn test_add_column_generates_id_after_existing_columns() { + for _ in 0..256 { + let mut table_catalog = TableCatalog::new( + "test".to_string().into(), + vec![ + ColumnCatalog::new( + "id".into(), + false, + ColumnDesc::new(LogicalType::Integer, None, false, None).unwrap(), + ), + ColumnCatalog::new( + "name".into(), + true, + ColumnDesc::new( + LogicalType::Varchar(None, sqlparser::ast::CharLengthUnits::Characters), + None, + false, + None, + ) + .unwrap(), + ), + ], + ) + .unwrap(); + let max_existing_id = table_catalog + .columns() + .filter_map(|column| column.id()) + .max() + .unwrap(); + let mut generator = Generator::new(); + let new_id = table_catalog + .add_column( + ColumnCatalog::new( + "age".into(), + true, + ColumnDesc::new(LogicalType::Integer, None, false, None).unwrap(), + ), + &mut generator, + ) + .unwrap(); + + assert!(new_id > max_existing_id); + } + } }