From cff95231d1c521c6f91909046bf987ce1a1a01e9 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Wed, 18 Dec 2024 08:43:23 +0530 Subject: [PATCH] refactor!: identifier to ident and update docs --- crates/proof-of-sql/README.md | 2 +- .../benches/scaffold/random_util.rs | 1 - .../examples/hello_world/README.md | 2 +- crates/proof-of-sql/examples/posql_db/main.rs | 9 ++- .../base/arrow/owned_and_arrow_conversions.rs | 10 ++-- .../src/base/arrow/record_batch_conversion.rs | 4 +- .../column_commitment_metadata_map.rs | 16 +++--- .../src/base/commitment/column_commitments.rs | 41 +++++++------- .../proof-of-sql/src/base/commitment/mod.rs | 4 +- .../src/base/commitment/table_commitment.rs | 30 +++++----- .../src/base/database/owned_table.rs | 2 +- .../src/base/database/owned_table_utility.rs | 49 +++++----------- .../proof-of-sql/src/base/database/table.rs | 6 +- .../src/base/database/table_utility.rs | 56 ++++++------------- .../src/sql/parse/query_expr_tests.rs | 2 +- .../src/sql/postprocessing/error.rs | 4 +- .../postprocessing/group_by_postprocessing.rs | 2 +- .../group_by_postprocessing_test.rs | 2 +- .../src/sql/proof_exprs/test_utility.rs | 4 -- 19 files changed, 99 insertions(+), 147 deletions(-) diff --git a/crates/proof-of-sql/README.md b/crates/proof-of-sql/README.md index a63417d5c..050408222 100644 --- a/crates/proof-of-sql/README.md +++ b/crates/proof-of-sql/README.md @@ -90,7 +90,7 @@ Parsing Query... 1.870256ms Generating Proof... 467.45371ms Verifying Proof... 7.106864ms Valid proof! -Query result: OwnedTable { table: {Identifier { name: "b" }: VarChar(["hello", "world"])} } +Query result: OwnedTable { table: {Ident { value: "b", quote_style: None }: VarChar(["hello", "world"])} } ``` For a detailed explanation of the example and its implementation, refer to the [README](https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/crates/proof-of-sql/examples/hello_world/README.md) and source code in [hello_world/main.rs](https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/crates/proof-of-sql/examples/hello_world/main.rs). diff --git a/crates/proof-of-sql/benches/scaffold/random_util.rs b/crates/proof-of-sql/benches/scaffold/random_util.rs index cdc42775d..c13b8db2e 100644 --- a/crates/proof-of-sql/benches/scaffold/random_util.rs +++ b/crates/proof-of-sql/benches/scaffold/random_util.rs @@ -10,7 +10,6 @@ pub type OptionalRandBound = Option i64>; /// # Panics /// /// Will panic if: -/// - The provided identifier cannot be parsed into an `Identifier` type. /// - An unsupported `ColumnType` is encountered, triggering a panic in the `todo!()` macro. #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] pub fn generate_random_columns<'a, S: Scalar>( diff --git a/crates/proof-of-sql/examples/hello_world/README.md b/crates/proof-of-sql/examples/hello_world/README.md index 9e8e3885c..342f29494 100644 --- a/crates/proof-of-sql/examples/hello_world/README.md +++ b/crates/proof-of-sql/examples/hello_world/README.md @@ -30,5 +30,5 @@ Parsing Query... 1.870256ms Generating Proof... 467.45371ms Verifying Proof... 7.106864ms Valid proof! -Query result: OwnedTable { table: {Identifier { name: "b" }: VarChar(["hello", "world"])} } +Query result: OwnedTable { table: {Ident { value: "b", quote_style: None }: VarChar(["hello", "world"])} } ``` diff --git a/crates/proof-of-sql/examples/posql_db/main.rs b/crates/proof-of-sql/examples/posql_db/main.rs index d591651e2..8a4b86eac 100644 --- a/crates/proof-of-sql/examples/posql_db/main.rs +++ b/crates/proof-of-sql/examples/posql_db/main.rs @@ -25,7 +25,8 @@ use proof_of_sql::{ }, sql::{parse::QueryExpr, proof::VerifiableQueryResult}, }; -use proof_of_sql_parser::{Identifier, SelectStatement}; +use proof_of_sql_parser::SelectStatement; +use sqlparser::ast::Ident; use std::{ fs, io::{prelude::Write, stdout}, @@ -78,7 +79,7 @@ enum Commands { table: TableRef, /// The comma delimited column names of the table. #[arg(short, long, value_parser, num_args = 0.., value_delimiter = ',')] - columns: Vec, + columns: Vec, /// The comma delimited data types of the columns. #[arg(short, long, value_parser, num_args = 0.., value_delimiter = ',')] data_types: Vec, @@ -175,7 +176,9 @@ fn main() { columns .iter() .zip_eq(data_types.iter()) - .map(|(name, data_type)| Field::new(name.as_str(), data_type.into(), false)) + .map(|(name, data_type)| { + Field::new(name.value.as_str(), data_type.into(), false) + }) .collect::>(), ); let batch = RecordBatch::new_empty(Arc::new(schema)); diff --git a/crates/proof-of-sql/src/base/arrow/owned_and_arrow_conversions.rs b/crates/proof-of-sql/src/base/arrow/owned_and_arrow_conversions.rs index 3b92bf911..9e8df184a 100644 --- a/crates/proof-of-sql/src/base/arrow/owned_and_arrow_conversions.rs +++ b/crates/proof-of-sql/src/base/arrow/owned_and_arrow_conversions.rs @@ -49,10 +49,10 @@ pub enum OwnedArrowConversionError { /// The unsupported datatype datatype: DataType, }, - /// This error occurs when trying to convert from a record batch with duplicate identifiers (e.g. `"a"` and `"A"`). - #[snafu(display("conversion resulted in duplicate identifiers"))] - DuplicateIdentifiers, - /// This error occurs when convering from a record batch name to an identifier fails. (Which may my impossible.) + /// This error occurs when trying to convert from a record batch with duplicate idents(e.g. `"a"` and `"A"`). + #[snafu(display("conversion resulted in duplicate idents"))] + DuplicateIdents, + /// This error occurs when convering from a record batch name to an idents fails. (Which may my impossible.) #[snafu(transparent)] FieldParseFail { /// The underlying source error @@ -311,7 +311,7 @@ impl TryFrom for OwnedTable { if num_columns == owned_table.num_columns() { Ok(owned_table) } else { - Err(OwnedArrowConversionError::DuplicateIdentifiers) + Err(OwnedArrowConversionError::DuplicateIdents) } } } diff --git a/crates/proof-of-sql/src/base/arrow/record_batch_conversion.rs b/crates/proof-of-sql/src/base/arrow/record_batch_conversion.rs index 9dcfe89e1..a0221578e 100644 --- a/crates/proof-of-sql/src/base/arrow/record_batch_conversion.rs +++ b/crates/proof-of-sql/src/base/arrow/record_batch_conversion.rs @@ -57,7 +57,7 @@ impl TableCommitment { panic!("RecordBatches cannot have columns of mixed length") } Err(AppendTableCommitmentError::AppendColumnCommitments { - source: AppendColumnCommitmentsError::DuplicateIdentifiers { .. }, + source: AppendColumnCommitmentsError::DuplicateIdents { .. }, }) => { panic!("RecordBatches cannot have duplicate identifiers") } @@ -92,7 +92,7 @@ impl TableCommitment { Err(TableCommitmentFromColumnsError::MixedLengthColumns { .. }) => { panic!("RecordBatches cannot have columns of mixed length") } - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) => { + Err(TableCommitmentFromColumnsError::DuplicateIdents { .. }) => { panic!("RecordBatches cannot have duplicate identifiers") } } diff --git a/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs b/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs index 3e2a41ef3..e00cadb40 100644 --- a/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs +++ b/crates/proof-of-sql/src/base/commitment/column_commitment_metadata_map.rs @@ -7,7 +7,7 @@ use alloc::string::{String, ToString}; use snafu::Snafu; use sqlparser::ast::Ident; -/// Mapping of column identifiers to column metadata used to associate metadata with commitments. +/// Mapping of column idents to column metadata used to associate metadata with commitments. pub type ColumnCommitmentMetadataMap = IndexMap; /// During commitment operation, metadata indicates that operand tables cannot be the same. @@ -22,16 +22,14 @@ pub enum ColumnCommitmentsMismatch { /// Commitments with different column counts cannot operate with each other. #[snafu(display("commitments with different column counts cannot operate with each other"))] NumColumns, - /// Columns with mismatched identifiers cannot operate with each other. + /// Columns with mismatched idents cannot operate with each other. /// - /// Strings are used here instead of Identifiers to decrease the size of this variant - #[snafu(display( - "column with identifier {id_a} cannot operate with column with identifier {id_b}" - ))] + /// Strings are used here instead of Idents to decrease the size of this variant + #[snafu(display("column with ident {id_a} cannot operate with column with ident {id_b}"))] Ident { - /// The first column identifier + /// The first column ident id_a: String, - /// The second column identifier + /// The second column ident id_b: String, }, } @@ -42,7 +40,7 @@ pub trait ColumnCommitmentMetadataMapExt { /// the widest possible bounds for the column type. fn from_column_fields_with_max_bounds(columns: &[ColumnField]) -> Self; - /// Construct this mapping from an iterator of column identifiers and columns. + /// Construct this mapping from an iterator of column ident and columns. fn from_columns<'a>( columns: impl IntoIterator)>, ) -> Self diff --git a/crates/proof-of-sql/src/base/commitment/column_commitments.rs b/crates/proof-of-sql/src/base/commitment/column_commitments.rs index 7f4ceb6bb..b2eae03db 100644 --- a/crates/proof-of-sql/src/base/commitment/column_commitments.rs +++ b/crates/proof-of-sql/src/base/commitment/column_commitments.rs @@ -16,10 +16,10 @@ use serde::{Deserialize, Serialize}; use snafu::Snafu; use sqlparser::ast::Ident; -/// Cannot create commitments with duplicate identifier. +/// Cannot create commitments with duplicate ident. #[derive(Debug, Snafu)] -#[snafu(display("cannot create commitments with duplicate identifier: {id}"))] -pub struct DuplicateIdentifiers { +#[snafu(display("cannot create commitments with duplicate ident: {id}"))] +pub struct DuplicateIdents { id: String, } @@ -32,11 +32,11 @@ pub enum AppendColumnCommitmentsError { /// The underlying source error source: ColumnCommitmentsMismatch, }, - /// New columns have duplicate identifiers. + /// New columns have duplicate idents. #[snafu(transparent)] - DuplicateIdentifiers { + DuplicateIdents { /// The underlying source error - source: DuplicateIdentifiers, + source: DuplicateIdents, }, } @@ -97,7 +97,7 @@ impl ColumnCommitments { self.column_metadata.is_empty() } - /// Returns the commitment with the given identifier. + /// Returns the commitment with the given ident. #[must_use] pub fn get_commitment(&self, identifier: &Ident) -> Option { self.column_metadata @@ -105,7 +105,7 @@ impl ColumnCommitments { .map(|index| self.commitments[index].clone()) } - /// Returns the metadata for the commitment with the given identifier. + /// Returns the metadata for the commitment with the given ident. #[must_use] pub fn get_metadata(&self, identifier: &Ident) -> Option<&ColumnCommitmentMetadata> { self.column_metadata.get(identifier) @@ -121,11 +121,11 @@ impl ColumnCommitments { columns: impl IntoIterator, offset: usize, setup: &C::PublicSetup<'_>, - ) -> Result, DuplicateIdentifiers> + ) -> Result, DuplicateIdents> where COL: Into>, { - // Check for duplicate identifiers + // Check for duplicate idents let mut unique_identifiers = IndexSet::default(); let unique_columns = columns .into_iter() @@ -133,7 +133,7 @@ impl ColumnCommitments { if unique_identifiers.insert(identifier) { Ok((identifier, column)) } else { - Err(DuplicateIdentifiers { + Err(DuplicateIdents { id: identifier.to_string(), }) } @@ -178,7 +178,7 @@ impl ColumnCommitments { where COL: Into>, { - // Check for duplicate identifiers. + // Check for duplicate idents. let mut unique_identifiers = IndexSet::default(); let unique_columns = columns .into_iter() @@ -186,7 +186,7 @@ impl ColumnCommitments { if unique_identifiers.insert(identifier) { Ok((identifier, column)) } else { - Err(DuplicateIdentifiers { + Err(DuplicateIdents { id: identifier.to_string(), }) } @@ -221,7 +221,7 @@ impl ColumnCommitments { columns: impl IntoIterator, offset: usize, setup: &C::PublicSetup<'_>, - ) -> Result<(), DuplicateIdentifiers> + ) -> Result<(), DuplicateIdents> where COL: Into>, { @@ -236,7 +236,7 @@ impl ColumnCommitments { .into_iter() .map(|(identifier, column)| { if self.column_metadata.contains_key(identifier) { - Err(DuplicateIdentifiers { + Err(DuplicateIdents { id: identifier.to_string(), }) } else { @@ -498,10 +498,7 @@ mod tests { 0, &(), ); - assert!(matches!( - from_columns_result, - Err(DuplicateIdentifiers { .. }) - )); + assert!(matches!(from_columns_result, Err(DuplicateIdents { .. }))); let mut existing_column_commitments = ColumnCommitments::::try_from_columns_with_offset( @@ -518,7 +515,7 @@ mod tests { .try_extend_columns_with_offset([(&duplicate_identifier_a, &empty_column)], 0, &()); assert!(matches!( extend_with_existing_column_result, - Err(DuplicateIdentifiers { .. }) + Err(DuplicateIdents { .. }) )); let extend_with_duplicate_columns_result = existing_column_commitments @@ -532,7 +529,7 @@ mod tests { ); assert!(matches!( extend_with_duplicate_columns_result, - Err(DuplicateIdentifiers { .. }) + Err(DuplicateIdents { .. }) )); let append_result = existing_column_commitments.try_append_rows_with_offset( @@ -546,7 +543,7 @@ mod tests { ); assert!(matches!( append_result, - Err(AppendColumnCommitmentsError::DuplicateIdentifiers { .. }) + Err(AppendColumnCommitmentsError::DuplicateIdents { .. }) )); } diff --git a/crates/proof-of-sql/src/base/commitment/mod.rs b/crates/proof-of-sql/src/base/commitment/mod.rs index 187ea2b54..2116b0478 100644 --- a/crates/proof-of-sql/src/base/commitment/mod.rs +++ b/crates/proof-of-sql/src/base/commitment/mod.rs @@ -28,9 +28,7 @@ pub use column_commitment_metadata_map::{ }; mod column_commitments; -pub use column_commitments::{ - AppendColumnCommitmentsError, ColumnCommitments, DuplicateIdentifiers, -}; +pub use column_commitments::{AppendColumnCommitmentsError, ColumnCommitments, DuplicateIdents}; mod table_commitment; pub use table_commitment::{ diff --git a/crates/proof-of-sql/src/base/commitment/table_commitment.rs b/crates/proof-of-sql/src/base/commitment/table_commitment.rs index dab7beb5d..80130363b 100644 --- a/crates/proof-of-sql/src/base/commitment/table_commitment.rs +++ b/crates/proof-of-sql/src/base/commitment/table_commitment.rs @@ -1,6 +1,6 @@ use super::{ committable_column::CommittableColumn, AppendColumnCommitmentsError, ColumnCommitments, - ColumnCommitmentsMismatch, Commitment, DuplicateIdentifiers, + ColumnCommitmentsMismatch, Commitment, DuplicateIdents, }; use crate::base::{ database::{ColumnField, CommitmentAccessor, OwnedTable, TableRef}, @@ -31,11 +31,11 @@ pub enum TableCommitmentFromColumnsError { /// The underlying source error source: MixedLengthColumns, }, - /// Cannot construct [`TableCommitment`] from columns with duplicate identifiers. + /// Cannot construct [`TableCommitment`] from columns with duplicate idents. #[snafu(transparent)] - DuplicateIdentifiers { + DuplicateIdents { /// The underlying source error - source: DuplicateIdentifiers, + source: DuplicateIdents, }, } @@ -158,7 +158,7 @@ impl TableCommitment { /// Returns a [`TableCommitment`] to the provided columns with the given row offset. /// - /// Provided columns must have the same length and no duplicate identifiers. + /// Provided columns must have the same length and no duplicate idents. pub fn try_from_columns_with_offset<'a, COL>( columns: impl IntoIterator, offset: usize, @@ -189,7 +189,7 @@ impl TableCommitment { /// Returns a [`TableCommitment`] to the provided table with the given row offset. #[allow( clippy::missing_panics_doc, - reason = "since OwnedTables cannot have columns of mixed length or duplicate identifiers" + reason = "since OwnedTables cannot have columns of mixed length or duplicate idents" )] pub fn from_owned_table_with_offset( owned_table: &OwnedTable, @@ -200,7 +200,7 @@ impl TableCommitment { S: Scalar, { Self::try_from_columns_with_offset(owned_table.inner_table(), offset, setup) - .expect("OwnedTables cannot have columns of mixed length or duplicate identifiers") + .expect("OwnedTables cannot have columns of mixed length or duplicate idents") } /// Append rows of data from the provided columns to the existing [`TableCommitment`]. @@ -238,7 +238,7 @@ impl TableCommitment { /// Will error on a variety of mismatches. /// See [`ColumnCommitmentsMismatch`] for an enumeration of these errors. /// # Panics - /// Panics if `owned_table` has duplicate identifiers. + /// Panics if `owned_table` has duplicate idents. /// Panics if `owned_table` contains columns of mixed length. pub fn append_owned_table( &mut self, @@ -252,8 +252,8 @@ impl TableCommitment { .map_err(|e| match e { AppendTableCommitmentError::AppendColumnCommitments { source: e } => match e { AppendColumnCommitmentsError::Mismatch { source: e } => e, - AppendColumnCommitmentsError::DuplicateIdentifiers { .. } => { - panic!("OwnedTables cannot have duplicate identifiers"); + AppendColumnCommitmentsError::DuplicateIdents { .. } => { + panic!("OwnedTables cannot have duplicate idents"); } }, AppendTableCommitmentError::MixedLengthColumns { .. } => { @@ -264,7 +264,7 @@ impl TableCommitment { /// Add new columns to this [`TableCommitment`]. /// - /// Columns must have the same length as the current commitment and no duplicate identifiers. + /// Columns must have the same length as the current commitment and no duplicate idents. pub fn try_extend_columns<'a, COL>( &mut self, columns: impl IntoIterator, @@ -480,7 +480,7 @@ mod tests { ); assert!(matches!( from_columns_result, - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) + Err(TableCommitmentFromColumnsError::DuplicateIdents { .. }) )); let mut table_commitment = @@ -499,7 +499,7 @@ mod tests { table_commitment.try_extend_columns([(&duplicate_identifier_a, &empty_column)], &()); assert!(matches!( extend_columns_result, - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) + Err(TableCommitmentFromColumnsError::DuplicateIdents { .. }) )); let extend_columns_result = table_commitment.try_extend_columns( @@ -511,7 +511,7 @@ mod tests { ); assert!(matches!( extend_columns_result, - Err(TableCommitmentFromColumnsError::DuplicateIdentifiers { .. }) + Err(TableCommitmentFromColumnsError::DuplicateIdents { .. }) )); // make sure the commitment wasn't mutated @@ -689,7 +689,7 @@ mod tests { assert!(matches!( append_column_result, Err(AppendTableCommitmentError::AppendColumnCommitments { - source: AppendColumnCommitmentsError::DuplicateIdentifiers { .. } + source: AppendColumnCommitmentsError::DuplicateIdents { .. } }) )); diff --git a/crates/proof-of-sql/src/base/database/owned_table.rs b/crates/proof-of-sql/src/base/database/owned_table.rs index 971055219..2218c5e42 100644 --- a/crates/proof-of-sql/src/base/database/owned_table.rs +++ b/crates/proof-of-sql/src/base/database/owned_table.rs @@ -30,7 +30,7 @@ pub(crate) enum TableCoercionError { ColumnCountMismatch, } -/// A table of data, with schema included. This is simply a map from `Identifier` to `OwnedColumn`, +/// A table of data, with schema included. This is simply a map from `Ident` to `OwnedColumn`, /// where columns order matters. /// This is primarily used as an internal result that is used before /// converting to the final result in either Arrow format or JSON. diff --git a/crates/proof-of-sql/src/base/database/owned_table_utility.rs b/crates/proof-of-sql/src/base/database/owned_table_utility.rs index 40f5ebd92..97a695ca4 100644 --- a/crates/proof-of-sql/src/base/database/owned_table_utility.rs +++ b/crates/proof-of-sql/src/base/database/owned_table_utility.rs @@ -19,7 +19,7 @@ use alloc::string::String; use proof_of_sql_parser::posql_time::{PoSQLTimeUnit, PoSQLTimeZone}; use sqlparser::ast::Ident; -/// Creates an [`OwnedTable`] from a list of `(Identifier, OwnedColumn)` pairs. +/// Creates an [`OwnedTable`] from a list of `(Ident, OwnedColumn)` pairs. /// This is a convenience wrapper around [`OwnedTable::try_from_iter`] primarily for use in tests and /// intended to be used along with the other methods in this module (e.g. [bigint], [boolean], etc). /// The function will panic under a variety of conditions. See [`OwnedTable::try_from_iter`] for more details. @@ -36,7 +36,7 @@ use sqlparser::ast::Ident; /// decimal75("f", 12, 1, [1, 2, 3]), /// ]); /// ``` -/// +/// /// /// # Panics /// - Panics if converting the iterator into an `OwnedTable` fails. pub fn owned_table( @@ -45,7 +45,7 @@ pub fn owned_table( OwnedTable::try_from_iter(iter).unwrap() } -/// Creates a (Identifier, `OwnedColumn`) pair for a tinyint column. +/// Creates a (Ident, `OwnedColumn`) pair for a tinyint column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ``` @@ -54,8 +54,6 @@ pub fn owned_table( /// tinyint("a", [1_i8, 2, 3]), /// ]); ///``` -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. pub fn tinyint( name: impl Into, data: impl IntoIterator>, @@ -66,7 +64,7 @@ pub fn tinyint( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a smallint column. +/// Creates a `(Ident, OwnedColumn)` pair for a smallint column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ```rust @@ -75,8 +73,6 @@ pub fn tinyint( /// smallint("a", [1_i16, 2, 3]), /// ]); /// ``` -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. pub fn smallint( name: impl Into, data: impl IntoIterator>, @@ -87,7 +83,7 @@ pub fn smallint( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for an int column. +/// Creates a `(Ident, OwnedColumn)` pair for an int column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ```rust @@ -96,8 +92,6 @@ pub fn smallint( /// int("a", [1, 2, 3]), /// ]); /// ``` -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. pub fn int( name: impl Into, data: impl IntoIterator>, @@ -108,7 +102,7 @@ pub fn int( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a bigint column. +/// Creates a `(Ident, OwnedColumn)` pair for a bigint column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ```rust @@ -128,7 +122,7 @@ pub fn bigint( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a boolean column. +/// Creates a `(Ident, OwnedColumn)` pair for a boolean column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ``` @@ -137,9 +131,6 @@ pub fn bigint( /// boolean("a", [true, false, true]), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. pub fn boolean( name: impl Into, data: impl IntoIterator>, @@ -150,7 +141,7 @@ pub fn boolean( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a int128 column. +/// Creates a `(Ident, OwnedColumn)` pair for a int128 column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ``` @@ -159,9 +150,6 @@ pub fn boolean( /// int128("a", [1, 2, 3]), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. pub fn int128( name: impl Into, data: impl IntoIterator>, @@ -172,7 +160,7 @@ pub fn int128( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a scalar column. +/// Creates a `(Ident, OwnedColumn)` pair for a scalar column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ``` @@ -181,9 +169,7 @@ pub fn int128( /// scalar("a", [1, 2, 3]), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn scalar( name: impl Into, data: impl IntoIterator>, @@ -194,7 +180,7 @@ pub fn scalar( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a varchar column. +/// Creates a `(Ident, OwnedColumn)` pair for a varchar column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ``` @@ -203,9 +189,7 @@ pub fn scalar( /// varchar("a", ["a", "b", "c"]), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn varchar( name: impl Into, data: impl IntoIterator>, @@ -216,7 +200,7 @@ pub fn varchar( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a decimal75 column. +/// Creates a `(Ident, OwnedColumn)` pair for a decimal75 column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// # Example /// ``` @@ -227,7 +211,6 @@ pub fn varchar( /// ``` /// /// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. /// - Panics if creating the `Precision` from the specified precision value fails. pub fn decimal75( name: impl Into, @@ -245,7 +228,7 @@ pub fn decimal75( ) } -/// Creates a `(Identifier, OwnedColumn)` pair for a timestamp column. +/// Creates a `(Ident, OwnedColumn)` pair for a timestamp column. /// This is primarily intended for use in conjunction with [`owned_table`]. /// /// # Parameters @@ -266,9 +249,7 @@ pub fn decimal75( /// timestamptz("event_time", PoSQLTimeUnit::Second, PoSQLTimeZone::utc(), vec![1625072400, 1625076000, 1625079600]), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn timestamptz( name: impl Into, time_unit: PoSQLTimeUnit, diff --git a/crates/proof-of-sql/src/base/database/table.rs b/crates/proof-of-sql/src/base/database/table.rs index c891d2f57..da15658f4 100644 --- a/crates/proof-of-sql/src/base/database/table.rs +++ b/crates/proof-of-sql/src/base/database/table.rs @@ -35,7 +35,7 @@ pub enum TableError { #[snafu(display("Table is empty and no row count is specified"))] EmptyTableWithoutSpecifiedRowCount, } -/// A table of data, with schema included. This is simply a map from `Identifier` to `Column`, +/// A table of data, with schema included. This is simply a map from `Ident` to `Column`, /// where columns order matters. /// This is primarily used as an internal result that is used before /// converting to the final result in either Arrow format or JSON. @@ -77,14 +77,14 @@ impl<'a, S: Scalar> Table<'a, S> { } } - /// Creates a new [`Table`] from an iterator of `(Identifier, Column)` pairs with default [`TableOptions`]. + /// Creates a new [`Table`] from an iterator of `(Ident, Column)` pairs with default [`TableOptions`]. pub fn try_from_iter)>>( iter: T, ) -> Result { Self::try_from_iter_with_options(iter, TableOptions::default()) } - /// Creates a new [`Table`] from an iterator of `(Identifier, Column)` pairs with [`TableOptions`]. + /// Creates a new [`Table`] from an iterator of `(Ident, Column)` pairs with [`TableOptions`]. pub fn try_from_iter_with_options)>>( iter: T, options: TableOptions, diff --git a/crates/proof-of-sql/src/base/database/table_utility.rs b/crates/proof-of-sql/src/base/database/table_utility.rs index 445d7e2bd..25458bac3 100644 --- a/crates/proof-of-sql/src/base/database/table_utility.rs +++ b/crates/proof-of-sql/src/base/database/table_utility.rs @@ -22,7 +22,7 @@ use bumpalo::Bump; use proof_of_sql_parser::posql_time::{PoSQLTimeUnit, PoSQLTimeZone}; use sqlparser::ast::Ident; -/// Creates an [`Table`] from a list of `(Identifier, Column)` pairs. +/// Creates an [`Table`] from a list of `(Ident, Column)` pairs. /// This is a convenience wrapper around [`Table::try_from_iter`] primarily for use in tests and /// intended to be used along with the other methods in this module (e.g. [`borrowed_bigint`], /// [`borrowed_boolean`], etc). @@ -51,7 +51,7 @@ pub fn table<'a, S: Scalar>( Table::try_from_iter(iter).unwrap() } -/// Creates an [`Table`] from a list of `(Identifier, Column)` pairs with a specified row count. +/// Creates an [`Table`] from a list of `(Ident, Column)` pairs with a specified row count. /// The main reason for this function is to allow for creating tables that may potentially have /// no columns, but still have a specified row count. /// @@ -64,7 +64,7 @@ pub fn table_with_row_count<'a, S: Scalar>( Table::try_from_iter_with_options(iter, TableOptions::new(Some(row_count))).unwrap() } -/// Creates a (Identifier, `Column`) pair for a tinyint column. +/// Creates a (Ident, `Column`) pair for a tinyint column. /// This is primarily intended for use in conjunction with [`table`]. /// # Example /// ``` @@ -75,8 +75,6 @@ pub fn table_with_row_count<'a, S: Scalar>( /// borrowed_tinyint("a", [1_i8, 2, 3], &alloc), /// ]); ///``` -/// # Panics -/// - Panics if `name.parse()()` fails to convert the name into an `Identifier`. pub fn borrowed_tinyint( name: impl Into, data: impl IntoIterator>, @@ -87,7 +85,7 @@ pub fn borrowed_tinyint( (name.into(), Column::TinyInt(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for a smallint column. +/// Creates a `(Ident, Column)` pair for a smallint column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Example @@ -100,8 +98,6 @@ pub fn borrowed_tinyint( /// ]); /// ``` /// -/// # Panics -/// - Panics if `name.parse()()` fails to convert the name into an `Identifier`. pub fn borrowed_smallint( name: impl Into, data: impl IntoIterator>, @@ -112,7 +108,7 @@ pub fn borrowed_smallint( (name.into(), Column::SmallInt(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for an int column. +/// Creates a `(Ident, Column)` pair for an int column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Example @@ -125,8 +121,6 @@ pub fn borrowed_smallint( /// ]); /// ``` /// -/// # Panics -/// - Panics if `name.parse()()` fails to convert the name into an `Identifier`. pub fn borrowed_int( name: impl Into, data: impl IntoIterator>, @@ -137,7 +131,7 @@ pub fn borrowed_int( (name.into(), Column::Int(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for a bigint column. +/// Creates a `(Ident, Column)` pair for a bigint column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Example @@ -149,9 +143,7 @@ pub fn borrowed_int( /// borrowed_bigint("a", [1, 2, 3], &alloc), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn borrowed_bigint( name: impl Into, data: impl IntoIterator>, @@ -162,7 +154,7 @@ pub fn borrowed_bigint( (name.into(), Column::BigInt(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for a boolean column. +/// Creates a `(Ident, Column)` pair for a boolean column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Example @@ -174,9 +166,7 @@ pub fn borrowed_bigint( /// borrowed_boolean("a", [true, false, true], &alloc), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn borrowed_boolean( name: impl Into, data: impl IntoIterator>, @@ -187,7 +177,7 @@ pub fn borrowed_boolean( (name.into(), Column::Boolean(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for an int128 column. +/// Creates a `(Ident, Column)` pair for an int128 column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Example @@ -199,9 +189,7 @@ pub fn borrowed_boolean( /// borrowed_int128("a", [1, 2, 3], &alloc), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn borrowed_int128( name: impl Into, data: impl IntoIterator>, @@ -212,7 +200,7 @@ pub fn borrowed_int128( (name.into(), Column::Int128(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for a scalar column. +/// Creates a `(Ident, Column)` pair for a scalar column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Example @@ -224,9 +212,7 @@ pub fn borrowed_int128( /// borrowed_scalar("a", [1, 2, 3], &alloc), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn borrowed_scalar( name: impl Into, data: impl IntoIterator>, @@ -237,7 +223,7 @@ pub fn borrowed_scalar( (name.into(), Column::Scalar(alloc_data)) } -/// Creates a `(Identifier, Column)` pair for a varchar column. +/// Creates a `(Ident, Column)` pair for a varchar column. /// This is primarily intended for use in conjunction with [`table`]. /// # Example /// ``` @@ -248,9 +234,7 @@ pub fn borrowed_scalar( /// borrowed_varchar("a", ["a", "b", "c"], &alloc), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn borrowed_varchar<'a, S: Scalar>( name: impl Into, data: impl IntoIterator>, @@ -269,7 +253,7 @@ pub fn borrowed_varchar<'a, S: Scalar>( (name.into(), Column::VarChar((alloc_strings, alloc_scalars))) } -/// Creates a `(Identifier, Column)` pair for a decimal75 column. +/// Creates a `(Ident, Column)` pair for a decimal75 column. /// This is primarily intended for use in conjunction with [`table`]. /// # Example /// ``` @@ -280,9 +264,7 @@ pub fn borrowed_varchar<'a, S: Scalar>( /// borrowed_decimal75("a", 12, 1, [1, 2, 3], &alloc), /// ]); /// ``` -/// /// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. /// - Panics if creating the `Precision` from the specified precision value fails. pub fn borrowed_decimal75( name: impl Into, @@ -303,7 +285,7 @@ pub fn borrowed_decimal75( ) } -/// Creates a `(Identifier, Column)` pair for a timestamp column. +/// Creates a `(Ident, Column)` pair for a timestamp column. /// This is primarily intended for use in conjunction with [`table`]. /// /// # Parameters @@ -327,9 +309,7 @@ pub fn borrowed_decimal75( /// borrowed_timestamptz("event_time", PoSQLTimeUnit::Second, PoSQLTimeZone::utc(), vec![1625072400, 1625076000, 1625079600], &alloc), /// ]); /// ``` -/// -/// # Panics -/// - Panics if `name.parse()` fails to convert the name into an `Identifier`. + pub fn borrowed_timestamptz( name: impl Into, time_unit: PoSQLTimeUnit, diff --git a/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs b/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs index 177e925d6..29a84d0f9 100644 --- a/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs +++ b/crates/proof-of-sql/src/sql/parse/query_expr_tests.rs @@ -1582,7 +1582,7 @@ fn we_cannot_use_non_grouped_columns_outside_agg() { assert!(matches!( result, Err(ConversionError::PostprocessingError { - source: PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. } + source: PostprocessingError::IdentNotInAggregationOperatorOrGroupByClause { .. } }) )); } diff --git a/crates/proof-of-sql/src/sql/postprocessing/error.rs b/crates/proof-of-sql/src/sql/postprocessing/error.rs index adce01894..054b07358 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/error.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/error.rs @@ -31,8 +31,8 @@ pub enum PostprocessingError { }, /// GROUP BY clause references a column not in a group by expression outside aggregate functions #[snafu(display("Invalid group by: column '{column}' must not appear outside aggregate functions or `GROUP BY` clause."))] - IdentifierNotInAggregationOperatorOrGroupByClause { - /// The column identifier + IdentNotInAggregationOperatorOrGroupByClause { + /// The column ident column: Ident, }, /// Errors in converting `Ident` to `Identifier` diff --git a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs index 804bcde0e..7bc701ba8 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing.rs @@ -152,7 +152,7 @@ fn check_and_get_aggregation_and_remainder( .next() .unwrap(); Err( - PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { + PostprocessingError::IdentNotInAggregationOperatorOrGroupByClause { column: diff.clone(), }, ) diff --git a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs index efcb9e0c8..3af8fae7e 100644 --- a/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs +++ b/crates/proof-of-sql/src/sql/postprocessing/group_by_postprocessing_test.rs @@ -17,7 +17,7 @@ fn we_cannot_have_invalid_group_bys() { let res = GroupByPostprocessing::try_new(vec!["a".into()], vec![aliased_expr(expr, "res")]); assert!(matches!( res, - Err(PostprocessingError::IdentifierNotInAggregationOperatorOrGroupByClause { .. }) + Err(PostprocessingError::IdentNotInAggregationOperatorOrGroupByClause { .. }) )); // Nested aggregation diff --git a/crates/proof-of-sql/src/sql/proof_exprs/test_utility.rs b/crates/proof-of-sql/src/sql/proof_exprs/test_utility.rs index e084916ab..baa8b44e5 100644 --- a/crates/proof-of-sql/src/sql/proof_exprs/test_utility.rs +++ b/crates/proof-of-sql/src/sql/proof_exprs/test_utility.rs @@ -7,9 +7,6 @@ use crate::base::{ use proof_of_sql_parser::intermediate_ast::AggregationOperator; use sqlparser::ast::Ident; -/// # Panics -/// Panics if: -/// - `name.parse()` fails, which means the provided string could not be parsed into the expected type (usually an `Identifier`). pub fn col_ref(tab: TableRef, name: &str, accessor: &impl SchemaAccessor) -> ColumnRef { let name: Ident = name.into(); let type_col = accessor.lookup_column(tab, name.clone()).unwrap(); @@ -18,7 +15,6 @@ pub fn col_ref(tab: TableRef, name: &str, accessor: &impl SchemaAccessor) -> Col /// # Panics /// Panics if: -/// - `name.parse()` fails to parse the column name. /// - `accessor.lookup_column()` returns `None`, indicating the column is not found. pub fn column(tab: TableRef, name: &str, accessor: &impl SchemaAccessor) -> DynProofExpr { let name: Ident = name.into();