Skip to content

Commit 78e21bf

Browse files
Shubham8287Shubham8287
committed
introduced type_check, and handle redundant defaults
1 parent c3dc868 commit 78e21bf

7 files changed

Lines changed: 103 additions & 68 deletions

File tree

‎crates/datastore/src/locking_tx_datastore/datastore.rs‎

Lines changed: 1 addition & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3526,7 +3526,7 @@ mod tests {
35263526
let defaults = vec![AlgebraicValue::U8(42)];
35273527

35283528
let mut tx = begin_mut_tx(&datastore);
3529-
// insert a row in tx_state when before adding column
3529+
// insert a row in tx_state before adding column
35303530
tx.insert_product_value(table_id, &initial_row).unwrap();
35313531
// add column and then rollback
35323532
let rollback_table_id =
@@ -3609,58 +3609,4 @@ mod tests {
36093609

36103610
Ok(())
36113611
}
3612-
3613-
#[test]
3614-
fn test_table_schemas_consistency() -> ResultTest<()> {
3615-
let datastore = get_datastore()?;
3616-
3617-
let mut tx = begin_mut_tx(&datastore);
3618-
3619-
let initial_sum_type = AlgebraicType::sum([("ba", AlgebraicType::U16)]);
3620-
let initial_columns = [
3621-
ColumnSchema::for_test(0, "a", AlgebraicType::U64),
3622-
ColumnSchema::for_test(1, "b", initial_sum_type.clone()),
3623-
];
3624-
3625-
let initial_indices = [
3626-
IndexSchema::for_test("index_a", BTreeAlgorithm::from(0)),
3627-
IndexSchema::for_test("index_b", BTreeAlgorithm::from(1)),
3628-
];
3629-
3630-
let sequence = SequenceRow {
3631-
id: SequenceId::SENTINEL.into(),
3632-
table: 0,
3633-
col_pos: 0,
3634-
name: "Foo_id_seq",
3635-
start: 5,
3636-
};
3637-
3638-
let schema = user_public_table(
3639-
initial_columns,
3640-
initial_indices.clone(),
3641-
[],
3642-
map_array([sequence]),
3643-
None,
3644-
None,
3645-
);
3646-
3647-
let table_id = datastore.create_table_mut_tx(&mut tx, schema)?;
3648-
let initial_row = product![0u64, AlgebraicValue::sum(0, AlgebraicValue::U16(1))];
3649-
3650-
for _ in 0..5000 {
3651-
insert(&datastore, &mut tx, table_id, &initial_row).unwrap();
3652-
}
3653-
commit(&datastore, tx)?;
3654-
3655-
let tx = begin_tx(&datastore);
3656-
let table_schema = tx.schema_for_table(table_id).unwrap();
3657-
let table_schema_raw = tx.schema_for_table_raw(table_id).unwrap();
3658-
3659-
//TODO: Fix the bug and update the assert
3660-
assert_ne!(
3661-
table_schema.sequences[0].allocated, table_schema_raw.sequences[0].allocated,
3662-
"Allocated sequence values are differ between schema and raw schema"
3663-
);
3664-
Ok(())
3665-
}
36663612
}

