Skip to content

Commit

Permalink
Allow ORDER BY to see projection names
Browse files Browse the repository at this point in the history
  • Loading branch information
jpschorr committed Feb 6, 2024
1 parent 3374e62 commit 06e0ccd
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 5 deletions.
72 changes: 67 additions & 5 deletions partiql-logical-planner/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ use partiql_catalog::call_defs::{CallArgument, CallDef};

use partiql_ast_passes::error::{AstTransformError, AstTransformationError};

use partiql_ast_passes::name_resolver::{NameLookup, NameRef};

Check warning on line 35 in partiql-logical-planner/src/lower.rs

View workflow job for this annotation

GitHub Actions / clippy

unused import: `NameLookup`

warning: unused import: `NameLookup` --> partiql-logical-planner/src/lower.rs:35:41 | 35 | use partiql_ast_passes::name_resolver::{NameLookup, NameRef}; | ^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default
use partiql_catalog::Catalog;
use partiql_extension_ion::decode::{IonDecoderBuilder, IonDecoderConfig};
use partiql_extension_ion::Encoding;
use partiql_logical::AggFunc::{AggAny, AggAvg, AggCount, AggEvery, AggMax, AggMin, AggSum};
use partiql_logical::ValueExpr::DynamicLookup;
use std::sync::atomic::{AtomicU32, Ordering};

type FnvIndexMap<K, V> = IndexMap<K, V, FnvBuildHasher>;
Expand Down Expand Up @@ -161,6 +163,8 @@ pub struct AstToLogical<'a> {

from_lets: HashSet<ast::NodeId>,

projection_renames: Vec<FnvIndexMap<String, BindingsName<'a>>>,

aliases: FnvIndexMap<NodeId, SymbolPrimitive>,

