From b3107efdd1b01bc90e4608461f08ce36c331bf7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Wed, 24 Jan 2024 13:03:36 +0100 Subject: [PATCH 1/8] feat: vars without value --- prqlc/prqlc-ast/src/stmt.rs | 2 +- prqlc/prqlc-parser/src/stmt.rs | 5 ++-- prqlc/prqlc-parser/src/test.rs | 2 +- prqlc/prqlc/src/cli/mod.rs | 2 +- prqlc/prqlc/src/codegen/ast.rs | 30 +++++++++++++------- prqlc/prqlc/src/ir/pl/fold.rs | 2 +- prqlc/prqlc/src/ir/pl/stmt.rs | 2 +- prqlc/prqlc/src/semantic/ast_expand.rs | 12 ++++---- prqlc/prqlc/src/semantic/eval.rs | 2 +- prqlc/prqlc/src/semantic/resolver/expr.rs | 8 +++--- prqlc/prqlc/src/semantic/resolver/generic.rs | 2 ++ prqlc/prqlc/src/semantic/resolver/stmt.rs | 29 ++++++++++++++----- 12 files changed, 62 insertions(+), 36 deletions(-) create mode 100644 prqlc/prqlc/src/semantic/resolver/generic.rs diff --git a/prqlc/prqlc-ast/src/stmt.rs b/prqlc/prqlc-ast/src/stmt.rs index 7acb74d270a3..c36a66244265 100644 --- a/prqlc/prqlc-ast/src/stmt.rs +++ b/prqlc/prqlc-ast/src/stmt.rs @@ -45,7 +45,7 @@ pub enum StmtKind { pub struct VarDef { pub kind: VarDefKind, pub name: String, - pub value: Box, + pub value: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub ty: Option, diff --git a/prqlc/prqlc-parser/src/stmt.rs b/prqlc/prqlc-parser/src/stmt.rs index 10497aa5f48d..c83b1dcf5612 100644 --- a/prqlc/prqlc-parser/src/stmt.rs +++ b/prqlc/prqlc-parser/src/stmt.rs @@ -107,8 +107,7 @@ fn var_def() -> impl Parser { let let_ = keyword("let") .ignore_then(ident_part()) .then(type_expr().delimited_by(ctrl('<'), ctrl('>')).or_not()) - .then_ignore(ctrl('=')) - .then(expr_call().map(Box::new)) + .then(ctrl('=').ignore_then(expr_call()).map(Box::new).or_not()) .map(|((name, ty), value)| { StmtKind::VarDef(VarDef { name, @@ -133,7 +132,7 @@ fn var_def() -> impl Parser { StmtKind::VarDef(VarDef { name, kind, - value, + value: Some(value), ty: None, }) }) diff --git a/prqlc/prqlc-parser/src/test.rs b/prqlc/prqlc-parser/src/test.rs index d8db715cf783..7d5c696d0fb7 100644 --- a/prqlc/prqlc-parser/src/test.rs +++ b/prqlc/prqlc-parser/src/test.rs @@ -16,7 +16,7 @@ fn parse_expr(source: &str) -> Result> { let stmts = parse_single(&source)?; let stmt = stmts.into_iter().exactly_one().unwrap(); - Ok(*stmt.kind.into_var_def().unwrap().value) + Ok(*stmt.kind.into_var_def().unwrap().value.unwrap()) } #[test] diff --git a/prqlc/prqlc/src/cli/mod.rs b/prqlc/prqlc/src/cli/mod.rs index 8574469ea922..bfbe92d8a440 100644 --- a/prqlc/prqlc/src/cli/mod.rs +++ b/prqlc/prqlc/src/cli/mod.rs @@ -361,7 +361,7 @@ impl Command { if let StmtKind::VarDef(def) = stmt.kind { res += &format!("## {}\n", def.name); - let val = semantic::eval(*def.value) + let val = semantic::eval(*def.value.unwrap()) .map_err(downcast) .map_err(|e| e.composed(sources))?; res += &semantic::write_pl(val); diff --git a/prqlc/prqlc/src/codegen/ast.rs b/prqlc/prqlc/src/codegen/ast.rs index ea1e9140f987..33c3df63cc43 100644 --- a/prqlc/prqlc/src/codegen/ast.rs +++ b/prqlc/prqlc/src/codegen/ast.rs @@ -346,20 +346,31 @@ impl WriteSource for Stmt { r += "\n"; } StmtKind::VarDef(var_def) => match var_def.kind { - VarDefKind::Let => { + _ if var_def.value.is_none() || var_def.ty.is_some() => { let typ = if let Some(ty) = &var_def.ty { format!("<{}> ", ty.write(opt.clone())?) } else { "".to_string() }; - r += opt.consume(&format!("let {} {}= ", var_def.name, typ))?; + r += opt.consume(&format!("let {} {}", var_def.name, typ))?; + + if let Some(val) = &var_def.value { + r += opt.consume("= ")?; + r += &val.write(opt)?; + } + r += "\n"; + } + + VarDefKind::Let => { + r += opt.consume(&format!("let {} = ", var_def.name))?; - r += &var_def.value.write(opt)?; + r += &var_def.value.as_ref().unwrap().write(opt)?; r += "\n"; } VarDefKind::Into | VarDefKind::Main => { - match &var_def.value.kind { + let val = var_def.value.as_ref().unwrap(); + match &val.kind { ExprKind::Pipeline(pipeline) => { for expr in &pipeline.exprs { r += &expr.write(opt.clone())?; @@ -367,7 +378,7 @@ impl WriteSource for Stmt { } } _ => { - r += &var_def.value.write(opt)?; + r += &val.write(opt)?; } } @@ -433,14 +444,13 @@ impl WriteSource for SwitchCase { #[cfg(test)] mod test { use insta::assert_snapshot; - use similar_asserts::assert_eq; use super::*; + #[track_caller] fn assert_is_formatted(input: &str) { - let stmt = format_single_stmt(input); - - assert_eq!(input.trim(), stmt.trim()); + let formatted = format_single_stmt(input); + similar_asserts::assert_eq!(input.trim(), formatted.trim()); } fn format_single_stmt(query: &str) -> String { @@ -511,7 +521,7 @@ mod test { assert_is_formatted(r#"sort {-duration}"#); assert_is_formatted(r#"select a = -b"#); - assert_is_formatted(r#"join `project-bar.dataset.table` (==col_bax)"#) + assert_is_formatted(r#"join `project-bar.dataset.table` (==col_bax)"#); } #[test] diff --git a/prqlc/prqlc/src/ir/pl/fold.rs b/prqlc/prqlc/src/ir/pl/fold.rs index 2be50242e7d1..6b9d9e824144 100644 --- a/prqlc/prqlc/src/ir/pl/fold.rs +++ b/prqlc/prqlc/src/ir/pl/fold.rs @@ -128,7 +128,7 @@ fn fold_module_def(fold: &mut F, module_def: ModuleDef) -> R pub fn fold_var_def(fold: &mut F, var_def: VarDef) -> Result { Ok(VarDef { name: var_def.name, - value: Box::new(fold.fold_expr(*var_def.value)?), + value: fold_optional_box(fold, var_def.value)?, ty: var_def.ty.map(|x| fold.fold_type(x)).transpose()?, }) } diff --git a/prqlc/prqlc/src/ir/pl/stmt.rs b/prqlc/prqlc/src/ir/pl/stmt.rs index cdc007450980..3bd212f6d961 100644 --- a/prqlc/prqlc/src/ir/pl/stmt.rs +++ b/prqlc/prqlc/src/ir/pl/stmt.rs @@ -33,7 +33,7 @@ pub enum StmtKind { #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] pub struct VarDef { pub name: String, - pub value: Box, + pub value: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub ty: Option, diff --git a/prqlc/prqlc/src/semantic/ast_expand.rs b/prqlc/prqlc/src/semantic/ast_expand.rs index d7f7a7033286..26ccc547d881 100644 --- a/prqlc/prqlc/src/semantic/ast_expand.rs +++ b/prqlc/prqlc/src/semantic/ast_expand.rs @@ -239,7 +239,7 @@ fn expand_stmt_kind(value: StmtKind) -> Result { StmtKind::QueryDef(v) => pl::StmtKind::QueryDef(v), StmtKind::VarDef(v) => pl::StmtKind::VarDef(pl::VarDef { name: v.name, - value: expand_expr_box(v.value)?, + value: v.value.map(expand_expr_box).transpose()?, ty: v.ty, }), StmtKind::TypeDef(v) => pl::StmtKind::TypeDef(pl::TypeDef { @@ -391,7 +391,7 @@ fn restrict_stmt(stmt: pl::Stmt) -> Stmt { pl::StmtKind::VarDef(def) => StmtKind::VarDef(prqlc_ast::VarDef { kind: VarDefKind::Let, name: def.name, - value: restrict_expr_box(def.value), + value: def.value.map(restrict_expr_box), ty: def.ty, }), pl::StmtKind::TypeDef(def) => StmtKind::TypeDef(prqlc_ast::TypeDef { @@ -450,14 +450,14 @@ fn restrict_decl(name: String, value: decl::Decl) -> Option { decl::DeclKind::TableDecl(table_decl) => StmtKind::VarDef(VarDef { kind: VarDefKind::Let, name: name.clone(), - value: Box::new(match table_decl.expr { + value: Some(Box::new(match table_decl.expr { decl::TableExpr::RelationVar(expr) => restrict_expr(*expr), decl::TableExpr::LocalTable => Expr::new(ExprKind::Internal("local_table".into())), decl::TableExpr::None => { Expr::new(ExprKind::Internal("literal_tracker".to_string())) } decl::TableExpr::Param(id) => Expr::new(ExprKind::Param(id)), - }), + })), ty: table_decl.ty, }), @@ -471,7 +471,7 @@ fn restrict_decl(name: String, value: decl::Decl) -> Option { kind: VarDefKind::Let, name, ty: expr.ty.take(), - value: restrict_expr_box(expr), + value: Some(restrict_expr_box(expr)), }), decl::DeclKind::Ty(ty) => StmtKind::TypeDef(TypeDef { name, @@ -486,7 +486,7 @@ fn new_internal_stmt(name: String, internal: String) -> StmtKind { StmtKind::VarDef(VarDef { kind: VarDefKind::Let, name, - value: Box::new(Expr::new(ExprKind::Internal(internal))), + value: Some(Box::new(Expr::new(ExprKind::Internal(internal)))), ty: None, }) } diff --git a/prqlc/prqlc/src/semantic/eval.rs b/prqlc/prqlc/src/semantic/eval.rs index 93aa6c672081..c7d643c9fe81 100644 --- a/prqlc/prqlc/src/semantic/eval.rs +++ b/prqlc/prqlc/src/semantic/eval.rs @@ -460,7 +460,7 @@ mod test { fn eval(source: &str) -> Result { let stmts = crate::prql_to_pl(source)?.into_iter().exactly_one()?; - let expr = stmts.kind.into_var_def().unwrap().value; + let expr = stmts.kind.into_var_def().unwrap().value.unwrap(); let value = super::eval(*expr)?; diff --git a/prqlc/prqlc/src/semantic/resolver/expr.rs b/prqlc/prqlc/src/semantic/resolver/expr.rs index 3f7391efded5..44409cfb2ddc 100644 --- a/prqlc/prqlc/src/semantic/resolver/expr.rs +++ b/prqlc/prqlc/src/semantic/resolver/expr.rs @@ -44,10 +44,10 @@ impl PlFold for Resolver<'_> { } fn fold_var_def(&mut self, var_def: VarDef) -> Result { - let value = if matches!(var_def.value.kind, ExprKind::Func(_)) { - var_def.value - } else { - Box::new(flatten::Flattener::fold(self.fold_expr(*var_def.value)?)) + let value = match var_def.value { + Some(value) if matches!(value.kind, ExprKind::Func(_)) => Some(value), + Some(value) => Some(Box::new(flatten::Flattener::fold(self.fold_expr(*value)?))), + None => None, }; Ok(VarDef { diff --git a/prqlc/prqlc/src/semantic/resolver/generic.rs b/prqlc/prqlc/src/semantic/resolver/generic.rs new file mode 100644 index 000000000000..0f0c8aab9f9b --- /dev/null +++ b/prqlc/prqlc/src/semantic/resolver/generic.rs @@ -0,0 +1,2 @@ +use prqlc_ast::Ty; + diff --git a/prqlc/prqlc/src/semantic/resolver/stmt.rs b/prqlc/prqlc/src/semantic/resolver/stmt.rs index a01fc8ebe29c..17e42666f5d3 100644 --- a/prqlc/prqlc/src/semantic/resolver/stmt.rs +++ b/prqlc/prqlc/src/semantic/resolver/stmt.rs @@ -80,20 +80,35 @@ impl super::Resolver<'_> { ])))); } - if let ExprKind::Func(closure) = &mut def.value.kind { + if let Some(ExprKind::Func(closure)) = def.value.as_mut().map(|x| &mut x.kind) { if closure.name_hint.is_none() { closure.name_hint = Some(ident.clone()); } } - let expected_ty = fold_type_opt(self, def.ty)?; - if expected_ty.is_some() { - let who = || Some(stmt_name.clone()); - self.validate_expr_type(&mut def.value, expected_ty.as_ref(), &who)?; - } + let decl = match def.value { + Some(mut def_value) => { + // var value is provided + + // validate type + let expected_ty = fold_type_opt(self, def.ty)?; + if expected_ty.is_some() { + let who = || Some(stmt_name.clone()); + self.validate_expr_type(&mut def_value, expected_ty.as_ref(), &who)?; + } - let decl = prepare_expr_decl(def.value); + // prepare declaration (includes a special case for tables) + prepare_expr_decl(def_value) + } + None => { + // var value is not provided + // treat this var as a param + let expr = Box::new(Expr::new(ExprKind::Param(ident.to_string()))); + + DeclKind::Expr(expr) + } + }; self.root_mod .declare(ident, decl, stmt.id, stmt.annotations) .with_span(stmt.span)?; From 104e537f619e6826d301b22a8a817eeee6a94d56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Wed, 24 Jan 2024 13:50:39 +0100 Subject: [PATCH 2/8] make relational vars without value references to local tables --- prqlc/prqlc/src/semantic/lowering.rs | 4 ++-- prqlc/prqlc/src/semantic/resolver/stmt.rs | 21 +++++++++++++----- prqlc/prqlc/tests/integration/sql.rs | 27 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/prqlc/prqlc/src/semantic/lowering.rs b/prqlc/prqlc/src/semantic/lowering.rs index 99cc5fa657e4..702138b51f81 100644 --- a/prqlc/prqlc/src/semantic/lowering.rs +++ b/prqlc/prqlc/src/semantic/lowering.rs @@ -79,8 +79,8 @@ fn extern_ref_to_relation( let (_, remainder) = fq_ident.clone().pop_front(); remainder.unwrap() } else { - // tables that are not from default_db - todo!() + // tables that are not from default_db: use full name + fq_ident.clone() }; // put wildcards last diff --git a/prqlc/prqlc/src/semantic/resolver/stmt.rs b/prqlc/prqlc/src/semantic/resolver/stmt.rs index 17e42666f5d3..eca6f0971c67 100644 --- a/prqlc/prqlc/src/semantic/resolver/stmt.rs +++ b/prqlc/prqlc/src/semantic/resolver/stmt.rs @@ -86,27 +86,36 @@ impl super::Resolver<'_> { } } + let expected_ty = fold_type_opt(self, def.ty)?; + let decl = match def.value { Some(mut def_value) => { // var value is provided // validate type - let expected_ty = fold_type_opt(self, def.ty)?; if expected_ty.is_some() { let who = || Some(stmt_name.clone()); self.validate_expr_type(&mut def_value, expected_ty.as_ref(), &who)?; } - // prepare declaration (includes a special case for tables) prepare_expr_decl(def_value) } None => { // var value is not provided - // treat this var as a param - let expr = Box::new(Expr::new(ExprKind::Param(ident.to_string()))); - - DeclKind::Expr(expr) + // is this a relation? + if expected_ty.as_ref().map_or(false, |t| t.is_relation()) { + // treat this var as a TableDecl + DeclKind::TableDecl(TableDecl { + ty: expected_ty, + expr: TableExpr::LocalTable, + }) + } else { + // treat this var as a param + let mut expr = Box::new(Expr::new(ExprKind::Param(stmt_name.clone()))); + expr.ty = expected_ty; + DeclKind::Expr(expr) + } } }; self.root_mod diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index b3c80a04f27e..8ba00797331b 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -4930,3 +4930,30 @@ fn test_group_exclude() { // x // "###); } + +#[test] +fn test_table_declarations() { + assert_display_snapshot!(compile( + r###" + module my_schema { + let my_table <[{ id = int, a = text }]> + } + + let another_table <[{ id = int, b = text }]> + + my_schema.my_table | join another_table (==id) | take 10 + "###, + ) + .unwrap(), @r###" + SELECT + my_table.id, + my_table.a, + another_table.id, + another_table.b + FROM + my_schema.my_table + JOIN another_table ON my_table.id = another_table.id + LIMIT + 10 + "###); +} From 2f54cebf4292ac1f265ed78844f57f46a41dc1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Wed, 24 Jan 2024 13:57:00 +0100 Subject: [PATCH 3/8] uncommit a file that should not be commited --- prqlc/prqlc/src/semantic/resolver/generic.rs | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 prqlc/prqlc/src/semantic/resolver/generic.rs diff --git a/prqlc/prqlc/src/semantic/resolver/generic.rs b/prqlc/prqlc/src/semantic/resolver/generic.rs deleted file mode 100644 index 0f0c8aab9f9b..000000000000 --- a/prqlc/prqlc/src/semantic/resolver/generic.rs +++ /dev/null @@ -1,2 +0,0 @@ -use prqlc_ast::Ty; - From 106d8c907831f1c7ad29519e1d7334072eec9f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Wed, 24 Jan 2024 16:28:38 +0100 Subject: [PATCH 4/8] add compiler parameter for database module --- prqlc/prqlc-ast/src/error.rs | 22 ++++++ prqlc/prqlc-ast/src/expr/ident.rs | 14 +++- prqlc/prqlc/src/cli/mod.rs | 9 +-- prqlc/prqlc/src/lib.rs | 10 ++- prqlc/prqlc/src/semantic/lowering.rs | 70 +++++++++++++------ prqlc/prqlc/src/semantic/mod.rs | 7 +- .../tests/integration/bad_error_messages.rs | 10 ++- prqlc/prqlc/tests/integration/sql.rs | 14 ++-- 8 files changed, 116 insertions(+), 40 deletions(-) diff --git a/prqlc/prqlc-ast/src/error.rs b/prqlc/prqlc-ast/src/error.rs index bd216dcdf7e1..8d6c52256aa5 100644 --- a/prqlc/prqlc-ast/src/error.rs +++ b/prqlc/prqlc-ast/src/error.rs @@ -112,15 +112,29 @@ impl std::fmt::Display for Errors { } pub trait WithErrorInfo: Sized { + fn span(&self) -> Option<&Span>; + fn push_hint>(self, hint: S) -> Self; fn with_hints, I: IntoIterator>(self, hints: I) -> Self; fn with_span(self, span: Option) -> Self; fn with_code(self, code: &'static str) -> Self; + + fn with_span_if_not_exists(self, span: Option) -> Self { + if self.span().is_none() { + self.with_span(span) + } else { + self + } + } } impl WithErrorInfo for Error { + fn span(&self) -> Option<&Span> { + self.span.as_ref() + } + fn with_hints, I: IntoIterator>(mut self, hints: I) -> Self { self.hints = hints.into_iter().map(|x| x.into()).collect(); self @@ -144,6 +158,10 @@ impl WithErrorInfo for Error { #[cfg(feature = "anyhow")] impl WithErrorInfo for anyhow::Error { + fn span(&self) -> Option<&Span> { + self.downcast_ref::().and_then(|e| e.span.as_ref()) + } + fn push_hint>(self, hint: S) -> Self { self.downcast_ref::() .map(|e| e.clone().push_hint(hint).into()) @@ -173,6 +191,10 @@ impl WithErrorInfo for anyhow::Error { } impl WithErrorInfo for Result { + fn span(&self) -> Option<&Span> { + self.as_ref().err().and_then(|x| x.span()) + } + fn with_hints, I: IntoIterator>(self, hints: I) -> Self { self.map_err(|e| e.with_hints(hints)) } diff --git a/prqlc/prqlc-ast/src/expr/ident.rs b/prqlc/prqlc-ast/src/expr/ident.rs index 3eedb99ed32f..a8d8c2d2b54c 100644 --- a/prqlc/prqlc-ast/src/expr/ident.rs +++ b/prqlc/prqlc-ast/src/expr/ident.rs @@ -67,10 +67,18 @@ impl Ident { .all(|(prefix_component, self_component)| prefix_component == self_component) } + pub fn starts_with_path>(&self, prefix: &[S]) -> bool { + if prefix.len() > self.path.len() + 1 { + return false; + } + prefix + .iter() + .zip(self.iter()) + .all(|(prefix_component, self_component)| prefix_component.as_ref() == self_component) + } + pub fn starts_with_part(&self, prefix: &str) -> bool { - self.iter() - .next() - .map_or(false, |self_component| self_component == prefix) + self.starts_with_path(&[prefix]) } } diff --git a/prqlc/prqlc/src/cli/mod.rs b/prqlc/prqlc/src/cli/mod.rs index bfbe92d8a440..61ead22a5049 100644 --- a/prqlc/prqlc/src/cli/mod.rs +++ b/prqlc/prqlc/src/cli/mod.rs @@ -12,6 +12,7 @@ use clap::{CommandFactory, Parser, Subcommand, ValueHint}; use clio::has_extension; use clio::Output; use itertools::Itertools; +use prqlc::semantic::NS_DEFAULT_DB; use prqlc_ast::stmt::StmtKind; use std::collections::HashMap; use std::env; @@ -376,7 +377,7 @@ impl Command { semantic::load_std_lib(sources); let ast = prql_to_pl_tree(sources)?; - let ir = pl_to_rq_tree(ast, &main_path)?; + let ir = pl_to_rq_tree(ast, &main_path, &[NS_DEFAULT_DB.to_string()])?; match format { Format::Json => serde_json::to_string_pretty(&ir)?.into_bytes(), @@ -397,7 +398,7 @@ impl Command { .with_format(*format); prql_to_pl_tree(sources) - .and_then(|pl| pl_to_rq_tree(pl, &main_path)) + .and_then(|pl| pl_to_rq_tree(pl, &main_path, &[NS_DEFAULT_DB.to_string()])) .and_then(|rq| rq_to_sql(rq, &opts)) .map_err(|e| e.composed(sources))? .as_bytes() @@ -408,7 +409,7 @@ impl Command { semantic::load_std_lib(sources); let ast = prql_to_pl_tree(sources)?; - let rq = pl_to_rq_tree(ast, &main_path)?; + let rq = pl_to_rq_tree(ast, &main_path, &[NS_DEFAULT_DB.to_string()])?; let srq = prqlc::sql::internal::preprocess(rq)?; format!("{srq:#?}").as_bytes().to_vec() } @@ -416,7 +417,7 @@ impl Command { semantic::load_std_lib(sources); let ast = prql_to_pl_tree(sources)?; - let rq = pl_to_rq_tree(ast, &main_path)?; + let rq = pl_to_rq_tree(ast, &main_path, &[NS_DEFAULT_DB.to_string()])?; let srq = prqlc::sql::internal::anchor(rq)?; let json = serde_json::to_string_pretty(&srq)?; diff --git a/prqlc/prqlc/src/lib.rs b/prqlc/prqlc/src/lib.rs index bdf3b43d200d..0dbf67823f0b 100644 --- a/prqlc/prqlc/src/lib.rs +++ b/prqlc/prqlc/src/lib.rs @@ -143,7 +143,7 @@ pub fn compile(prql: &str, options: &Options) -> Result { semantic::load_std_lib(&mut sources); parser::parse(&sources) - .and_then(|ast| semantic::resolve_and_lower(ast, &[])) + .and_then(|ast| semantic::resolve_and_lower(ast, &[], None)) .and_then(|rq| sql::compile(rq, options)) .map_err(error_message::downcast) .map_err(|e| e.composed(&prql.into())) @@ -269,6 +269,7 @@ impl Options { pub struct ReadmeDoctests; /// Parse PRQL into a PL AST +// TODO: rename this to `prql_to_pl_simple` pub fn prql_to_pl(prql: &str) -> Result, ErrorMessages> { let sources = SourceTree::from(prql); @@ -288,17 +289,20 @@ pub fn prql_to_pl_tree( } /// Perform semantic analysis and convert PL to RQ. +// TODO: rename this to `pl_to_rq_simple` pub fn pl_to_rq(pl: Vec) -> Result { let source_tree = SourceTree::single(PathBuf::new(), pl); - semantic::resolve_and_lower(source_tree, &[]).map_err(error_message::downcast) + semantic::resolve_and_lower(source_tree, &[], None).map_err(error_message::downcast) } /// Perform semantic analysis and convert PL to RQ. pub fn pl_to_rq_tree( pl: SourceTree>, main_path: &[String], + database_module_path: &[String], ) -> Result { - semantic::resolve_and_lower(pl, main_path).map_err(error_message::downcast) + semantic::resolve_and_lower(pl, main_path, Some(database_module_path)) + .map_err(error_message::downcast) } /// Generate SQL from RQ. diff --git a/prqlc/prqlc/src/semantic/lowering.rs b/prqlc/prqlc/src/semantic/lowering.rs index 702138b51f81..b8c70de94a8f 100644 --- a/prqlc/prqlc/src/semantic/lowering.rs +++ b/prqlc/prqlc/src/semantic/lowering.rs @@ -19,15 +19,22 @@ use crate::COMPILER_VERSION; use crate::{Error, Reason, Span, WithErrorInfo}; use prqlc_ast::expr::generic::{InterpolateItem, Range, SwitchCase}; -use super::NS_DEFAULT_DB; - -/// Convert AST into IR and make sure that: +/// Convert a resolved expression at path `main_path` relative to `root_mod` +/// into RQ and make sure that: /// - transforms are not nested, /// - transforms have correct partition, window and sort set, /// - make sure there are no unresolved expressions. +/// +/// All table references must reside within module at `database_module_path`. +/// They are compiled to table identifiers, using their path relative to the database module. +/// For example, with `database_module_path=my_database`: +/// - `my_database.my_table` will compile to `"my_table"`, +/// - `my_database.my_schema.my_table` will compile to `"my_schema.my_table"`, +/// - `my_table` will error out saying that this table does not reside in current database. pub fn lower_to_ir( root_mod: RootModule, main_path: &[String], + database_module_path: &[String], ) -> Result<(RelationalQuery, RootModule)> { // find main log::debug!("lookup for main pipeline in {main_path:?}"); @@ -50,12 +57,17 @@ pub fn lower_to_ir( let tables = toposort_tables(tables, &main_ident); // lower tables - let mut l = Lowerer::new(root_mod); + let mut l = Lowerer::new(root_mod, database_module_path); let mut main_relation = None; - for (fq_ident, table) in tables { + for (fq_ident, (table, declared_at)) in tables { let is_main = fq_ident == main_ident; - l.lower_table_decl(table, fq_ident)?; + let span = declared_at + .and_then(|id| l.root_mod.span_map.get(&id)) + .cloned(); + + l.lower_table_decl(table, fq_ident) + .with_span_if_not_exists(span)?; if is_main { let main_table = l.table_buffer.pop().unwrap(); @@ -74,13 +86,24 @@ pub fn lower_to_ir( fn extern_ref_to_relation( mut columns: Vec, fq_ident: &Ident, -) -> (rq::Relation, Option) { - let extern_name = if fq_ident.starts_with_part(NS_DEFAULT_DB) { - let (_, remainder) = fq_ident.clone().pop_front(); - remainder.unwrap() + database_module_path: &[String], +) -> Result<(rq::Relation, Option), Error> { + let extern_name = if fq_ident.starts_with_path(database_module_path) { + let relative_to_database: Vec<&String> = + fq_ident.iter().skip(database_module_path.len()).collect(); + if relative_to_database.is_empty() { + None + } else { + Some(Ident::from_path(relative_to_database)) + } } else { - // tables that are not from default_db: use full name - fq_ident.clone() + None + }; + + let Some(extern_name) = extern_name else { + let database_module = Ident::from_path(database_module_path.to_vec()); + return Err(Error::new_simple("this table is not in the current database") + .push_hint(format!("If this is a table in the current database, move its declaration into module {database_module}"))); }; // put wildcards last @@ -90,7 +113,7 @@ fn extern_ref_to_relation( kind: rq::RelationKind::ExternRef(extern_name), columns: tuple_fields_to_relation_columns(columns), }; - (relation, None) + Ok((relation, None)) } fn tuple_fields_to_relation_columns(columns: Vec) -> Vec { @@ -118,6 +141,7 @@ struct Lowerer { tid: IdGenerator, root_mod: RootModule, + database_module_path: Vec, /// describes what has certain id has been lowered to node_mapping: HashMap, @@ -146,9 +170,10 @@ enum LoweredTarget { } impl Lowerer { - fn new(root_mod: RootModule) -> Self { + fn new(root_mod: RootModule, database_module_path: &[String]) -> Self { Lowerer { root_mod, + database_module_path: database_module_path.to_vec(), cid: IdGenerator::new(), tid: IdGenerator::new(), @@ -173,7 +198,9 @@ impl Lowerer { // a CTE (self.lower_relation(*expr)?, Some(fq_ident.name.clone())) } - TableExpr::LocalTable => extern_ref_to_relation(columns, &fq_ident), + TableExpr::LocalTable => { + extern_ref_to_relation(columns, &fq_ident, &self.database_module_path)? + } TableExpr::Param(_) => unreachable!(), TableExpr::None => return Ok(()), }; @@ -1008,12 +1035,12 @@ fn validate_take_range(range: &Range, span: Option) -> Result<() struct TableExtractor { path: Vec, - tables: Vec<(Ident, decl::TableDecl)>, + tables: Vec<(Ident, (decl::TableDecl, Option))>, } impl TableExtractor { /// Finds table declarations in a module, recursively. - fn extract(root_module: &Module) -> Vec<(Ident, decl::TableDecl)> { + fn extract(root_module: &Module) -> Vec<(Ident, (decl::TableDecl, Option))> { let mut te = TableExtractor::default(); te.extract_from_module(root_module); te.tables @@ -1030,7 +1057,8 @@ impl TableExtractor { } DeclKind::TableDecl(table) => { let fq_ident = Ident::from_path(self.path.clone()); - self.tables.push((fq_ident, table.clone())); + self.tables + .push((fq_ident, (table.clone(), entry.declared_at))); } _ => {} } @@ -1043,14 +1071,14 @@ impl TableExtractor { /// are not needed for the main pipeline. To do this, it needs to collect references /// between pipelines. fn toposort_tables( - tables: Vec<(Ident, decl::TableDecl)>, + tables: Vec<(Ident, (decl::TableDecl, Option))>, main_table: &Ident, -) -> Vec<(Ident, decl::TableDecl)> { +) -> Vec<(Ident, (decl::TableDecl, Option))> { let tables: HashMap<_, _, RandomState> = HashMap::from_iter(tables); let mut dependencies: Vec<(Ident, Vec)> = Vec::new(); for (ident, table) in &tables { - let deps = if let TableExpr::RelationVar(e) = &table.expr { + let deps = if let TableExpr::RelationVar(e) = &table.0.expr { TableDepsCollector::collect(*e.clone()) } else { vec![] diff --git a/prqlc/prqlc/src/semantic/mod.rs b/prqlc/prqlc/src/semantic/mod.rs index 16e8f7678392..4b4092787992 100644 --- a/prqlc/prqlc/src/semantic/mod.rs +++ b/prqlc/prqlc/src/semantic/mod.rs @@ -27,10 +27,13 @@ use crate::{Error, Reason, SourceTree}; pub fn resolve_and_lower( file_tree: SourceTree>, main_path: &[String], + database_module_path: Option<&[String]>, ) -> Result { let root_mod = resolve(file_tree, Default::default())?; - let (query, _) = lowering::lower_to_ir(root_mod, main_path)?; + let default_db = [NS_DEFAULT_DB.to_string()]; + let database_module_path = database_module_path.unwrap_or(&default_db); + let (query, _) = lowering::lower_to_ir(root_mod, main_path, database_module_path)?; Ok(query) } @@ -284,7 +287,7 @@ pub mod test { pub fn parse_resolve_and_lower(query: &str) -> Result { let source_tree = query.into(); - resolve_and_lower(parse(&source_tree)?, &[]) + resolve_and_lower(parse(&source_tree)?, &[], None) } pub fn parse_and_resolve(query: &str) -> Result { diff --git a/prqlc/prqlc/tests/integration/bad_error_messages.rs b/prqlc/prqlc/tests/integration/bad_error_messages.rs index c37e1c94fed1..f98156b7473d 100644 --- a/prqlc/prqlc/tests/integration/bad_error_messages.rs +++ b/prqlc/prqlc/tests/integration/bad_error_messages.rs @@ -192,6 +192,14 @@ fn nested_groups() { ) ) "###).unwrap_err(), @r###" - Error: internal compiler error; tracked at https://github.com/PRQL/prql/issues/3870 + Error: + ╭─[:2:5] + │ + 2 │ ╭─▶ from inv=invoices + ┆ ┆ + 12 │ ├─▶ ) + │ │ + │ ╰─────────── internal compiler error; tracked at https://github.com/PRQL/prql/issues/3870 + ────╯ "###); } diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index 8ba00797331b..2b69831f016a 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -2273,7 +2273,7 @@ fn test_from_json() { prqlc::semantic::load_std_lib(&mut source_tree); let sql_from_prql = Ok(prqlc::prql_to_pl_tree(&source_tree).unwrap()) - .and_then(|ast| prqlc::semantic::resolve_and_lower(ast, &[])) + .and_then(|ast| prqlc::semantic::resolve_and_lower(ast, &[], None)) .and_then(|rq| sql::compile(rq, &Options::default())) .unwrap(); @@ -4935,13 +4935,15 @@ fn test_group_exclude() { fn test_table_declarations() { assert_display_snapshot!(compile( r###" - module my_schema { - let my_table <[{ id = int, a = text }]> - } + module default_db { + module my_schema { + let my_table <[{ id = int, a = text }]> + } - let another_table <[{ id = int, b = text }]> + let another_table <[{ id = int, b = text }]> + } - my_schema.my_table | join another_table (==id) | take 10 + from my_schema.my_table | join another_table (==id) | take 10 "###, ) .unwrap(), @r###" From 9b3d854994b2747f5eb11c6270db93049ca4e468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Thu, 25 Jan 2024 10:20:38 +0100 Subject: [PATCH 5/8] refactor with_span_if_not_exists --- prqlc/prqlc-ast/src/error.rs | 22 ---------------------- prqlc/prqlc/src/semantic/lowering.rs | 28 +++++++++++++++++++++++----- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/prqlc/prqlc-ast/src/error.rs b/prqlc/prqlc-ast/src/error.rs index 8d6c52256aa5..bd216dcdf7e1 100644 --- a/prqlc/prqlc-ast/src/error.rs +++ b/prqlc/prqlc-ast/src/error.rs @@ -112,29 +112,15 @@ impl std::fmt::Display for Errors { } pub trait WithErrorInfo: Sized { - fn span(&self) -> Option<&Span>; - fn push_hint>(self, hint: S) -> Self; fn with_hints, I: IntoIterator>(self, hints: I) -> Self; fn with_span(self, span: Option) -> Self; fn with_code(self, code: &'static str) -> Self; - - fn with_span_if_not_exists(self, span: Option) -> Self { - if self.span().is_none() { - self.with_span(span) - } else { - self - } - } } impl WithErrorInfo for Error { - fn span(&self) -> Option<&Span> { - self.span.as_ref() - } - fn with_hints, I: IntoIterator>(mut self, hints: I) -> Self { self.hints = hints.into_iter().map(|x| x.into()).collect(); self @@ -158,10 +144,6 @@ impl WithErrorInfo for Error { #[cfg(feature = "anyhow")] impl WithErrorInfo for anyhow::Error { - fn span(&self) -> Option<&Span> { - self.downcast_ref::().and_then(|e| e.span.as_ref()) - } - fn push_hint>(self, hint: S) -> Self { self.downcast_ref::() .map(|e| e.clone().push_hint(hint).into()) @@ -191,10 +173,6 @@ impl WithErrorInfo for anyhow::Error { } impl WithErrorInfo for Result { - fn span(&self) -> Option<&Span> { - self.as_ref().err().and_then(|x| x.span()) - } - fn with_hints, I: IntoIterator>(self, hints: I) -> Self { self.map_err(|e| e.with_hints(hints)) } diff --git a/prqlc/prqlc/src/semantic/lowering.rs b/prqlc/prqlc/src/semantic/lowering.rs index b8c70de94a8f..ff56ac9da85b 100644 --- a/prqlc/prqlc/src/semantic/lowering.rs +++ b/prqlc/prqlc/src/semantic/lowering.rs @@ -62,12 +62,8 @@ pub fn lower_to_ir( for (fq_ident, (table, declared_at)) in tables { let is_main = fq_ident == main_ident; - let span = declared_at - .and_then(|id| l.root_mod.span_map.get(&id)) - .cloned(); - l.lower_table_decl(table, fq_ident) - .with_span_if_not_exists(span)?; + .map_err(with_span_if_not_exists(|| get_span_of_id(&l, declared_at)))?; if is_main { let main_table = l.table_buffer.pop().unwrap(); @@ -1133,3 +1129,25 @@ impl PlFold for TableDepsCollector { Ok(expr) } } + +fn get_span_of_id(l: &Lowerer, id: Option) -> Option { + id.and_then(|id| l.root_mod.span_map.get(&id)).cloned() +} + +fn with_span_if_not_exists<'a, F>(get_span: F) -> impl FnOnce(anyhow::Error) -> anyhow::Error + 'a +where + F: FnOnce() -> Option + 'a, +{ + move |e| { + let e = match e.downcast::() { + Ok(e) => e, + Err(e) => return e, + }; + + if e.span.is_some() { + return e.into(); + } + + e.with_span(get_span()).into() + } +} From 9fcfbbd01140bbf88ff469951a45914ce1c1c65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Thu, 25 Jan 2024 10:23:23 +0100 Subject: [PATCH 6/8] refactor ident len, for readability --- prqlc/prqlc-ast/src/expr/ident.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/prqlc/prqlc-ast/src/expr/ident.rs b/prqlc/prqlc-ast/src/expr/ident.rs index a8d8c2d2b54c..0d6e1a856156 100644 --- a/prqlc/prqlc-ast/src/expr/ident.rs +++ b/prqlc/prqlc-ast/src/expr/ident.rs @@ -27,6 +27,14 @@ impl Ident { } } + pub fn len(&self) -> usize { + self.path.len() + 1 + } + + pub fn is_empty(&self) -> bool { + false + } + /// Remove last part of the ident. /// Result will generally refer to the parent of this ident. pub fn pop(self) -> Option { @@ -58,7 +66,7 @@ impl Ident { } pub fn starts_with(&self, prefix: &Ident) -> bool { - if prefix.path.len() > self.path.len() { + if prefix.len() > self.len() { return false; } prefix @@ -68,7 +76,8 @@ impl Ident { } pub fn starts_with_path>(&self, prefix: &[S]) -> bool { - if prefix.len() > self.path.len() + 1 { + // self is an I + if prefix.len() > self.len() { return false; } prefix @@ -116,7 +125,7 @@ impl Serialize for Ident { where S: Serializer, { - let mut seq = serializer.serialize_seq(Some(self.path.len() + 1))?; + let mut seq = serializer.serialize_seq(Some(self.len()))?; for part in &self.path { seq.serialize_element(part)?; } From 4be1ae5f9de922acc58ebbd386d6426d4e7d89a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Tue, 2 Jan 2024 13:29:08 +0100 Subject: [PATCH 7/8] a few more tests --- flake.nix | 1 + prqlc/Taskfile.yaml | 19 ++--- prqlc/prqlc/src/codegen/ast.rs | 80 ++++++++++++++++++++-- prqlc/prqlc/tests/integration/resolving.rs | 13 ++++ prqlc/prqlc/tests/integration/sql.rs | 19 +++++ 5 files changed, 120 insertions(+), 12 deletions(-) diff --git a/flake.nix b/flake.nix index 3bd01a6c5a10..95f804827024 100644 --- a/flake.nix +++ b/flake.nix @@ -26,6 +26,7 @@ cargo-release pkg-config openssl + cargo-llvm-cov # actions go-task diff --git a/prqlc/Taskfile.yaml b/prqlc/Taskfile.yaml index 4deb3c1445d4..af21b0e92b40 100644 --- a/prqlc/Taskfile.yaml +++ b/prqlc/Taskfile.yaml @@ -46,20 +46,23 @@ tasks: - cmd: cargo clippy --all-targets {{.packages_core}} test: - desc: A full test of prqlc + desc: | + A full test of prqlc (excluding --test-dbs-external). + Generates coverage report. + env: + # Use a different target dir so we don't poison the cache + CARGO_LLVM_COV_TARGET_DIR: ../target-cov cmds: - - cmd: - cargo nextest run {{.packages_core}} {{.packages_addon}} - {{.packages_bindings}} + - cmd: | + cargo \ + llvm-cov --lcov --output-path lcov.info \ + nextest --features=test-dbs \ + {{.packages_core}} {{.packages_addon}} {{.packages_bindings}} - cmd: cargo clippy --all-targets {{.packages_core}} {{.packages_addon}} {{.packages_bindings}} -- -D warnings - - cmd: - cargo test --package prqlc --features=default,test-dbs - --test=integration -- queries::results - pull-request: desc: Most checks that run within GH actions for a pull request cmds: diff --git a/prqlc/prqlc/src/codegen/ast.rs b/prqlc/prqlc/src/codegen/ast.rs index 33c3df63cc43..98efe9daa693 100644 --- a/prqlc/prqlc/src/codegen/ast.rs +++ b/prqlc/prqlc/src/codegen/ast.rs @@ -379,6 +379,7 @@ impl WriteSource for Stmt { } _ => { r += &val.write(opt)?; + r += "\n"; } } @@ -524,6 +525,19 @@ mod test { assert_is_formatted(r#"join `project-bar.dataset.table` (==col_bax)"#); } + #[test] + fn test_binary() { + assert_is_formatted(r#"let a = 5 * (4 + 3) ?? (5 / 2) // 2 == 1 and true"#); + + // TODO: associativity is not handled correctly + // assert_is_formatted(r#"let a = 5 / 2 / 2"#); + } + + #[test] + fn test_func() { + assert_is_formatted(r#"let a = func x y:false -> x and y"#); + } + #[test] fn test_simple() { assert_is_formatted( @@ -553,15 +567,73 @@ group {title, country} (aggregate { fn test_range() { assert_is_formatted( r#" -from foo -is_negative = -100..0 +let negative = -100..0 +"#, + ); + + assert_is_formatted( + r#" +let negative = -(100..0) +"#, + ); + + assert_is_formatted( + r#" +let negative = -100.. +"#, + ); + + assert_is_formatted( + r#" +let negative = ..-100 +"#, + ); + } + + #[test] + fn test_annotation() { + assert_is_formatted( + r#" +@deprecated +module hello { +} +"#, + ); + } + + #[test] + fn test_var_def() { + assert_is_formatted( + r#" +let a "#, ); assert_is_formatted( r#" -from foo -is_negative = -(100..0) +let a +"#, + ); + + assert_is_formatted( + r#" +let a = 5 +"#, + ); + + assert_is_formatted( + r#" +5 +into a +"#, + ); + } + + #[test] + fn test_query_def() { + assert_is_formatted( + r#" +prql version:"^0.9" target:sql.sqlite "#, ); } diff --git a/prqlc/prqlc/tests/integration/resolving.rs b/prqlc/prqlc/tests/integration/resolving.rs index 0f2cddaf51ee..d43770bcacd8 100644 --- a/prqlc/prqlc/tests/integration/resolving.rs +++ b/prqlc/prqlc/tests/integration/resolving.rs @@ -96,6 +96,19 @@ fn resolve_types_04() { "###); } +#[test] +fn resolve_types_05() { + // TODO: this is very strange, it should only be allowed in std + assert_snapshot!(resolve( + r#" + type A + "#, + ) + .unwrap(), @r###" + type A = null + "###); +} + #[test] fn resolve_generics_01() { assert_snapshot!(resolve( diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index 2b69831f016a..ba15c4e842fe 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -4959,3 +4959,22 @@ fn test_table_declarations() { 10 "###); } + +#[test] +fn test_param_declarations() { + assert_display_snapshot!(compile( + r###" + let a + + from x | filter b == a + "###, + ) + .unwrap(), @r###" + SELECT + * + FROM + x + WHERE + b = $a + "###); +} From c3a7e96822947f7fa23247d34dcec5b6333ba5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Thu, 25 Jan 2024 11:28:04 +0100 Subject: [PATCH 8/8] rebase --- prqlc/prqlc/tests/integration/static_eval.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prqlc/prqlc/tests/integration/static_eval.rs b/prqlc/prqlc/tests/integration/static_eval.rs index db768b9f3f8b..211ef5feecf6 100644 --- a/prqlc/prqlc/tests/integration/static_eval.rs +++ b/prqlc/prqlc/tests/integration/static_eval.rs @@ -10,7 +10,7 @@ fn static_eval(prql_source: &str) -> ConstExpr { let stmts = stmts_tree.sources.values().next().unwrap(); let stmt = stmts.iter().next().unwrap(); let var_def: &VarDef = stmt.kind.as_var_def().unwrap(); - let expr: Expr = var_def.value.as_ref().clone(); + let expr: Expr = *var_def.value.as_ref().unwrap().clone(); let expr = prqlc::semantic::ast_expand::expand_expr(expr).unwrap();