‎crates/datastore/src/locking_tx_datastore/mut_tx.rs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -512,9 +512,16 @@ impl MutTxId {
512512
&mut self,
513513
table_id: TableId,
514514
column_schemas: Vec<ColumnSchema>,
515-
default_values: Vec<AlgebraicValue>,
515+
mut default_values: Vec<AlgebraicValue>,
516516
) -> Result<TableId> {
517+
let original_table_schema = Arc::unwrap_or_clone(self.schema_for_table(table_id)?);
518+
519+
// Remove default values for columns that already exist in the schema.
520+
let remove_from_start = column_schemas.len().saturating_sub(original_table_schema.columns.len());
521+
default_values.drain(..remove_from_start);
522+
517523
let ((tx_table, ..), _) = self.get_or_create_insert_table_mut(table_id)?;
524+
518525
tx_table
519526
.validate_add_columns_schema(&column_schemas, &default_values)
520527
.map_err(TableError::from)?;
@@ -529,15 +536,6 @@ impl MutTxId {
529536
.map(|r| r.to_product_value())
530537
.collect();
531538

532-
// Intentionally using `schema_for_table_raw` instead of `schema_for_table`.
533-
//
534-
// Rationale:
535-
// - `schema_for_table_raw` queries system tables directly, ensuring the
536-
// most up-to-date schema.
537-
// - `schema_for_table` relies on an in-memory cache, which may not reflect
538-
// the latest sequence updates and can therefore return stale schemas.
539-
let original_table_schema = self.schema_for_table_raw(table_id)?;
540-
541539
log::debug!(
542540
"ADDING TABLE COLUMN (incompatible layout): {}, table_id: {}",
543541
original_table_schema.table_name,

‎crates/sats/src/algebraic_type.rs‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,8 +445,42 @@ impl AlgebraicType {
445445
_ => true,
446446
}
447447
}
448-
}
449448

449+
pub fn type_check(&self, value: &AlgebraicValue) -> bool {
450+
match value {
451+
AlgebraicValue::Min | AlgebraicValue::Max => return false,
452+
_ => {}
453+
}
454+
455+
match self {
456+
AlgebraicType::String => matches!(value, AlgebraicValue::String(_)),
457+
AlgebraicType::Bool => matches!(value, AlgebraicValue::Bool(_)),
458+
AlgebraicType::I8 => matches!(value, AlgebraicValue::I8(_)),
459+
AlgebraicType::U8 => matches!(value, AlgebraicValue::U8(_)),
460+
AlgebraicType::I16 => matches!(value, AlgebraicValue::I16(_)),
461+
AlgebraicType::U16 => matches!(value, AlgebraicValue::U16(_)),
462+
AlgebraicType::I32 => matches!(value, AlgebraicValue::I32(_)),
463+
AlgebraicType::U32 => matches!(value, AlgebraicValue::U32(_)),
464+
AlgebraicType::I64 => matches!(value, AlgebraicValue::I64(_)),
465+
AlgebraicType::U64 => matches!(value, AlgebraicValue::U64(_)),
466+
AlgebraicType::I128 => matches!(value, AlgebraicValue::I128(_)),
467+
AlgebraicType::U128 => matches!(value, AlgebraicValue::U128(_)),
468+
AlgebraicType::I256 => matches!(value, AlgebraicValue::I256(_)),
469+
AlgebraicType::U256 => matches!(value, AlgebraicValue::U256(_)),
470+
AlgebraicType::F32 => matches!(value, AlgebraicValue::F32(_)),
471+
AlgebraicType::F64 => matches!(value, AlgebraicValue::F64(_)),
472+
473+
AlgebraicType::Sum(sum_ty) => sum_ty.type_check(value),
474+
AlgebraicType::Product(product_ty) => product_ty.type_check(value),
475+
AlgebraicType::Array(element_ty) => element_ty.type_check(value),
476+
477+
AlgebraicType::Ref(_) => {
478+
// We cannot type check a Ref without the Typespace context
479+
false
480+
}
481+
}
482+
}
483+
}
450484
#[cfg(test)]
451485
mod tests {
452486
use super::AlgebraicType;
@@ -456,7 +490,7 @@ mod tests {
456490
algebraic_type::fmt::fmt_algebraic_type, algebraic_type::map_notation::fmt_algebraic_type as fmt_map,
457491
algebraic_type_ref::AlgebraicTypeRef, typespace::Typespace,
458492
};
459-
use crate::{ValueWithType, WithTypespace};
493+
use crate::{product, AlgebraicValue, ValueWithType, WithTypespace};
460494

461495
#[test]
462496
fn never() {
@@ -702,4 +736,22 @@ mod tests {
702736
assert!(AlgebraicType::time_duration().is_special());
703737
assert!(AlgebraicType::time_duration().is_time_duration());
704738
}
739+
740+
#[test]
741+
fn type_check() {
742+
let av = AlgebraicValue::sum(1, AlgebraicValue::from(product![0u16, 1u32]));
743+
let at = AlgebraicType::sum([
744+
("a", AlgebraicType::U8),
745+
(
746+
"b",
747+
AlgebraicType::Product(crate::ProductType::new(
748+
[AlgebraicType::U16.into(), AlgebraicType::U32.into()]
749+
.to_vec()
750+
.into_boxed_slice(),
751+
)),
752+
),
753+
]);
754+
755+
at.type_check(&av);
756+
}
705757
}

‎crates/sats/src/array_type.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,14 @@ impl MetaType for ArrayType {
2323
AlgebraicType::ZERO_REF
2424
}
2525
}
26+
27+
impl ArrayType {
28+
pub fn type_check(&self, value: &crate::AlgebraicValue) -> bool {
29+
match value {
30+
crate::AlgebraicValue::Array(array_value) => {
31+
array_value.iter_cloned().all(|elem| self.elem_ty.type_check(&elem))
32+
}
33+
_ => false,
34+
}
35+
}
36+
}

‎crates/sats/src/product_type.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,21 @@ impl ProductType {
198198
Ok(AlgebraicType::product(fields.into_boxed_slice()))
199199
}
200200
}
201+
202+
pub fn type_check(&self, value: &AlgebraicValue) -> bool {
203+
match value {
204+
AlgebraicValue::Product(pv) => {
205+
if pv.elements.len() != self.elements.len() {
206+
return false;
207+
}
208+
self.elements
209+
.iter()
210+
.zip(&pv.elements)
211+
.all(|(ty, val)| ty.algebraic_type.type_check(val))
212+
}
213+
_ => false,
214+
}
215+
}
201216
}
202217

203218
impl<I: Into<ProductTypeElement>> FromIterator<I> for ProductType {

‎crates/sats/src/sum_type.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,19 @@ impl SumType {
173173
pub fn get_variant_by_tag(&self, tag: u8) -> Option<&SumTypeVariant> {
174174
self.variants.get(tag as usize)
175175
}
176+
177+
pub fn type_check(&self, value: &AlgebraicValue) -> bool {
178+
match value {
179+
AlgebraicValue::Sum(sv) => {
180+
if let Some(variant) = self.get_variant_by_tag(sv.tag) {
181+
variant.algebraic_type.type_check(&sv.value)
182+
} else {
183+
false
184+
}
185+
}
186+
_ => false,
187+
}
188+
}
176189
}
177190

178191
impl From<Box<[SumTypeVariant]>> for SumType {

‎crates/table/src/table.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ impl Table {
453453
let default_value = default_values
454454
.get(idx)
455455
.ok_or_else(|| make_err(AddColumnsErrorReason::DefaultValueMissing(new_col.col_pos)))?;
456-
if Some(new_col.col_type.clone()) != default_value.type_of() {
456+
if !new_col.col_type.type_check(default_value) {
457457
return Err(make_err(AddColumnsErrorReason::DefaultValueTypeNotMatch(
458458
new_col.col_pos,
459459
)));

0 commit comments

Comments
 (0)