// generator of 'fresh' ids
Expand Down Expand Up @@ -230,6 +234,8 @@ impl<'a> AstToLogical<'a> {

from_lets: Default::default(),

projection_renames: Default::default(),

aliases: Default::default(),

// generator of 'fresh' ids
Expand Down Expand Up @@ -283,6 +289,30 @@ impl<'a> AstToLogical<'a> {
}

fn resolve_varref(&self, varref: &ast::VarRef) -> logical::ValueExpr {
fn binding_to_symprim<'a>(binding: &'a BindingsName<'a>) -> SymbolPrimitive {

Check warning on line 292 in partiql-logical-planner/src/lower.rs

View workflow job for this annotation

GitHub Actions / clippy

function `binding_to_symprim` is never used

warning: function `binding_to_symprim` is never used --> partiql-logical-planner/src/lower.rs:292:12 | 292 | fn binding_to_symprim<'a>(binding: &'a BindingsName<'a>) -> SymbolPrimitive { | ^^^^^^^^^^^^^^^^^^ | = note: `#[warn(dead_code)]` on by default
match binding {
BindingsName::CaseSensitive(n) => SymbolPrimitive {
value: n.as_ref().to_owned(),
case: CaseSensitivity::CaseSensitive,
},
BindingsName::CaseInsensitive(n) => SymbolPrimitive {
value: n.as_ref().to_owned(),
case: CaseSensitivity::CaseInsensitive,
},
}
}

fn binding_to_static<'a>(binding: &'a BindingsName<'a>) -> BindingsName<'static> {
match binding {
BindingsName::CaseSensitive(n) => {
BindingsName::CaseSensitive(Cow::Owned(n.as_ref().to_string()))
}
BindingsName::CaseInsensitive(n) => {
BindingsName::CaseInsensitive(Cow::Owned(n.as_ref().to_string()))
}
}
}

// Convert a `SymbolPrimitive` into a `BindingsName`
fn symprim_to_binding(sym: &SymbolPrimitive) -> BindingsName<'static> {
match sym.case {
Expand All @@ -306,14 +336,35 @@ impl<'a> AstToLogical<'a> {
if let Some(key_schema) = self.key_registry.schema.get(id) {
let key_schema: &name_resolver::KeySchema = key_schema;

let name_ref: &name_resolver::NameRef = key_schema
let name_ref: &NameRef = &key_schema
.consume
.iter()
.find(|name_ref| name_ref.sym == varref.name)
.expect("NameRef");

Check warning on line 343 in partiql-logical-planner/src/lower.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler

warning: this expression creates a reference which is immediately dereferenced by the compiler --> partiql-logical-planner/src/lower.rs:339:42 | 339 | let name_ref: &NameRef = &key_schema | __________________________________________^ 340 | | .consume 341 | | .iter() 342 | | .find(|name_ref| name_ref.sym == varref.name) 343 | | .expect("NameRef"); | |______________________________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow = note: `#[warn(clippy::needless_borrow)]` on by default help: change this to | 339 ~ let name_ref: &NameRef = key_schema 340 + .consume 341 + .iter() 342 + .find(|name_ref| name_ref.sym == varref.name) 343 ~ .expect("NameRef"); |

let var_binding = symprim_to_binding(&name_ref.sym);
let mut lookups = vec![];

if matches!(self.current_ctx(), Some(QueryContext::Order)) {
let renames = self.projection_renames.last().unwrap();
let binding = renames
.iter()
.find(|(k, _)| {
let SymbolPrimitive { value, case } = &name_ref.sym;
match case {
CaseSensitivity::CaseSensitive => value == *k,
CaseSensitivity::CaseInsensitive => unicase::eq(value, *k),
}
})
.map(|(k, v)| binding_to_static(v))

Check warning on line 359 in partiql-logical-planner/src/lower.rs

View workflow job for this annotation

GitHub Actions / clippy

unused variable: `k`

warning: unused variable: `k` --> partiql-logical-planner/src/lower.rs:359:32 | 359 | .map(|(k, v)| binding_to_static(v)) | ^ help: if this is intentional, prefix it with an underscore: `_k` | = note: `#[warn(unused_variables)]` on by default
.unwrap_or_else(|| symprim_to_binding(&name_ref.sym));

lookups.push(DynamicLookup(Box::new(vec![ValueExpr::VarRef(
binding,
VarRefType::Local,
)])));
}

for lookup in &name_ref.lookup {
match lookup {
name_resolver::NameLookup::Global => {
Expand Down Expand Up @@ -442,11 +493,13 @@ impl<'a> AstToLogical<'a> {
fn enter_q(&mut self) {
self.q_stack.push(Default::default());
self.ctx_stack.push(QueryContext::Query);
self.projection_renames.push(Default::default());
}

#[inline]
fn exit_q(&mut self) -> QueryClauses {
self.ctx_stack.pop();
self.projection_renames.pop().expect("q level");
self.ctx_stack.pop().expect("q level");
self.q_stack.pop().expect("q level")
}

Expand Down Expand Up @@ -856,6 +909,15 @@ impl<'a, 'ast> Visitor<'ast> for AstToLogical<'a> {
"".to_string()
}
};

if !alias.is_empty() {
if let ValueExpr::VarRef(name, vrtype) = &value {

Check warning on line 914 in partiql-logical-planner/src/lower.rs

View workflow job for this annotation

GitHub Actions / clippy

unused variable: `vrtype`

warning: unused variable: `vrtype` --> partiql-logical-planner/src/lower.rs:914:56 | 914 | if let ValueExpr::VarRef(name, vrtype) = &value { | ^^^^^^ help: if this is intentional, prefix it with an underscore: `_vrtype`
self.projection_renames
.last_mut()
.expect("renames")
.insert(alias.clone(), name.clone());
}
}
exprs.push((alias, value));
}

Expand Down Expand Up @@ -1286,16 +1348,16 @@ impl<'a, 'ast> Visitor<'ast> for AstToLogical<'a> {
Traverse::Continue
}

fn enter_var_ref(&mut self, _var_ref: &'ast VarRef) -> Traverse {
fn enter_var_ref(&mut self, var_ref: &'ast VarRef) -> Traverse {
let is_path = matches!(self.current_ctx(), Some(QueryContext::Path));
if !is_path {
let options = self.resolve_varref(_var_ref);
let options = self.resolve_varref(var_ref);
self.push_vexpr(options);
} else {
let VarRef {
name: SymbolPrimitive { value, case },
qualifier: _,
} = _var_ref;
} = var_ref;
let name = match case {
CaseSensitivity::CaseSensitive => {
BindingsName::CaseSensitive(Cow::Owned(value.clone()))
Expand Down
1 change: 1 addition & 0 deletions partiql/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ bench = false
[dev-dependencies]
partiql-parser = { path = "../partiql-parser" }
partiql-ast = { path = "../partiql-ast" }
partiql-ast-passes = { path = "../partiql-ast-passes" }
partiql-catalog = { path = "../partiql-catalog"}
partiql-value = { path = "../partiql-value" }
partiql-logical = { path = "../partiql-logical" }
Expand Down

0 comments on commit 06e0ccd

Please sign in to comment.