Skip to content

Commit

Permalink
Cache the 'row_type' on TableSchema so is not re-build in each access (
Browse files Browse the repository at this point in the history
  • Loading branch information
mamcx authored Dec 8, 2023
1 parent f276b25 commit 683a803
Show file tree
Hide file tree
Showing 12 changed files with 112 additions and 74 deletions.
8 changes: 4 additions & 4 deletions crates/cli/src/subcommands/generate/csharp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ fn autogen_csharp_product_table_common(
if let Some(schema) = &schema {
let constraints = schema.column_constraints();
// Declare custom index dictionaries
for col in &schema.columns {
for col in schema.columns() {
let field_name = col.col_name.replace("r#", "").to_case(Case::Pascal);
if !constraints[&NonEmpty::new(col.col_pos)].has_unique() {
continue;
Expand Down Expand Up @@ -694,7 +694,7 @@ fn autogen_csharp_product_table_common(
{
indent_scope!(output);
writeln!(output, "var val = ({name})insertedValue;").unwrap();
for col in &schema.columns {
for col in schema.columns() {
let field_name = col.col_name.replace("r#", "").to_case(Case::Pascal);
if !constraints[&NonEmpty::new(col.col_pos)].has_unique() {
continue;
Expand All @@ -714,7 +714,7 @@ fn autogen_csharp_product_table_common(
{
indent_scope!(output);
writeln!(output, "var val = ({name})deletedValue;").unwrap();
for col in &schema.columns {
for col in schema.columns() {
let field_name = col.col_name.replace("r#", "").to_case(Case::Pascal);
if !constraints[&NonEmpty::new(col.col_pos)].has_unique() {
continue;
Expand Down Expand Up @@ -970,7 +970,7 @@ fn autogen_csharp_access_funcs_for_struct(
});

let constraints = schema.column_constraints();
for col in &schema.columns {
for col in schema.columns() {
let is_unique = constraints[&NonEmpty::new(col.col_pos)].has_unique();

let col_i: usize = col.col_pos.into();
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/subcommands/generate/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ fn print_table_filter_methods(ctx: &GenCtx, out: &mut Indenter, table_type_name:
out.delimited_block(
"{",
|out| {
for field in &table.columns {
for field in table.columns() {
let field_name = field.col_name.to_case(Case::Snake);
// TODO: ensure that fields are PartialEq
writeln!(out, "{}", ALLOW_UNUSED).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/subcommands/generate/typescript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ fn autogen_typescript_access_funcs_for_struct(
writeln!(output).unwrap();

let constraints = table.column_constraints();
for col in &table.columns {
for col in table.columns() {
let is_unique = constraints[&NonEmpty::new(col.col_pos)].has_unique();
let field = &product_type.elements[usize::from(col.col_pos)];
let field_name = field.name.as_ref().expect("autogen'd tuples should have field names");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl BTreeIndex {
cols: self
.cols
.iter()
.map(|&x| table.schema.columns[usize::from(x)].col_name.clone())
.map(|&x| table.schema.columns()[usize::from(x)].col_name.clone())
.collect(),
value,
}
Expand Down
63 changes: 31 additions & 32 deletions crates/core/src/db/datastore/locking_tx_datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl CommittedState {
// Insert the columns into `st_columns`
let st_columns = self.get_or_create_table(ST_COLUMNS_ID, st_columns_schema());

for col in system_tables().into_iter().flat_map(|x| x.columns) {
for col in system_tables().into_iter().flat_map(|x| x.columns().to_vec()) {
let row = StColumnRow {
table_id: col.table_id,
col_pos: col.col_pos,
Expand Down Expand Up @@ -560,16 +560,16 @@ impl CommittedState {
indexes.push(index_schema);
}

Ok(Cow::Owned(TableSchema {
Ok(Cow::Owned(TableSchema::new(
table_id,
table_name,
columns,
indexes,
constraints,
sequences,
table_type: el.table_type,
table_access: el.table_access,
}))
el.table_type,
el.table_access,
)))
}

fn get_schema(&self, table_id: &TableId) -> Option<&TableSchema> {
Expand Down Expand Up @@ -1021,7 +1021,7 @@ impl MutTxId {
let table_schema = table_schema.into_schema(table_id);

// Insert the columns into `st_columns`
for col in &table_schema.columns {
for col in table_schema.columns() {
let row = StColumnRow {
table_id,
col_pos: col.col_pos,
Expand Down Expand Up @@ -1068,7 +1068,7 @@ impl MutTxId {
// Fetch the `ProductType` from the in memory table if it exists.
// The `ProductType` is invalidated if the schema of the table changes.
if let Some(row_type) = self.get_row_type(&table_id) {
return Ok(Cow::Owned(row_type));
return Ok(Cow::Borrowed(row_type));
}

// Look up the columns for the table in question.
Expand All @@ -1077,9 +1077,10 @@ impl MutTxId {
// representation of a table. This would happen in situations where
// we have created the table in the database, but have not yet
// represented in memory or inserted any rows into it.
Ok(Cow::Owned(
self.schema_for_table(table_id, database_address)?.get_row_type(),
))
Ok(match self.schema_for_table(table_id, database_address)? {
Cow::Borrowed(x) => Cow::Borrowed(x.get_row_type()),
Cow::Owned(x) => Cow::Owned(x.into_row_type()),
})
}

// NOTE: It is essential to keep this function in sync with the
Expand Down Expand Up @@ -1192,16 +1193,16 @@ impl MutTxId {
indexes.push(index_schema);
}

Ok(Cow::Owned(TableSchema {
Ok(Cow::Owned(TableSchema::new(
table_id,
table_name,
columns,
indexes,
constraints,
sequences,
table_type: el.table_type,
table_access: el.table_access,
}))
el.table_type,
el.table_access,
)))
}

fn drop_table(&mut self, table_id: TableId, database_address: Address) -> super::Result<()> {
Expand Down Expand Up @@ -1500,7 +1501,7 @@ impl MutTxId {

if let Some((col_id, sequence_id)) = col_to_update {
let col_idx = col_id.idx();
let col = &schema.columns[col_idx];
let col = &schema.columns()[col_idx];
if !Self::algebraic_type_is_numeric(&col.col_type) {
return Err(SequenceError::NotInteger {
col: format!("{}.{}", &schema.table_name, &col.col_name),
Expand Down Expand Up @@ -1609,7 +1610,7 @@ impl MutTxId {
let insert_table = self.tx_state.get_insert_table_mut(&table_id).unwrap();

// TODO(cloutiertyler): should probably also check that all the columns are correct? Perf considerations.
if insert_table.schema.columns.len() != row.elements.len() {
if insert_table.schema.columns().len() != row.elements.len() {
return Err(TableError::RowInvalidType { table_id, row }.into());
}

Expand Down Expand Up @@ -1648,7 +1649,7 @@ impl MutTxId {
.map(|row| DataRef::new(row_id, row)))
}

fn get_row_type(&self, table_id: &TableId) -> Option<ProductType> {
fn get_row_type(&self, table_id: &TableId) -> Option<&ProductType> {
if let Some(row_type) = self
.tx_state
.insert_tables
Expand Down Expand Up @@ -1893,7 +1894,7 @@ impl Locking {
Operation::Insert => {
let row_type = schema.get_row_type();
let product_value = match write.data_key {
DataKey::Data(data) => ProductValue::decode(&row_type, &mut &data[..]).unwrap_or_else(|e| {
DataKey::Data(data) => ProductValue::decode(row_type, &mut &data[..]).unwrap_or_else(|e| {
panic!(
"Couldn't decode product value from message log: `{}`. Expected row type: {:?}",
e, row_type
Expand All @@ -1903,7 +1904,7 @@ impl Locking {
let data = odb.get(hash).unwrap_or_else(|| {
panic!("Object {hash} referenced from transaction not present in object DB");
});
ProductValue::decode(&row_type, &mut &data[..]).unwrap_or_else(|e| {
ProductValue::decode(row_type, &mut &data[..]).unwrap_or_else(|e| {
panic!(
"Couldn't decode product value {} from object DB: `{}`. Expected row type: {:?}",
hash, e, row_type
Expand Down Expand Up @@ -2975,29 +2976,27 @@ mod tests {
.unwrap()
}

#[rustfmt::skip]
fn basic_table_schema_created(table_id: TableId) -> TableSchema {
TableSchema {
TableSchema::new(
table_id,
table_name: "Foo".into(),
#[rustfmt::skip]
columns: map_array(basic_table_schema_cols()),
#[rustfmt::skip]
indexes: map_array([
"Foo".into(),
map_array(basic_table_schema_cols()),
map_array([
IdxSchema { id: 6, table: 6, col: 0, name: "id_idx", unique: true },
IdxSchema { id: 7, table: 6, col: 1, name: "name_idx", unique: true },
]),
#[rustfmt::skip]
constraints: map_array([

map_array([
ConstraintRow { constraint_id: 6, table_id: 6, columns: col(0), constraints: Constraints::indexed(), constraint_name: "ct_Foo_id_idx_indexed" },
ConstraintRow { constraint_id: 7, table_id: 6, columns: col(1), constraints: Constraints::indexed(), constraint_name: "ct_Foo_name_idx_indexed" }
]),
#[rustfmt::skip]
sequences: map_array([
map_array([
SequenceRow { id: 4, table: 6, col_pos: 0, name: "seq_Foo_id", start: 1 }
]),
table_type: StTableType::User,
table_access: StAccess::Public,
}
StTableType::User,
StAccess::Public,
)
}

fn setup_table() -> ResultTest<(Locking, MutTxId, TableId)> {
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/db/datastore/locking_tx_datastore/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl Table {
self.rows.get(row_id)
}

pub(crate) fn get_row_type(&self) -> ProductType {
pub(crate) fn get_row_type(&self) -> &ProductType {
self.schema.get_row_type()
}

Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/db/relational_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ mod tests {
stdb.create_table(&mut tx, schema)?;
let table_id = stdb.table_id_from_name(&tx, "MyTable")?.unwrap();
let schema = stdb.schema_for_table(&tx, table_id)?;
let col = schema.columns.iter().find(|x| x.col_name == "my_col").unwrap();
let col = schema.columns().iter().find(|x| x.col_name == "my_col").unwrap();
assert_eq!(col.col_pos, 0.into());
Ok(())
}
Expand Down
20 changes: 10 additions & 10 deletions crates/core/src/db/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,21 +257,21 @@ mod tests {

#[test]
fn test_updates_new_table() -> anyhow::Result<()> {
let current = vec![Cow::Owned(TableSchema {
table_id: TableId(42),
table_name: "Person".into(),
columns: vec![ColumnSchema {
let current = vec![Cow::Owned(TableSchema::new(
TableId(42),
"Person".into(),
vec![ColumnSchema {
table_id: TableId(42),
col_pos: ColId(0),
col_name: "name".into(),
col_type: AlgebraicType::String,
}],
indexes: vec![],
constraints: vec![],
sequences: vec![],
table_type: StTableType::User,
table_access: StAccess::Public,
})];
vec![],
vec![],
vec![],
StTableType::User,
StAccess::Public,
))];
let proposed = vec![
TableDef::new(
"Person".into(),
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/sql/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl From {
pub fn find_field(&self, f: &str) -> Result<Vec<FieldDef>, RelationError> {
let field = extract_table_field(f)?;
let fields = self.iter_tables().flat_map(|t| {
t.columns.iter().filter_map(|column| {
t.columns().iter().filter_map(|column| {
if column.col_name == field.field {
Some(FieldDef {
column: column.clone(),
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/sql/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ fn compile_select(table: From, project: Vec<Column>, selection: Option<Selection
},
Column::QualifiedWildcard { table: name } => {
if let Some(t) = table.iter_tables().find(|x| x.table_name == name) {
for c in t.columns.iter() {
for c in t.columns().iter() {
col_ids.push(FieldName::named(&t.table_name, &c.col_name).into());
}
qualified_wildcards.push(t.table_id);
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/sql/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ pub(crate) mod tests {
let t = db.table_id_from_name(tx, table_name)?.unwrap();
let t = db.schema_for_table(tx, t)?;

let col = t.columns.first().unwrap();
let col = t.columns().first().unwrap();
let idx = t.indexes.first().map(|x| x.is_unique);
let column_auto_inc = t
.constraints
Expand Down
Loading

0 comments on commit 683a803

Please sign in to comment.