Skip to content

Commit

Permalink
refactor!: ResourceId -> ast::ObjectName
Browse files Browse the repository at this point in the history
  • Loading branch information
varshith257 committed Dec 22, 2024
1 parent 91173a3 commit 79fc62f
Show file tree
Hide file tree
Showing 22 changed files with 85 additions and 71 deletions.
10 changes: 10 additions & 0 deletions crates/proof-of-sql-parser/src/sqlparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ impl From<ResourceId> for ObjectName {
}
}

/// Converts a dot-separated string into an `ObjectName`.
#[must_use]
pub fn object_name_from(s: &str) -> ObjectName {
ObjectName(
s.split('.')
.map(|part| Ident::new(part.to_string()))
.collect(),
)
}

impl From<TableExpression> for TableFactor {
fn from(table: TableExpression) -> Self {
match table {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl<C: Commitment> ColumnCommitments<C> {
ColumnCommitmentMetadataMap::from_column_fields_with_max_bounds(columns);
let commitments = columns
.iter()
.map(|c| accessor.get_commitment(ColumnRef::new(table, c.name(), c.data_type())))
.map(|c| accessor.get_commitment(ColumnRef::new(table.clone(), c.name(), c.data_type())))
.collect();
ColumnCommitments {
commitments,
Expand Down
6 changes: 3 additions & 3 deletions crates/proof-of-sql/src/base/commitment/query_commitments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ impl<C: Commitment> QueryCommitmentsExt<C> for QueryCommitments<C> {
.into_iter()
.map(|(table_ref, columns)| {
(
table_ref,
table_ref.clone(),
TableCommitment::from_accessor_with_max_bounds(
table_ref,
table_ref.clone(),
accessor
.lookup_schema(table_ref)
.lookup_schema(table_ref.clone())
.iter()
.filter_map(|c| {
columns.iter().find(|x| x.name() == c.0.clone()).cloned()
Expand Down
4 changes: 2 additions & 2 deletions crates/proof-of-sql/src/base/commitment/table_commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ impl<C: Commitment> TableCommitment<C> {
columns: &[ColumnField],
accessor: &impl CommitmentAccessor<C>,
) -> Self {
let length = accessor.get_length(table_ref);
let offset = accessor.get_offset(table_ref);
let length = accessor.get_length(table_ref.clone());
let offset = accessor.get_offset(table_ref.clone());
Self::try_new(
ColumnCommitments::from_accessor_with_max_bounds(table_ref, columns, accessor),
offset..offset + length,
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/base/database/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl ColumnRef {
/// Returns the table reference of this column
#[must_use]
pub fn table_ref(&self) -> TableRef {
self.table_ref
self.table_ref.clone()
}

/// Returns the column identifier of this column
Expand Down
24 changes: 12 additions & 12 deletions crates/proof-of-sql/src/base/database/table_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,52 @@ use core::{
fmt::{Display, Formatter},
str::FromStr,
};
use proof_of_sql_parser::{impl_serde_from_str, ResourceId};
use sqlparser::ast::Ident;
use proof_of_sql_parser::{impl_serde_from_str, sqlparser::object_name_from};
use sqlparser::ast::{Ident, ObjectName};

/// Expression for an SQL table
#[derive(Debug, Clone, PartialEq, Eq, Hash, Copy)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct TableRef {
resource_id: ResourceId,
object_name: ObjectName,
}

impl TableRef {
/// Creates a new table reference from a resource id
#[must_use]
pub fn new(resource_id: ResourceId) -> Self {
Self { resource_id }
pub fn new(object_name: ObjectName) -> Self {
Self { object_name: object_name.clone() }
}

/// Returns the identifier of the schema
#[must_use]
pub fn schema_id(&self) -> Ident {
self.resource_id.schema().into()
self.object_name.0.get(0).clone()
}

/// Returns the identifier of the table
#[must_use]
pub fn table_id(&self) -> Ident {
self.resource_id.object_name().into()
self.object_name.0.get(1).clone()
}

/// Returns the underlying resource id of the table
#[must_use]
pub fn resource_id(&self) -> ResourceId {
self.resource_id
pub fn object_name(&self) -> ObjectName {
self.object_name.clone()
}
}

impl FromStr for TableRef {
type Err = proof_of_sql_parser::ParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(Self::new(s.parse()?))
Ok(Self::new(object_name_from(s)))
}
}

impl Display for TableRef {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
self.resource_id.fmt(f)
self.object_name.clone().fmt(f)
}
}

Expand Down
18 changes: 10 additions & 8 deletions crates/proof-of-sql/src/base/database/test_schema_accessor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::{ColumnType, SchemaAccessor, TableRef};
use crate::base::map::IndexMap;
use sqlparser::ast::Ident;

/// A simple in-memory `SchemaAccessor` for testing intermediate AST -> Provable AST conversion.
pub struct TestSchemaAccessor {
schemas: IndexMap<TableRef, IndexMap<Ident, ColumnType>>,
Expand Down Expand Up @@ -32,10 +33,11 @@ impl SchemaAccessor for TestSchemaAccessor {
mod tests {
use super::*;
use crate::base::map::indexmap;
use proof_of_sql_parser::sqlparser::object_name_from;

fn sample_test_schema_accessor() -> TestSchemaAccessor {
let table1: TableRef = TableRef::new("schema.table1".parse().unwrap());
let table2: TableRef = TableRef::new("schema.table2".parse().unwrap());
let table1: TableRef = TableRef::new(object_name_from("schema.table1"));
let table2: TableRef = TableRef::new(object_name_from("schema.table2"));
TestSchemaAccessor::new(indexmap! {
table1 => indexmap! {
"col1".into() => ColumnType::BigInt,
Expand All @@ -50,9 +52,9 @@ mod tests {
#[test]
fn test_lookup_column() {
let accessor = sample_test_schema_accessor();
let table1: TableRef = TableRef::new("schema.table1".parse().unwrap());
let table2: TableRef = TableRef::new("schema.table2".parse().unwrap());
let not_a_table: TableRef = TableRef::new("schema.not_a_table".parse().unwrap());
let table1: TableRef = TableRef::new(object_name_from("schema.table1"));
let table2: TableRef = TableRef::new(object_name_from("schema.table2"));
let not_a_table: TableRef = TableRef::new(object_name_from("schema.not_a_table"));
assert_eq!(
accessor.lookup_column(table1, "col1".into()),
Some(ColumnType::BigInt)
Expand All @@ -78,9 +80,9 @@ mod tests {
#[test]
fn test_lookup_schema() {
let accessor = sample_test_schema_accessor();
let table1: TableRef = TableRef::new("schema.table1".parse().unwrap());
let table2: TableRef = TableRef::new("schema.table2".parse().unwrap());
let not_a_table: TableRef = TableRef::new("schema.not_a_table".parse().unwrap());
let table1: TableRef = TableRef::new(object_name_from("schema.table1"));
let table2: TableRef = TableRef::new(object_name_from("schema.table2"));
let not_a_table: TableRef = TableRef::new(object_name_from("schema.not_a_table"));
assert_eq!(
accessor.lookup_schema(table1),
vec![
Expand Down
8 changes: 4 additions & 4 deletions crates/proof-of-sql/src/sql/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@ use alloc::{
string::{String, ToString},
};
use core::result::Result;
use proof_of_sql_parser::{posql_time::PoSQLTimestampError, ResourceId};
use proof_of_sql_parser::posql_time::PoSQLTimestampError;
use snafu::Snafu;
use sqlparser::ast::Ident;

/// Errors from converting an intermediate AST into a provable AST.
#[derive(Snafu, Debug, PartialEq, Eq)]
pub enum ConversionError {
#[snafu(display("Column '{identifier}' was not found in table '{resource_id}'"))]
#[snafu(display("Column '{identifier}' was not found in table '{object_name}'"))]
/// The column is missing in the table
MissingColumn {
/// The missing column identifier
identifier: Box<Ident>,
/// The table resource id
resource_id: Box<ResourceId>,
/// The table object name
object_name: String,
},

#[snafu(display("Column '{identifier}' was not found"))]
Expand Down
4 changes: 2 additions & 2 deletions crates/proof-of-sql/src/sql/parse/query_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl TryFrom<&QueryContext> for Option<GroupByExec> {
expression: "QueryContext has no table_ref".to_owned(),
},
)?;
let resource_id = table.table_ref.resource_id();
let object_name = table.table_ref.object_name();
let group_by_exprs = value
.group_by_exprs
.iter()
Expand All @@ -252,7 +252,7 @@ impl TryFrom<&QueryContext> for Option<GroupByExec> {
.get(expr)
.ok_or(ConversionError::MissingColumn {
identifier: Box::new((expr).clone()),
resource_id: Box::new(resource_id),
object_name: Box::new(object_name),
})
.map(|column_ref| ColumnExpr::new(column_ref.clone()))
})
Expand Down
10 changes: 5 additions & 5 deletions crates/proof-of-sql/src/sql/parse/query_context_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ use proof_of_sql_parser::{
AggregationOperator, AliasedResultExpr, Expression, Literal, OrderBy, SelectResultExpr,
Slice, TableExpression,
},
Identifier, ResourceId,
Identifier,
};
use sqlparser::ast::{BinaryOperator, UnaryOperator};
pub struct QueryContextBuilder<'a> {
context: QueryContext,
schema_accessor: &'a dyn SchemaAccessor,
}
use sqlparser::ast::Ident;
use sqlparser::ast::{Ident, ObjectName};

// Public interface
impl<'a> QueryContextBuilder<'a> {
Expand All @@ -44,7 +44,7 @@ impl<'a> QueryContextBuilder<'a> {
TableExpression::Named { table, schema } => {
let schema_identifier = schema.unwrap_or(default_schema);
self.context
.set_table_ref(TableRef::new(ResourceId::new(schema_identifier, table)));
.set_table_ref(TableRef::new(ObjectName::new(schema_identifier, table)));
}
}
self
Expand Down Expand Up @@ -109,7 +109,7 @@ impl<'a> QueryContextBuilder<'a> {
)]
fn lookup_schema(&self) -> Vec<(Ident, ColumnType)> {
let table_ref = self.context.get_table_ref();
let columns = self.schema_accessor.lookup_schema(*table_ref);
let columns = self.schema_accessor.lookup_schema(table_ref.clone());
assert!(!columns.is_empty(), "At least one column must exist");
columns
}
Expand Down Expand Up @@ -265,7 +265,7 @@ impl<'a> QueryContextBuilder<'a> {

let column_type = column_type.ok_or_else(|| ConversionError::MissingColumn {
identifier: Box::new(column_name.clone()),
resource_id: Box::new(table_ref.resource_id()),
object_name: Box::new(table_ref.object_name()),
})?;

let column = ColumnRef::new(*table_ref, column_name.clone(), column_type);
Expand Down
4 changes: 2 additions & 2 deletions crates/proof-of-sql/src/sql/parse/query_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl QueryExpr {
})
.collect::<Vec<_>>();
let filter = FilterExecBuilder::new(context.get_column_mapping())
.add_table_expr(*context.get_table_ref())
.add_table_expr(context.get_table_ref().clone())
.add_where_expr(context.get_where_expr().clone())?
.add_result_columns(&raw_enriched_exprs)
.build();
Expand Down Expand Up @@ -144,7 +144,7 @@ impl QueryExpr {
.map(|enriched_expr| enriched_expr.residue_expression.clone())
.collect::<Vec<_>>();
let filter = FilterExecBuilder::new(context.get_column_mapping())
.add_table_expr(*context.get_table_ref())
.add_table_expr(context.get_table_ref().clone())
.add_where_expr(context.get_where_expr().clone())?
.add_result_columns(&enriched_exprs)
.build();
Expand Down
8 changes: 4 additions & 4 deletions crates/proof-of-sql/src/sql/proof/query_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ fn get_index_range<'a>(
table_refs
.into_iter()
.map(|table_ref| {
let length = accessor.get_length(*table_ref);
let offset = accessor.get_offset(*table_ref);
let length = accessor.get_length(table_ref.clone());
let offset = accessor.get_offset(table_ref.clone());
(offset, offset + length)
})
.reduce(|(min_start, max_end), (start, end)| (min_start.min(start), max_end.max(end)))
Expand Down Expand Up @@ -99,7 +99,7 @@ impl<CP: CommitmentEvaluationProof> QueryProof<CP> {
.filter(|col_ref| col_ref.table_ref() == table_ref)
.cloned()
.collect();
(table_ref, accessor.get_table(table_ref, &col_refs))
(table_ref.clone(), accessor.get_table(table_ref, &col_refs))
})
.collect();

Expand Down Expand Up @@ -338,7 +338,7 @@ impl<CP: CommitmentEvaluationProof> QueryProof<CP> {
// Always prepend input lengths to the one evaluation lengths
let table_length_map = table_refs
.iter()
.map(|table_ref| (table_ref, accessor.get_length(*table_ref)))
.map(|table_ref| (table_ref, accessor.get_length(table_ref.clone())))
.collect::<IndexMap<_, _>>();

let one_evaluation_lengths = table_length_map
Expand Down
19 changes: 10 additions & 9 deletions crates/proof-of-sql/src/sql/proof/query_proof_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::{
use bumpalo::Bump;
use serde::Serialize;
use sqlparser::ast::Ident;
use proof_of_sql_parser::sqlparser::object_name_from;

/// Type to allow us to prove and verify an artificial polynomial where we prove
/// that every entry in the result is zero
Expand Down Expand Up @@ -112,7 +113,7 @@ impl ProofPlan for TrivialTestProofPlan {
indexset! {}
}
fn get_table_references(&self) -> IndexSet<TableRef> {
indexset! {TableRef::new("sxt.test".parse().unwrap())}
indexset! {TableRef::new(object_name_from("sxt.test"))}
}
}

Expand Down Expand Up @@ -278,7 +279,7 @@ impl ProverEvaluate for SquareTestProofPlan {
table_map: &IndexMap<TableRef, Table<'a, S>>,
) -> Table<'a, S> {
let x = *table_map
.get(&TableRef::new("sxt.test".parse().unwrap()))
.get(&TableRef::new(object_name_from("sxt.test")))
.unwrap()
.inner_table()
.get(&Ident::new("x"))
Expand Down Expand Up @@ -333,7 +334,7 @@ impl ProofPlan for SquareTestProofPlan {
)}
}
fn get_table_references(&self) -> IndexSet<TableRef> {
indexset! {TableRef::new("sxt.test".parse().unwrap())}
indexset! {TableRef::new(object_name_from("sxt.test"))}
}
}

Expand Down Expand Up @@ -457,7 +458,7 @@ impl ProverEvaluate for DoubleSquareTestProofPlan {
table_map: &IndexMap<TableRef, Table<'a, S>>,
) -> Table<'a, S> {
let x = *table_map
.get(&TableRef::new("sxt.test".parse().unwrap()))
.get(&TableRef::new(object_name_from("sxt.test")))
.unwrap()
.inner_table()
.get(&Ident::new("x"))
Expand Down Expand Up @@ -534,7 +535,7 @@ impl ProofPlan for DoubleSquareTestProofPlan {
)}
}
fn get_table_references(&self) -> IndexSet<TableRef> {
indexset! {TableRef::new("sxt.test".parse().unwrap())}
indexset! {TableRef::new(object_name_from("sxt.test"))}
}
}

Expand Down Expand Up @@ -669,7 +670,7 @@ impl ProverEvaluate for ChallengeTestProofPlan {
table_map: &IndexMap<TableRef, Table<'a, S>>,
) -> Table<'a, S> {
let x = *table_map
.get(&TableRef::new("sxt.test".parse().unwrap()))
.get(&TableRef::new(object_name_from("sxt.test")))
.unwrap()
.inner_table()
.get(&Ident::new("x"))
Expand Down Expand Up @@ -727,7 +728,7 @@ impl ProofPlan for ChallengeTestProofPlan {
)}
}
fn get_table_references(&self) -> IndexSet<TableRef> {
indexset! {TableRef::new("sxt.test".parse().unwrap())}
indexset! {TableRef::new(object_name_from("sxt.test"))}
}
}

Expand Down Expand Up @@ -811,7 +812,7 @@ impl ProverEvaluate for FirstRoundSquareTestProofPlan {
table_map: &IndexMap<TableRef, Table<'a, S>>,
) -> Table<'a, S> {
let x = *table_map
.get(&TableRef::new("sxt.test".parse().unwrap()))
.get(&TableRef::new(object_name_from("sxt.test")))
.unwrap()
.inner_table()
.get(&Ident::new("x"))
Expand Down Expand Up @@ -868,7 +869,7 @@ impl ProofPlan for FirstRoundSquareTestProofPlan {
)}
}
fn get_table_references(&self) -> IndexSet<TableRef> {
indexset! {TableRef::new("sxt.test".parse().unwrap())}
indexset! {TableRef::new(object_name_from("sxt.test"))}
}
}

Expand Down
Loading

0 comments on commit 79fc62f

Please sign in to comment.