From fda888d44a24bc4319f76415c34ebe7c17879db5 Mon Sep 17 00:00:00 2001 From: Hana Kessler <58595147+hanakslr@users.noreply.github.com> Date: Fri, 6 Dec 2024 15:11:46 -0500 Subject: [PATCH 1/5] wip: insert store and load statements Co-Authored-By: Eric Kidd <36963+emk@users.noreply.github.com> --- ARCHITECTURE.md | 21 ++-- LOAD.md | 19 ++++ src/ast.rs | 51 +++++++-- src/cmd/run.rs | 3 +- src/cmd/sql_test.rs | 3 +- src/cmd/transpile.rs | 5 +- src/drivers/mod.rs | 2 +- src/infer/insert_store_expressions.rs | 156 ++++++++++++++++++++++++++ src/infer/mod.rs | 46 ++++++-- src/types.rs | 59 +++++++++- 10 files changed, 324 insertions(+), 41 deletions(-) create mode 100644 LOAD.md create mode 100644 src/infer/insert_store_expressions.rs diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 848a145..3883f7b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -7,20 +7,21 @@ Here's a high level overview of how `joinery` works. Compilation procedes in several phases: 1. [Tokenize](./src/tokenizer.rs). - - Split the source into identifiers, punctuation, literals, etc. All tokens contain the original source code, location information, and surrounding whitespace. + - Split the source into identifiers, punctuation, literals, etc. All tokens contain the original source code, location information, and surrounding whitespace. 2. [Parse into AST](./src/ast.rs). - - We use the [`peg` crate](https://docs.rs/peg/). This is a [Parsing Expression Grammar](https://en.wikipedia.org/wiki/Parsing_expression_grammar) (PEG) parser. This is a bit _ad hoc_ as grammars go, but `peg` is a very nice library. - - We make heavy use of `#[derive]` macros to implement the AST types. + - We use the [`peg` crate](https://docs.rs/peg/). This is a [Parsing Expression Grammar](https://en.wikipedia.org/wiki/Parsing_expression_grammar) (PEG) parser. This is a bit _ad hoc_ as grammars go, but `peg` is a very nice library. + - We make heavy use of `#[derive]` macros to implement the AST types. 3. [Check types](./src/infer/mod.rs). - - The internal type system is defined in [`src/types.rs`](./src/types.rs). This is distinct from the simplisitic "source level" type system parsed by [`src/ast.rs`](./src/ast.rs), and better suited to doing inference. - - Name lookup is handled in [`src/scopes.rs`](./src/scopes.rs). Note that SQL requires several different kinds of scopes. + - The internal type system is defined in [`src/types.rs`](./src/types.rs). This is distinct from the simplisitic "source level" type system parsed by [`src/ast.rs`](./src/ast.rs), and better suited to doing inference. + - Name lookup is handled in [`src/scopes.rs`](./src/scopes.rs). Note that SQL requires several different kinds of scopes. + - Type checking also needs to know about "memory" types (like Trino's UUID) versus "storage" types (like Trino's VARCHAR when using Hive, which doesn't allow storing UUID). And it needs make sure that all appropriate `LoadExpression` and `StoreExpression` values get inserted. 4. [Apply transforms](./src/transforms/mod.rs). - - A list of transforms is supplied by each database driver. - - Transforms use Rust pattern-matching to match parts of the AST, and build new AST nodes using `sql_quote!`. Note that `sql_quote!` outputs _tokens_, so we need to call back into the parser. This is closely patterned after Rust programmatic macros using [`syn`](https://docs.rs/syn/) and [`quote`](https://docs.rs/quote/). - - After applying a transform, we _may_ need to check types again to support later transforms. This works a bit like an LLVM analysis pass, where specific transforms may indicate that the require types, and the harness ensures that valid types are available. - - The output of a transform must be structurally valid BigQuery SQL, though after a certain point it may no longer type check. + - A list of transforms is supplied by each database driver. + - Transforms use Rust pattern-matching to match parts of the AST, and build new AST nodes using `sql_quote!`. Note that `sql_quote!` outputs _tokens_, so we need to call back into the parser. This is closely patterned after Rust programmatic macros using [`syn`](https://docs.rs/syn/) and [`quote`](https://docs.rs/quote/). + - After applying a transform, we _may_ need to check types again to support later transforms. This works a bit like an LLVM analysis pass, where specific transforms may indicate that the require types, and the harness ensures that valid types are available. + - The output of a transform must be structurally valid BigQuery SQL, though after a certain point it may no longer type check. 5. [Emit SQL](./src/ast.rs). - - This consumes AST nodes and emits them as database-specific strings. We prefer to do as much work as possible using AST transforms, but sometimes we can't represent database-specific features in the AST. + - This consumes AST nodes and emits them as database-specific strings. We prefer to do as much work as possible using AST transforms, but sometimes we can't represent database-specific features in the AST. 6. [Run](./src/drivers/mod.rs). - This is a slightly dodgy layer that knows how to run SQL. Mostly it's intended for running our test suites, not for production use. Some of the Rust database drivers have problems reading complex data types back into Rust. diff --git a/LOAD.md b/LOAD.md new file mode 100644 index 0000000..525883a --- /dev/null +++ b/LOAD.md @@ -0,0 +1,19 @@ +All SQL must be run through joinery. +Every time that we create a table, we need to insert its BigQuery `CREATE TABLE name (col...)` into pg. +That table looks like this: + +```sql +PRIMARY KEY (bq_project, bq_dataset, bq_table_name) + +bq_project -- The BigQuery project, which gets mapped to a Trino catalog somehow +bq_dataset -- The BigQuery dataset, which is a trino schema with the same name +bq_table_name -- The BigQuery table, which is a trino table with the same name +create_table_sql -- this is always a raw typed CREATE TABLE statement in BigQuery SQL. CREATE TABLE (my_col data_type, ...); +``` + +When we run another SQL query tomorrow, we need to make sure that we have access to the BigQuery table names and their `CREATE TABLE` SQL +We can load those table definitions into our scope +Then run type inference normally + +So when we try to access prod_gke.my_dataset.my_table, +...we find a CREATE TABLE for it, parse it, and inject it in the scope diff --git a/src/ast.rs b/src/ast.rs index 4cdf756..775db10 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -34,12 +34,14 @@ use crate::{ trino::{TrinoString, KEYWORDS as TRINO_KEYWORDS}, }, errors::{format_err, Error, Result}, + infer::{InferTypes, InsertStoreExpressions as _}, known_files::{FileId, KnownFiles}, + scope::{Scope, ScopeHandle}, tokenizer::{ tokenize_sql, EmptyFile, Ident, Keyword, Literal, LiteralValue, PseudoKeyword, Punct, RawToken, Span, Spanned, ToTokens, Token, TokenStream, TokenWriter, }, - types::{StructType, TableType, ValueType}, + types::{SimpleType, StructType, TableType, ValueType}, util::{is_c_ident, AnsiIdent}, }; @@ -621,6 +623,16 @@ pub struct SqlProgram { pub statements: NodeVec, } +impl SqlProgram { + /// Call `infer_types` for the first time, using the root scope, and doing + /// the one-time task of inserting [`StoreExpression`] where needed. + pub fn infer_types_for_first_time(&mut self) -> Result<(Option, ScopeHandle)> { + self.insert_store_expressions()?; + let scope = Scope::root(); + self.infer_types(&scope) + } +} + /// A statement in our abstract syntax tree. #[derive(Clone, Debug, Drive, DriveMut, Emit, EmitDefault, Spanned, ToTokens)] pub enum Statement { @@ -1646,7 +1658,7 @@ pub struct LoadExpression { #[emit(skip)] #[to_tokens(skip)] #[drive(skip)] - memory_type: Option, + pub memory_type: Option, /// Our underlying expression. pub expression: Box, @@ -1655,6 +1667,11 @@ pub struct LoadExpression { impl Emit for LoadExpression { fn emit(&self, t: Target, f: &mut TokenWriter<'_>) -> ::std::io::Result<()> { match t { + Target::BigQuery => { + f.write_token_start("%LOAD(")?; + self.expression.emit(t, f)?; + f.write_token_start(")") + } Target::Trino(connector_type) => { let bq_memory_type = self .memory_type @@ -1692,7 +1709,7 @@ pub struct StoreExpression { #[emit(skip)] #[to_tokens(skip)] #[drive(skip)] - memory_type: Option, + pub memory_type: Option, /// Our underlying expression. pub expression: Box, @@ -1701,21 +1718,33 @@ pub struct StoreExpression { impl Emit for StoreExpression { fn emit(&self, t: Target, f: &mut TokenWriter<'_>) -> ::std::io::Result<()> { match t { + Target::BigQuery => { + f.write_token_start("%STORE(")?; + self.expression.emit(t, f)?; + f.write_token_start(")") + } Target::Trino(connector_type) => { let bq_memory_type = self .memory_type .as_ref() .expect("memory_type should have been filled in by type inference"); - let trino_memory_type = - TrinoDataType::try_from(bq_memory_type).map_err(io::Error::other)?; - let transform = connector_type.storage_transform_for(&trino_memory_type); - let (prefix, suffix) = transform.store_prefix_and_suffix(); - f.write_token_start(&prefix)?; - self.expression.emit(t, f)?; - f.write_token_start(&suffix) + // If our bq_memory_type is NULL, we don't need to do any transforms because + // NULL is NULL in both storage and memory types and dbcrossbar_trino doesn't + // support NULL as a memory type. + if let ValueType::Simple(SimpleType::Null) = bq_memory_type { + self.expression.emit(t, f) + } else { + let trino_memory_type = + TrinoDataType::try_from(bq_memory_type).map_err(io::Error::other)?; + let transform = connector_type.storage_transform_for(&trino_memory_type); + let (prefix, suffix) = transform.store_prefix_and_suffix(); + + f.write_token_start(&prefix)?; + self.expression.emit(t, f)?; + f.write_token_start(&suffix) + } } - _ => self.emit_default(t, f), } } } diff --git a/src/cmd/run.rs b/src/cmd/run.rs index 7af351b..7a40758 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -34,8 +34,7 @@ pub async fn cmd_run(files: &mut KnownFiles, opt: &RunOpt) -> Result<()> { let mut ast = parse_sql(files, file_id)?; // Run the type checker, but do not fail on errors. - let scope = Scope::root(); - if let Err(err) = ast.infer_types(&scope) { + if let Err(err) = ast.infer_types_for_first_time() { err.emit(files); eprintln!("\nType checking failed. Manual fixes will probably be required!"); } diff --git a/src/cmd/sql_test.rs b/src/cmd/sql_test.rs index 1c9f9df..f5787b1 100644 --- a/src/cmd/sql_test.rs +++ b/src/cmd/sql_test.rs @@ -148,8 +148,7 @@ async fn run_test( let mut ast = parse_sql(files, file_id)?; // Type check the AST. - let scope = Scope::root(); - ast.infer_types(&scope)?; + ast.infer_types_for_first_time()?; //eprintln!("SQLite3: {}", ast.emit_to_string(Target::SQLite3)); let output_tables = find_output_tables(&ast)?; diff --git a/src/cmd/transpile.rs b/src/cmd/transpile.rs index 24736d4..f354661 100644 --- a/src/cmd/transpile.rs +++ b/src/cmd/transpile.rs @@ -9,9 +9,7 @@ use crate::{ ast::{parse_sql, Emit}, drivers, errors::Result, - infer::InferTypes, known_files::KnownFiles, - scope::Scope, }; /// Run SQL tests from a directory. @@ -38,8 +36,7 @@ pub async fn cmd_transpile(files: &mut KnownFiles, opt: &TranspileOpt) -> Result let mut ast = parse_sql(files, file_id)?; // Run the type checker, but do not fail on errors. - let scope = Scope::root(); - if let Err(err) = ast.infer_types(&scope) { + if let Err(err) = ast.infer_types_for_first_time() { err.emit(files); eprintln!("\nType checking failed. Manual fixes will probably be required!"); } diff --git a/src/drivers/mod.rs b/src/drivers/mod.rs index bccb006..8adb780 100644 --- a/src/drivers/mod.rs +++ b/src/drivers/mod.rs @@ -9,7 +9,7 @@ use tracing::{debug, trace}; use crate::{ ast::{self, Emit, Target}, errors::{format_err, Error, Result}, - infer::InferTypes, + infer::InferTypes as _, scope::Scope, transforms::{Transform, TransformExtra}, }; diff --git a/src/infer/insert_store_expressions.rs b/src/infer/insert_store_expressions.rs new file mode 100644 index 0000000..d126559 --- /dev/null +++ b/src/infer/insert_store_expressions.rs @@ -0,0 +1,156 @@ +//! A preliminary, once-only type inference step where we patch up the AST +//! to include [`ast::StoreExpression`]. + +use crate::{ + ast::{self}, + errors::Result, +}; + +use super::nyi; + +/// Walk an AST tree, inserting [`ast::StoreExpression`] everywhere we need it. +/// +/// This is called only once, before the first time we run type inference. +pub trait InsertStoreExpressions { + /// Find all the places that need a [`ast::StoreExpression`] and insert them. + fn insert_store_expressions(&mut self) -> Result<()>; +} + +impl InsertStoreExpressions for ast::SqlProgram { + fn insert_store_expressions(&mut self) -> Result<()> { + self.statements.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::Statement { + fn insert_store_expressions(&mut self) -> Result<()> { + match self { + ast::Statement::Query(stmt) => stmt.insert_store_expressions(), + ast::Statement::DeleteFrom(_) => Ok(()), + ast::Statement::InsertInto(stmt) => stmt.insert_store_expressions(), + ast::Statement::CreateTable(stmt) => stmt.insert_store_expressions(), + // This is a problem for another day and another poor developer. Do + // views output values in memory format or backend-specific storage + // format? + ast::Statement::CreateView(_) => Err(nyi(self, "CREATE VIEW storage expressions")), + ast::Statement::DropTable(_) => Ok(()), + ast::Statement::DropView(_) => Ok(()), + } + } +} +impl InsertStoreExpressions for ast::QueryStatement { + fn insert_store_expressions(&mut self) -> Result<()> { + self.query_expression.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::QueryExpression { + fn insert_store_expressions(&mut self) -> Result<()> { + self.query.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::QueryExpressionQuery { + fn insert_store_expressions(&mut self) -> Result<()> { + match self { + ast::QueryExpressionQuery::Select(expr) => expr.insert_store_expressions(), + ast::QueryExpressionQuery::Nested { query, .. } => query.insert_store_expressions(), + ast::QueryExpressionQuery::SetOperation { left, right, .. } => { + left.insert_store_expressions()?; + right.insert_store_expressions() + } + } + } +} + +impl InsertStoreExpressions for ast::SelectExpression { + fn insert_store_expressions(&mut self) -> Result<()> { + self.select_list.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::SelectList { + fn insert_store_expressions(&mut self) -> Result<()> { + self.items.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::SelectListItem { + fn insert_store_expressions(&mut self) -> Result<()> { + match self { + ast::SelectListItem::Expression { expression, .. } => { + expression.insert_store_expressions() + } + ast::SelectListItem::Wildcard { .. } => { + Err(nyi(self, "InsertStoreExpressions(Wildcard)")) + } + ast::SelectListItem::TableNameWildcard { .. } => { + Err(nyi(self, "InsertStoreExpressions(TableNameWildcard)")) + } + ast::SelectListItem::ExpressionWildcard { .. } => { + Err(nyi(self, "InsertStoreExpressions(TableNameWildcard)")) + } + } + } +} + +impl InsertStoreExpressions for ast::Expression { + /// Wrap ourselves in a `StoreExpression`. Not recursive! + fn insert_store_expressions(&mut self) -> Result<()> { + let store_expr = ast::Expression::Store(ast::StoreExpression { + memory_type: None, + expression: Box::new(self.clone()), + }); + *self = store_expr; + Ok(()) + } +} + +impl InsertStoreExpressions for ast::InsertIntoStatement { + fn insert_store_expressions(&mut self) -> Result<()> { + self.inserted_data.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::InsertedData { + fn insert_store_expressions(&mut self) -> Result<()> { + match self { + ast::InsertedData::Values { rows, .. } => rows.insert_store_expressions(), + ast::InsertedData::Select { query, .. } => query.insert_store_expressions(), + } + } +} + +impl InsertStoreExpressions for ast::ValuesRow { + fn insert_store_expressions(&mut self) -> Result<()> { + self.expressions.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::CreateTableStatement { + fn insert_store_expressions(&mut self) -> Result<()> { + self.definition.insert_store_expressions() + } +} + +impl InsertStoreExpressions for ast::CreateTableDefinition { + fn insert_store_expressions(&mut self) -> Result<()> { + match self { + // We don't need to do anything here because we aren't actually + // storing anything. It is a plain column definition. + ast::CreateTableDefinition::Columns { .. } => Ok(()), + ast::CreateTableDefinition::As { + query_statement, .. + } => query_statement.insert_store_expressions(), + } + } +} + +impl InsertStoreExpressions for ast::NodeVec { + fn insert_store_expressions(&mut self) -> Result<()> { + for item in self.node_iter_mut() { + item.insert_store_expressions()?; + } + Ok(()) + } +} diff --git a/src/infer/mod.rs b/src/infer/mod.rs index e0a8831..7cc9aba 100644 --- a/src/infer/mod.rs +++ b/src/infer/mod.rs @@ -17,8 +17,10 @@ use crate::{ }; use self::contains_aggregate::ContainsAggregate; +pub use self::insert_store_expressions::InsertStoreExpressions; mod contains_aggregate; +mod insert_store_expressions; // TODO: Remember this rather scary example. Verify BigQuery supports it // and that we need it. @@ -120,7 +122,7 @@ impl InferTypes for ast::CreateTableStatement { let ty = ValueType::try_from(&column.data_type)?; let col_ty = ColumnType { name: Some(column.name.clone()), - ty: ArgumentType::Value(ty), + ty: ArgumentType::Stored(ty), // TODO: We don't support this in the main grammar yet. not_null: false, }; @@ -201,10 +203,10 @@ impl InferTypes for ast::ValuesRow { // functions can't be used here. let scope = ColumnSetScope::new_empty(scope); let ty = expr.infer_types(&scope)?; - let ty = ty.expect_value_type(expr)?.to_owned(); + ty.expect_value_or_stored_type(expr)?; cols.push(ColumnType { name: None, - ty: ArgumentType::Value(ty), + ty, not_null: false, }); } @@ -418,14 +420,17 @@ impl InferTypes for ast::SelectExpression { // BigQuery does not allow select list items to see names // bound by other select list items. let ty = expression.infer_types(&column_set_scope)?; - // Make sure any aggregates have been turned into values. - let ty = ty.expect_value_type(expression)?.to_owned(); + // Make sure any aggregates have been turned into values or + // stored values (depending on whether this query is going + // to be stored, or where it's a sub-SELECT being input into + // a further calculation). + ty.expect_value_or_stored_type(expression)?; let name = alias .infer_column_name() .or_else(|| expression.infer_column_name()); cols.push(ColumnType { name, - ty: ArgumentType::Value(ty), + ty, not_null: false, }); } @@ -1386,9 +1391,21 @@ impl InferTypes for ast::LoadExpression { type Scope = ColumnSetScope; type Output = ArgumentType; + /// `self.expression` should have type `Stored`, and we return `?T`. + /// + /// For example, `?T` might map to UUID, and `Stored` might map to + /// VARCHAR, but that's someone else's problem. We only deal with this in + /// the abstract. fn infer_types(&mut self, scope: &Self::Scope) -> Result { - // TODO: More here. - self.expression.infer_types(scope) + let inferred_type = self.expression.infer_types(scope)?; + // Nobody should ever call us on any argument that doesn't have type + // `Stored`, because we're the load operation. + let value_type = + inferred_type.expect_stored_type_and_return_value_type(&self.expression)?; + // Record this for `emit` to use if needed. + self.memory_type = Some(value_type.to_owned()); + // Return the `?T` from our original `Stored`. + Ok(ArgumentType::Value(value_type.to_owned())) } } @@ -1396,9 +1413,18 @@ impl InferTypes for ast::StoreExpression { type Scope = ColumnSetScope; type Output = ArgumentType; + /// `self.expression` should have type `?T`, and we return `Stored`. + /// + /// For example, `?T` might map to UUID, and `Stored` might map to + /// VARCHAR, but that's someone else's problem. We only deal with this in + /// the abstract. fn infer_types(&mut self, scope: &Self::Scope) -> Result { - // TODO: More here. - self.expression.infer_types(scope) + let inferred_type = self.expression.infer_types(scope)?; + let value_type = inferred_type.expect_value_type(&self.expression)?; + // Record this for `emit` to use if needed. + self.memory_type = Some(value_type.to_owned()); + // Return the Stored for our original ?T. + Ok(ArgumentType::Stored(value_type.to_owned())) } } diff --git a/src/types.rs b/src/types.rs index 7537399..eab1cc2 100644 --- a/src/types.rs +++ b/src/types.rs @@ -159,6 +159,12 @@ impl fmt::Display for Type { pub enum ArgumentType { /// A value type. Value(ValueType), + /// A stored value type, which may have undergone a target-specific + /// transformation. For example, Trino supports `UUID` as an in-memory type, + /// but Trino's Hive storage connector does not allow `UUID` columns. So we + /// need to store `UUID` as a `VARCHAR` in Hive, and transform it on load + /// and store. + Stored(ValueType), /// An aggregating value type. Note that we can nest aggregating types. Aggregating(Box>), } @@ -183,6 +189,11 @@ impl ArgumentType { pub fn expect_value_type(&self, spanned: &dyn Spanned) -> Result<&ValueType> { match self { ArgumentType::Value(t) => Ok(t), + ArgumentType::Stored(_) => Err(Error::annotated( + format!("expected value type, found stored value type {}", self), + spanned.span(), + "type mismatch", + )), ArgumentType::Aggregating(_) => Err(Error::annotated( format!("expected value type, found aggregate type {}", self), spanned.span(), @@ -191,6 +202,44 @@ impl ArgumentType { } } + /// Expect a [`ArgumentType::Value`] or [`ArgumentType::Stored`]. We don't + /// return the matched type because you probably want to use `self` for any + /// downstream processing. This is just a type check. + pub fn expect_value_or_stored_type(&self, spanned: &dyn Spanned) -> Result<()> { + match self { + ArgumentType::Value(_) => Ok(()), + ArgumentType::Stored(_) => Ok(()), + ArgumentType::Aggregating(_) => Err(Error::annotated( + format!( + "expected value or stored type, found aggregate type {}", + self + ), + spanned.span(), + "type mismatch", + )), + } + } + + /// Expect a [`ArgumentType::Stored`] and return the `ValueType` it contains. + pub fn expect_stored_type_and_return_value_type( + &self, + spanned: &dyn Spanned, + ) -> Result<&ValueType> { + match self { + ArgumentType::Value(_) => Err(Error::annotated( + format!("expected stored type, found in-memory value type {}", self), + spanned.span(), + "type mismatch", + )), + ArgumentType::Stored(t) => Ok(t), + ArgumentType::Aggregating(_) => Err(Error::annotated( + format!("expected stored type, found aggregate type {}", self), + spanned.span(), + "type mismatch", + )), + } + } + /// Expect a [`SimpleType`]. pub fn expect_simple_type(&self, spanned: &dyn Spanned) -> Result<&SimpleType> { match self { @@ -248,6 +297,7 @@ impl ArgumentType { // at least until we discover otherwise. match (self, other) { (ArgumentType::Value(a), ArgumentType::Value(b)) => a.is_subtype_of(b), + (ArgumentType::Stored(a), ArgumentType::Stored(b)) => a.is_subtype_of(b), (ArgumentType::Aggregating(a), ArgumentType::Aggregating(b)) => a.is_subtype_of(b), _ => false, } @@ -309,6 +359,7 @@ impl Unify for ArgumentType { fn resolve(&self, table: &UnificationTable, spanned: &dyn Spanned) -> Result { match self { ArgumentType::Value(t) => Ok(ArgumentType::Value(t.resolve(table, spanned)?)), + ArgumentType::Stored(t) => Ok(ArgumentType::Stored(t.resolve(table, spanned)?)), ArgumentType::Aggregating(t) => Ok(ArgumentType::Aggregating(Box::new( t.resolve(table, spanned)?, ))), @@ -320,6 +371,7 @@ impl fmt::Display for ArgumentType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { ArgumentType::Value(v) => write!(f, "{}", v), + ArgumentType::Stored(v) => write!(f, "Stored<{}>", v), ArgumentType::Aggregating(v) => write!(f, "Agg<{}>", v), } } @@ -1075,7 +1127,12 @@ impl ColumnType { /// [`TableType::name_anonymous_columns`]. pub fn expect_creatable(&self, spanned: &dyn Spanned) -> Result<()> { match &self.ty { - ArgumentType::Value(ty) => ty.expect_inhabited(spanned), + ArgumentType::Value(ty) => Err(Error::annotated( + format!("Internal error: cannot store {} in a column unless it has been converted to a storage type", ty), + spanned.span(), + "missing conversion to storage type", + )), + ArgumentType::Stored(ty) => ty.expect_inhabited(spanned), ArgumentType::Aggregating(_) => Err(Error::annotated( "Cannot store an aggregate column", spanned.span(), From 4eadb97a9ed61ec241e5e1176ac146ee5b186d95 Mon Sep 17 00:00:00 2001 From: Eric Kidd Date: Sat, 7 Dec 2024 13:49:32 -0500 Subject: [PATCH 2/5] Fix warnings --- src/ast.rs | 1 - src/cmd/run.rs | 5 +---- src/cmd/sql_test.rs | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 775db10..68584f1 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1691,7 +1691,6 @@ impl Emit for LoadExpression { self.expression.emit(t, f)?; f.write_token_start(&suffix) } - _ => self.emit_default(t, f), } } } diff --git a/src/cmd/run.rs b/src/cmd/run.rs index 7a40758..29a18fb 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -3,10 +3,7 @@ use std::path::PathBuf; use clap::Parser; use tracing::instrument; -use crate::{ - ast::parse_sql, drivers, errors::Result, infer::InferTypes, known_files::KnownFiles, - scope::Scope, -}; +use crate::{ast::parse_sql, drivers, errors::Result, known_files::KnownFiles}; /// Run an SQL file using the specified database. #[derive(Debug, Parser)] diff --git a/src/cmd/sql_test.rs b/src/cmd/sql_test.rs index f5787b1..a208ec7 100644 --- a/src/cmd/sql_test.rs +++ b/src/cmd/sql_test.rs @@ -17,9 +17,7 @@ use crate::{ ast::{self, parse_sql, CreateTableStatement, CreateViewStatement, Target}, drivers::{self, Driver}, errors::{format_err, Context, Error, Result}, - infer::InferTypes, known_files::{FileId, KnownFiles}, - scope::Scope, }; /// Run SQL tests from a directory. From edb4a05dab194288a4d933700d5fdcc40ff82a02 Mon Sep 17 00:00:00 2001 From: Eric Kidd Date: Sat, 7 Dec 2024 15:03:23 -0500 Subject: [PATCH 3/5] Add in some missing Stored<..> branches --- src/types.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/types.rs b/src/types.rs index eab1cc2..1cd90d4 100644 --- a/src/types.rs +++ b/src/types.rs @@ -323,7 +323,9 @@ impl ArgumentType { (ArgumentType::Value(a), ArgumentType::Value(b)) => { Some(ArgumentType::Value(a.common_supertype(b)?)) } - + (Self::Stored(a), Self::Stored(b)) => { + Some(ArgumentType::Stored(a.common_supertype(b)?)) + } (ArgumentType::Aggregating(a), ArgumentType::Aggregating(b)) => { Some(ArgumentType::Aggregating(Box::new(a.common_supertype(b)?))) } @@ -345,6 +347,9 @@ impl Unify for ArgumentType { (ArgumentType::Value(a), ArgumentType::Value(b)) => { Ok(ArgumentType::Value(a.unify(b, table, spanned)?)) } + (ArgumentType::Stored(a), ArgumentType::Stored(b)) => { + Ok(ArgumentType::Stored(a.unify(b, table, spanned)?)) + } (ArgumentType::Aggregating(a), ArgumentType::Aggregating(b)) => Ok( ArgumentType::Aggregating(Box::new(a.unify(b, table, spanned)?)), ), From 2f2b3fde178445961a020685a63a2aa3debfde50 Mon Sep 17 00:00:00 2001 From: Eric Kidd Date: Sun, 8 Dec 2024 19:15:50 -0500 Subject: [PATCH 4/5] WIP: Implement LOAD operations This turned out to be surprisingly tricky. The key insight is that LOAD and STORE aren't really symmetrical, at least not in this version of the code. Instead, %STORE(expr) gets manually inserted as a phantom operator, but %LOAD(var_ref) only occurs as part of a variable reference. This means that transforms, `sql_quote!` and all our other rewriting machinery don't even notice that LOADs exist. This is a weird decision, and it might not be optimal in the long run, but it works. This brings us down to `7 tests failed, 64 passed, 3 pending`, and all the failing test cases are places we need to expand SELECT wildcards like `*`, `table.*`, etc., into column lists with explicit STORE operations. --- src/ast.rs | 215 ++++++++++++++++---------------- src/infer/contains_aggregate.rs | 9 +- src/infer/mod.rs | 119 ++++++++++-------- src/types.rs | 56 ++++++--- 4 files changed, 214 insertions(+), 185 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 68584f1..8d8eefb 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -837,7 +837,8 @@ pub enum Expression { Literal(Literal), BoolValue(Keyword), Null(Keyword), - Name(Name), + Name(NameExpression), + Store(StoreExpression), Cast(Cast), Is(IsExpression), In(InExpression), @@ -867,8 +868,6 @@ pub enum Expression { FunctionCall(FunctionCall), Index(IndexExpression), FieldAccess(FieldAccessExpression), - Load(LoadExpression), - Store(StoreExpression), } impl Expression { @@ -913,6 +912,111 @@ impl DatePart { } } +/// A "load" expression, which transforms an SQL value from a "storage" type (eg +/// "VARCHAR") to a "memory" type (eg "UUID"). Used for databases like Trino, +/// where the storage types for a given connector may be more limited than the +/// standard Trino memory types. +/// +/// These are not found in the original parsed AST, but are added while +/// transforming the AST. +#[derive(Clone, Debug, Drive, DriveMut, EmitDefault, Spanned, ToTokens)] +pub struct NameExpression { + /// **If** we need to do a load conversion, this will be the inferred memory + /// type. + #[emit(skip)] + #[to_tokens(skip)] + #[drive(skip)] + pub load_to_memory_type: Option, + + /// Our underlying expression. + pub name: Name, +} + +impl Emit for NameExpression { + fn emit(&self, t: Target, f: &mut TokenWriter<'_>) -> ::std::io::Result<()> { + match t { + // Target::BigQuery => { + // f.write_token_start("%LOAD(")?; + // self.name.emit(t, f)?; + // f.write_token_start(")") + // } + Target::Trino(connector_type) if self.load_to_memory_type.is_some() => { + let bq_memory_type = self + .load_to_memory_type + .as_ref() + .expect("memory_type should have been filled in by type inference"); + let trino_memory_type = + TrinoDataType::try_from(bq_memory_type).map_err(io::Error::other)?; + let transform = connector_type.storage_transform_for(&trino_memory_type); + let (prefix, suffix) = transform.load_prefix_and_suffix(); + + // Wrapping the expression in our prefix and suffix. + // If the expression was col_name containing '[1,2]' in Trino, + // BQ memory type -> JSON, Trino memory type -> JSON, Trino storage type -> VARCHAR + // The Trino storage type is dependent on what the connector can support. + // In this case, the wrapped version would be JSON_PARSE(col_name) + f.write_token_start(&prefix)?; + self.name.emit(t, f)?; + f.write_token_start(&suffix) + } + _ => self.name.emit(t, f), + } + } +} + +/// A "store" expression, which transforms an SQL value from a "memory" type +/// (eg "UUID") to a "storage" type (eg "VARCHAR"). Used for databases like +/// Trino, where the storage types for a given connector may be more limited +/// than the standard Trino memory types. +/// +/// These are not found in the original parsed AST, but are added while +/// transforming the AST. +#[derive(Clone, Debug, Drive, DriveMut, EmitDefault, Spanned, ToTokens)] +pub struct StoreExpression { + /// Inferred memory type. + #[emit(skip)] + #[to_tokens(skip)] + #[drive(skip)] + pub memory_type: Option, + + /// Our underlying expression. + pub expression: Box, +} + +impl Emit for StoreExpression { + fn emit(&self, t: Target, f: &mut TokenWriter<'_>) -> ::std::io::Result<()> { + match t { + Target::BigQuery => { + f.write_token_start("%STORE(")?; + self.expression.emit(t, f)?; + f.write_token_start(")") + } + Target::Trino(connector_type) => { + let bq_memory_type = self + .memory_type + .as_ref() + .expect("memory_type should have been filled in by type inference"); + + // If our bq_memory_type is NULL, we don't need to do any transforms because + // NULL is NULL in both storage and memory types and dbcrossbar_trino doesn't + // support NULL as a memory type. + if let ValueType::Simple(SimpleType::Null) = bq_memory_type { + self.expression.emit(t, f) + } else { + let trino_memory_type = + TrinoDataType::try_from(bq_memory_type).map_err(io::Error::other)?; + let transform = connector_type.storage_transform_for(&trino_memory_type); + let (prefix, suffix) = transform.store_prefix_and_suffix(); + + f.write_token_start(&prefix)?; + self.expression.emit(t, f)?; + f.write_token_start(&suffix) + } + } + } + } +} + /// A cast expression. #[derive(Clone, Debug, Drive, DriveMut, Emit, EmitDefault, Spanned, ToTokens)] pub struct Cast { @@ -1645,109 +1749,6 @@ pub struct FieldAccessExpression { pub field_name: Ident, } -/// A "load" expression, which transforms an SQL value from a "storage" type (eg -/// "VARCHAR") to a "memory" type (eg "UUID"). Used for databases like Trino, -/// where the storage types for a given connector may be more limited than the -/// standard Trino memory types. -/// -/// These are not found in the original parsed AST, but are added while -/// transforming the AST. -#[derive(Clone, Debug, Drive, DriveMut, EmitDefault, Spanned, ToTokens)] -pub struct LoadExpression { - /// Inferred memory type. - #[emit(skip)] - #[to_tokens(skip)] - #[drive(skip)] - pub memory_type: Option, - - /// Our underlying expression. - pub expression: Box, -} - -impl Emit for LoadExpression { - fn emit(&self, t: Target, f: &mut TokenWriter<'_>) -> ::std::io::Result<()> { - match t { - Target::BigQuery => { - f.write_token_start("%LOAD(")?; - self.expression.emit(t, f)?; - f.write_token_start(")") - } - Target::Trino(connector_type) => { - let bq_memory_type = self - .memory_type - .as_ref() - .expect("memory_type should have been filled in by type inference"); - let trino_memory_type = - TrinoDataType::try_from(bq_memory_type).map_err(io::Error::other)?; - let transform = connector_type.storage_transform_for(&trino_memory_type); - let (prefix, suffix) = transform.load_prefix_and_suffix(); - - // Wrapping the expression in our prefix and suffix. - // If the expression was col_name containing '[1,2]' in Trino, - // BQ memory type -> JSON, Trino memory type -> JSON, Trino storage type -> VARCHAR - // The Trino storage type is dependent on what the connector can support. - // In this case, the wrapped version would be JSON_PARSE(col_name) - f.write_token_start(&prefix)?; - self.expression.emit(t, f)?; - f.write_token_start(&suffix) - } - } - } -} - -/// A "store" expression, which transforms an SQL value from a "memory" type -/// (eg "UUID") to a "storage" type (eg "VARCHAR"). Used for databases like -/// Trino, where the storage types for a given connector may be more limited -/// than the standard Trino memory types. -/// -/// These are not found in the original parsed AST, but are added while -/// transforming the AST. -#[derive(Clone, Debug, Drive, DriveMut, EmitDefault, Spanned, ToTokens)] -pub struct StoreExpression { - /// Inferred memory type. - #[emit(skip)] - #[to_tokens(skip)] - #[drive(skip)] - pub memory_type: Option, - - /// Our underlying expression. - pub expression: Box, -} - -impl Emit for StoreExpression { - fn emit(&self, t: Target, f: &mut TokenWriter<'_>) -> ::std::io::Result<()> { - match t { - Target::BigQuery => { - f.write_token_start("%STORE(")?; - self.expression.emit(t, f)?; - f.write_token_start(")") - } - Target::Trino(connector_type) => { - let bq_memory_type = self - .memory_type - .as_ref() - .expect("memory_type should have been filled in by type inference"); - - // If our bq_memory_type is NULL, we don't need to do any transforms because - // NULL is NULL in both storage and memory types and dbcrossbar_trino doesn't - // support NULL as a memory type. - if let ValueType::Simple(SimpleType::Null) = bq_memory_type { - self.expression.emit(t, f) - } else { - let trino_memory_type = - TrinoDataType::try_from(bq_memory_type).map_err(io::Error::other)?; - let transform = connector_type.storage_transform_for(&trino_memory_type); - let (prefix, suffix) = transform.store_prefix_and_suffix(); - - f.write_token_start(&prefix)?; - self.expression.emit(t, f)?; - f.write_token_start(&suffix) - } - } - } - } -} - /// An `AS` alias. #[derive(Clone, Debug, Drive, DriveMut, Emit, EmitDefault, Spanned, ToTokens)] pub struct Alias { @@ -2446,7 +2447,7 @@ peg::parser! { // Things from here down might start with arbitrary identifiers, so // we need to be careful about the order. function_call:function_call() { Expression::FunctionCall(function_call) } - column_name:name() { Expression::Name(column_name) } + column_name:name() { Expression::Name(NameExpression { load_to_memory_type: None, name: column_name }) } } rule interval_expression() -> IntervalExpression diff --git a/src/infer/contains_aggregate.rs b/src/infer/contains_aggregate.rs index 65076eb..230c544 100644 --- a/src/infer/contains_aggregate.rs +++ b/src/infer/contains_aggregate.rs @@ -65,6 +65,7 @@ impl ContainsAggregate for ast::Expression { ast::Expression::BoolValue(_) => false, ast::Expression::Null(_) => false, ast::Expression::Name(_) => false, + ast::Expression::Store(store_expr) => store_expr.contains_aggregate(scope), ast::Expression::Cast(cast) => cast.contains_aggregate(scope), ast::Expression::Is(is) => is.contains_aggregate(scope), ast::Expression::In(in_expr) => in_expr.contains_aggregate(scope), @@ -91,8 +92,6 @@ impl ContainsAggregate for ast::Expression { // Putting an aggregate here would be very weird. Do not allow it // until forced to do so. ast::Expression::FieldAccess(_) => false, - ast::Expression::Load(load_expr) => load_expr.contains_aggregate(scope), - ast::Expression::Store(store_expr) => store_expr.contains_aggregate(scope), } } } @@ -269,12 +268,6 @@ impl ContainsAggregate for ast::IndexOffset { } } -impl ContainsAggregate for ast::LoadExpression { - fn contains_aggregate(&self, scope: &ColumnSetScope) -> bool { - self.expression.contains_aggregate(scope) - } -} - impl ContainsAggregate for ast::StoreExpression { fn contains_aggregate(&self, scope: &ColumnSetScope) -> bool { self.expression.contains_aggregate(scope) diff --git a/src/infer/mod.rs b/src/infer/mod.rs index 7cc9aba..cbd0d15 100644 --- a/src/infer/mod.rs +++ b/src/infer/mod.rs @@ -676,7 +676,7 @@ impl InferTypes for ast::GroupBy { for expr in self.expressions.node_iter_mut() { let _ty = expr.infer_types(scope)?; match expr { - Expression::Name(name) => { + Expression::Name(ast::NameExpression { name, .. }) => { group_by_names.push(name.clone()); } _ => { @@ -719,7 +719,8 @@ impl InferTypes for ast::Expression { ast::Expression::Literal(Literal { value, .. }) => value.infer_types(&()), ast::Expression::BoolValue(_) => Ok(ArgumentType::bool()), ast::Expression::Null { .. } => Ok(ArgumentType::null()), - ast::Expression::Name(name) => name.infer_types(scope), + ast::Expression::Name(name_expr) => name_expr.infer_types(scope), + ast::Expression::Store(store_expr) => store_expr.infer_types(scope), ast::Expression::Cast(cast) => cast.infer_types(scope), ast::Expression::Is(is) => is.infer_types(scope), ast::Expression::In(in_expr) => in_expr.infer_types(scope), @@ -744,8 +745,6 @@ impl InferTypes for ast::Expression { ast::Expression::FunctionCall(fcall) => fcall.infer_types(scope), ast::Expression::Index(index) => index.infer_types(scope), ast::Expression::FieldAccess(field_access) => field_access.infer_types(scope), - ast::Expression::Load(load_expr) => load_expr.infer_types(scope), - ast::Expression::Store(store_expr) => store_expr.infer_types(scope), } } } @@ -774,6 +773,44 @@ impl InferTypes for Ident { } } +impl InferTypes for ast::NameExpression { + type Scope = ColumnSetScope; + type Output = ArgumentType; + + /// `self.name` may be a bare type `?T`, an `Agg` type (possibly + /// nested), or type involving `Stored<..>`, such as `Stored`, + /// `Agg>` or even `Agg>>` to any depth. + /// + /// We have two jobs: + /// + /// 1. Record what type we need to load from, if any. In the examples above, + /// this would be `?T`. + /// 2. Infer the type after the load, which removed `Stored<..>` but keeps + /// `Agg<..>` if present. + /// + /// | Name type | Load to memory type | Inferrred type | + /// |------------------------|---------------------|----------------| + /// | `?T` | `None` | `?T` | + /// | `Agg` | `None` | `Agg` | + /// | `Agg>` | `None` | `Agg>` | + /// | `Stored` | `?T` | `?T` | + /// | `Agg>` | `?T` | `Agg` | + /// | `Agg>>` | `?T` | `Agg>` | + fn infer_types(&mut self, scope: &Self::Scope) -> Result { + let inferred_type = self.name.infer_types(scope)?; + // Do we need to perform a load operation? + if let Some(load_to_memory_type) = inferred_type.load_to_memory_type() { + // Record this for `emit` to use. + self.load_to_memory_type = Some(load_to_memory_type.to_owned()); + // Remove `Stored<..>` from our type. + inferred_type.type_after_load() + } else { + self.load_to_memory_type = None; + Ok(inferred_type) + } + } +} + impl InferTypes for ast::Name { type Scope = ColumnSetScope; type Output = ArgumentType; @@ -823,6 +860,25 @@ impl InferTypes for ast::Name { } } +impl InferTypes for ast::StoreExpression { + type Scope = ColumnSetScope; + type Output = ArgumentType; + + /// `self.expression` should have type `?T`, and we return `Stored`. + /// + /// For example, `?T` might map to UUID, and `Stored` might map to + /// VARCHAR, but that's someone else's problem. We only deal with this in + /// the abstract. + fn infer_types(&mut self, scope: &Self::Scope) -> Result { + let inferred_type = self.expression.infer_types(scope)?; + let value_type = inferred_type.expect_value_type(&self.expression)?; + // Record this for `emit` to use if needed. + self.memory_type = Some(value_type.to_owned()); + // Return the Stored for our original ?T. + Ok(ArgumentType::Stored(value_type.to_owned())) + } +} + impl InferTypes for ast::Cast { type Scope = ColumnSetScope; type Output = ArgumentType; @@ -1346,7 +1402,7 @@ impl InferTypes for ast::PartitionBy { let mut partition_by_names = vec![]; for expr in self.expressions.node_iter_mut() { match expr { - ast::Expression::Name(name) => { + ast::Expression::Name(ast::NameExpression { name, .. }) => { scope.get_argument_type(name)?; partition_by_names.push(name.clone()); } @@ -1387,47 +1443,6 @@ impl InferTypes for ast::FieldAccessExpression { } } -impl InferTypes for ast::LoadExpression { - type Scope = ColumnSetScope; - type Output = ArgumentType; - - /// `self.expression` should have type `Stored`, and we return `?T`. - /// - /// For example, `?T` might map to UUID, and `Stored` might map to - /// VARCHAR, but that's someone else's problem. We only deal with this in - /// the abstract. - fn infer_types(&mut self, scope: &Self::Scope) -> Result { - let inferred_type = self.expression.infer_types(scope)?; - // Nobody should ever call us on any argument that doesn't have type - // `Stored`, because we're the load operation. - let value_type = - inferred_type.expect_stored_type_and_return_value_type(&self.expression)?; - // Record this for `emit` to use if needed. - self.memory_type = Some(value_type.to_owned()); - // Return the `?T` from our original `Stored`. - Ok(ArgumentType::Value(value_type.to_owned())) - } -} - -impl InferTypes for ast::StoreExpression { - type Scope = ColumnSetScope; - type Output = ArgumentType; - - /// `self.expression` should have type `?T`, and we return `Stored`. - /// - /// For example, `?T` might map to UUID, and `Stored` might map to - /// VARCHAR, but that's someone else's problem. We only deal with this in - /// the abstract. - fn infer_types(&mut self, scope: &Self::Scope) -> Result { - let inferred_type = self.expression.infer_types(scope)?; - let value_type = inferred_type.expect_value_type(&self.expression)?; - // Record this for `emit` to use if needed. - self.memory_type = Some(value_type.to_owned()); - // Return the Stored for our original ?T. - Ok(ArgumentType::Stored(value_type.to_owned())) - } -} - /// Figure out whether an expression defines an implicit column name. pub trait InferColumnName { /// Infer the column name, if any. @@ -1446,15 +1461,19 @@ impl InferColumnName for Option { impl InferColumnName for ast::Expression { fn infer_column_name(&mut self) -> Option { match self { - ast::Expression::Name(name) => { - let (_table, col) = name.split_table_and_column(); - Some(col) - } + ast::Expression::Name(name) => name.infer_column_name(), _ => None, } } } +impl InferColumnName for ast::NameExpression { + fn infer_column_name(&mut self) -> Option { + let (_table, col) = self.name.split_table_and_column(); + Some(col) + } +} + impl InferColumnName for ast::Alias { fn infer_column_name(&mut self) -> Option { Some(self.ident.clone()) diff --git a/src/types.rs b/src/types.rs index 1cd90d4..3d1d66e 100644 --- a/src/types.rs +++ b/src/types.rs @@ -220,26 +220,6 @@ impl ArgumentType { } } - /// Expect a [`ArgumentType::Stored`] and return the `ValueType` it contains. - pub fn expect_stored_type_and_return_value_type( - &self, - spanned: &dyn Spanned, - ) -> Result<&ValueType> { - match self { - ArgumentType::Value(_) => Err(Error::annotated( - format!("expected stored type, found in-memory value type {}", self), - spanned.span(), - "type mismatch", - )), - ArgumentType::Stored(t) => Ok(t), - ArgumentType::Aggregating(_) => Err(Error::annotated( - format!("expected stored type, found aggregate type {}", self), - spanned.span(), - "type mismatch", - )), - } - } - /// Expect a [`SimpleType`]. pub fn expect_simple_type(&self, spanned: &dyn Spanned) -> Result<&SimpleType> { match self { @@ -334,6 +314,42 @@ impl ArgumentType { } } +// Methods which only work after we've resolved type variables. +impl ArgumentType { + /// Get the type we should load as, if we are stored. + /// + /// Conceptually, we load before we aggregate, so if you pass + /// `Agg>>`, this will return `Some(T)`. We need to know this + /// type to generate appropriate loading code (which does not care about + /// aggregation). + /// + /// This will normally be followed up by calling [`Self::type_after_load`]. + pub fn load_to_memory_type(&self) -> Option { + match self { + ArgumentType::Value(_) => None, + ArgumentType::Stored(value_type) => Some(value_type.clone()), + ArgumentType::Aggregating(argument_type) => argument_type.load_to_memory_type(), + } + } + + /// The type of this argument after we've loaded it into memory. This will included any + /// aggregations. + /// + /// For example, if we have `Agg>>`, this will return + /// `Agg>`. + /// + /// This is normally called after [`Self::load_to_memory_type`]. + pub fn type_after_load(&self) -> Result { + match self { + ArgumentType::Value(_) => Err(format_err!("cannot load a value type")), + ArgumentType::Stored(value_type) => Ok(ArgumentType::Value(value_type.clone())), + ArgumentType::Aggregating(argument_type) => Ok(ArgumentType::Aggregating(Box::new( + argument_type.type_after_load()?, + ))), + } + } +} + impl Unify for ArgumentType { type Resolved = ArgumentType; From a7c1722bdc5650c0e05dd62362a240cf24ecfe80 Mon Sep 17 00:00:00 2001 From: Eric Kidd Date: Sun, 8 Dec 2024 19:43:18 -0500 Subject: [PATCH 5/5] WIP: Re-enable tests for CAST(NULL ..) --- tests/sql/data_types/cast_types.sql | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/sql/data_types/cast_types.sql b/tests/sql/data_types/cast_types.sql index 35037c2..ff5958c 100644 --- a/tests/sql/data_types/cast_types.sql +++ b/tests/sql/data_types/cast_types.sql @@ -1,5 +1,3 @@ --- pending: trino TESTS ONLY: Rust driver seems to have problems with NULL in certain columns - -- CAST(NULL AS ) for common types. CREATE OR REPLACE TABLE __result1 AS SELECT @@ -8,7 +6,7 @@ SELECT CAST(NULL AS INT64) AS null_int64, CAST(NULL AS FLOAT64) AS null_float64, -- We deal with NUMERIC separately. - -- + -- -- CAST(NULL AS NUMERIC) AS null_numeric, -- -- The only reason to use this is to never lose data, but there is no @@ -28,9 +26,11 @@ SELECT -- -- CAST(NULL AS GEOGRAPHY) AS null_geography, CAST(NULL AS ARRAY) AS null_array_bool, - CAST(NULL AS ARRAY) AS null_array_int64, - CAST(NULL AS STRUCT) AS null_struct_with_named_fields, - CAST(NULL AS STRUCT) AS null_struct_with_unnamed_fields; + CAST(NULL AS ARRAY) AS null_array_int64; + -- trino: Row types don't seem to allow NULL + -- + --CAST(NULL AS STRUCT) AS null_struct_with_named_fields, + --CAST(NULL AS STRUCT) AS null_struct_with_unnamed_fields; CREATE OR REPLACE TABLE __expected1 ( null_bool BOOL, @@ -48,8 +48,8 @@ CREATE OR REPLACE TABLE __expected1 ( -- null_geography GEOGRAPHY, null_array_bool ARRAY, null_array_int64 ARRAY, - null_struct_with_named_fields STRUCT, - null_struct_with_unnamed_fields STRUCT, + --null_struct_with_named_fields STRUCT, + --null_struct_with_unnamed_fields STRUCT, ); INSERT INTO __expected1 VALUES ( NULL, -- null_bool @@ -67,6 +67,6 @@ INSERT INTO __expected1 VALUES ( -- NULL, -- null_geography NULL, -- null_array_bool NULL, -- null_array_int64 - NULL, -- null_struct_with_named_fields - NULL, -- null_struct_with_unnamed_fields -); \ No newline at end of file + --NULL, -- null_struct_with_named_fields + --NULL, -- null_struct_with_unnamed_fields +);