Skip to content

Commit

Permalink
Add Catalog to NameResolver
Browse files Browse the repository at this point in the history
During the work for adding a steel thread for PartiQL Typing (Recent PR #389)
and after merging #410 to `feat-type-plan-poc` it is realized that we need to
refactor the code to remove `DynamicLocalup` `VarExpr` with the assumption that
we work based off of Typing and Value Environment from the Catalog. We have a
Typing Environment in the Catalog at the moment and we are going to add the
Variable Environment as well. In preparation for such task, we need to make
the `NameResolver` Catalog aware. In that regard this commit adds the `Catalog`
to `NameResolver`

Expecting subsequent PR(s) for the name resolving using the Catalog.
  • Loading branch information
am357 committed Aug 16, 2023
1 parent 154bd59 commit 99ff05d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
31 changes: 27 additions & 4 deletions partiql-ast-passes/src/name_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use indexmap::{IndexMap, IndexSet};
use partiql_ast::ast;
use partiql_ast::ast::{GroupByExpr, GroupKey};
use partiql_ast::visit::{Traverse, Visit, Visitor};
use partiql_catalog::Catalog;
use std::sync::atomic::{AtomicU32, Ordering};

type FnvIndexSet<T> = IndexSet<T, FnvBuildHasher>;
Expand Down Expand Up @@ -82,8 +83,8 @@ enum EnclosingClause {
/// Resolves which clauses in a query produce & consume variable references by walking the
/// AST and collecting variable references. Also partially infers alias if no `AS` alias
/// was provided in the query.
#[derive(Default, Debug)]
pub struct NameResolver {
#[derive(Debug)]
pub struct NameResolver<'c> {
// environment stack tracking
id_path_to_root: Vec<ast::NodeId>,
id_child_stack: Vec<Vec<ast::NodeId>>,
Expand All @@ -99,9 +100,31 @@ pub struct NameResolver {

// errors that occur during name resolution
errors: Vec<AstTransformError>,
catalog: &'c dyn Catalog,
}

impl NameResolver {
impl<'c> NameResolver<'c> {
pub fn new(catalog: &'c dyn Catalog) -> Self {
NameResolver {
// environment stack tracking
id_path_to_root: Default::default(),
id_child_stack: Default::default(),
keyref_stack: Default::default(),
lateral_stack: Default::default(),
id_gen: Default::default(),

// data flow tracking
enclosing_clause: Default::default(),
in_scope: Default::default(),
schema: Default::default(),
aliases: Default::default(),

// errors that occur during name resolution
errors: Default::default(),
catalog,
}
}

pub fn resolve(
&mut self,
query: &ast::AstNode<ast::TopLevelQuery>,
Expand Down Expand Up @@ -188,7 +211,7 @@ impl NameResolver {
}
}

impl<'ast> Visitor<'ast> for NameResolver {
impl<'ast, 'c> Visitor<'ast> for NameResolver<'c> {
fn enter_ast_node(&mut self, id: ast::NodeId) -> Traverse {
self.id_path_to_root.push(id);
if let Some(children) = self.id_child_stack.last_mut() {
Expand Down
5 changes: 3 additions & 2 deletions partiql-logical-planner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use partiql_ast_passes::name_resolver::NameResolver;
use partiql_logical as logical;
use partiql_parser::Parsed;

use partiql_catalog::Catalog;
use partiql_catalog::{Catalog, PartiqlCatalog};

mod builtins;
mod lower;
Expand All @@ -25,7 +25,8 @@ impl<'c> LogicalPlanner<'c> {
parsed: &Parsed,
) -> Result<logical::LogicalPlan<logical::BindingsOp>, AstTransformationError> {
let q = &parsed.ast;
let mut resolver = NameResolver::default();
let catalog = PartiqlCatalog::default();
let mut resolver = NameResolver::new(&catalog);
let registry = resolver.resolve(q)?;
let planner = AstToLogical::new(self.catalog, registry);
planner.lower_query(q)
Expand Down

0 comments on commit 99ff05d

Please sign in to comment.