Skip to content

Commit 4c8bad3

Browse files
BenoitRanquedanieljharvey
authored andcommitted
Replace RootAndCurrentTables with TableScope, which keeps track of any tables in scope for an exists, instead of only root and current. (#674)
<!-- The PR description should answer 2 (maybe 3) important questions: --> ### What <!-- What is this PR trying to accomplish (and why, if it's not obvious)? --> `ComparisonTarget::RootCollectionColumn` was removed, to be replaced by [named scopes](https://github.com/hasura/ndc-spec/blob/36855ff20dcbd7d129427794aee9746b895390af/rfcs/0015-named-scopes.md). This PR implements the replacement functionality. <!-- Consider: do we need to add a changelog entry? --> ### How <!-- How is it trying to accomplish it (what are the implementation steps)? --> This PR replaces RootAndCurrentTables, with TableScope, a struct that keeps track of the current table and any tables in scope for exists expression. See the accompanying review for details on the code itself.
1 parent 361a926 commit 4c8bad3

File tree

9 files changed

+174
-165
lines changed

9 files changed

+174
-165
lines changed

crates/query-engine/translation/src/translation/error.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ pub enum Error {
6363
scalar: models::ScalarTypeName,
6464
function: models::AggregateFunctionName,
6565
},
66+
ScopeOutOfBounds {
67+
current_collection_name: String,
68+
tables_in_scope_names: Vec<String>,
69+
scope: usize,
70+
},
6671
}
6772

6873
/// Capabilities we don't currently support.
@@ -237,6 +242,26 @@ impl std::fmt::Display for Error {
237242
f,
238243
"Missing single column aggregate function {function:?} for scalar type {scalar:?}"
239244
),
245+
Error::ScopeOutOfBounds {
246+
current_collection_name,
247+
tables_in_scope_names,
248+
scope,
249+
} => {
250+
write!(
251+
f,
252+
"Scope {scope} out of bounds. Current collection is {current_collection_name}. Collections in scope: ["
253+
)?;
254+
let mut first = true;
255+
for collection in tables_in_scope_names {
256+
if first {
257+
first = false;
258+
} else {
259+
write!(f, ", ")?;
260+
}
261+
write!(f, "{collection}")?;
262+
}
263+
write!(f, "].")
264+
}
240265
}
241266
}
242267
}

crates/query-engine/translation/src/translation/helpers.rs

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Helpers for processing requests and building SQL.
22
3-
use std::collections::BTreeMap;
3+
use std::collections::{BTreeMap, VecDeque};
44

55
use ndc_models as models;
66

@@ -47,16 +47,68 @@ pub struct NativeQueryInfo {
4747
pub alias: sql::ast::TableAlias,
4848
}
4949

50-
/// For the root table in the query, and for the current table we are processing,
50+
/// For the current table we are processing, and all ancestor table up to the closest Query,
5151
/// We'd like to track what is their reference in the query (the name we can use to address them,
5252
/// an alias we generate), and what is their name in the metadata (so we can get
5353
/// their information such as which columns are available for that table).
5454
#[derive(Debug, Clone, PartialEq, Eq)]
55-
pub struct RootAndCurrentTables {
56-
/// The root (top-most) table in the query.
57-
pub root_table: TableSourceAndReference,
55+
pub struct TableScope {
5856
/// The current table we are processing.
59-
pub current_table: TableSourceAndReference,
57+
current_table: TableSourceAndReference,
58+
/// Tables in scope. Index 0 corresponds to scope 1, which is the table immediately above the current table in the exists chain.
59+
tables_in_scope: VecDeque<TableSourceAndReference>,
60+
}
61+
62+
impl TableScope {
63+
/// Create a scope from a Query. There will be no tables available through scopes
64+
pub fn new(current_table: TableSourceAndReference) -> Self {
65+
Self {
66+
current_table,
67+
tables_in_scope: VecDeque::new(),
68+
}
69+
}
70+
/// Create a scope from an exists expression or path. The ancestor tables up until the closest query will stay in scope
71+
#[must_use]
72+
pub fn new_from_scope(&self, current_table: TableSourceAndReference) -> Self {
73+
let TableScope {
74+
current_table: parent_table,
75+
tables_in_scope,
76+
} = self;
77+
let mut tables_in_scope = tables_in_scope.clone();
78+
tables_in_scope.push_front(parent_table.clone());
79+
Self {
80+
current_table,
81+
tables_in_scope,
82+
}
83+
}
84+
/// Get the table source and reference for the current table
85+
pub fn current_table(&self) -> &TableSourceAndReference {
86+
&self.current_table
87+
}
88+
/// Get the table source and reference for a table in scope.
89+
/// The scope is an index, where 0 is the current table, 1 is the parent, and so on
90+
/// Errors if the scope is out of bounds
91+
pub fn scoped_table(&self, scope: &Option<usize>) -> Result<&TableSourceAndReference, Error> {
92+
if let Some(scope) = scope {
93+
if *scope > 0 && *scope <= self.tables_in_scope.len() {
94+
Ok(&self.tables_in_scope[scope - 1])
95+
} else if *scope == 0 {
96+
Ok(&self.current_table)
97+
} else {
98+
Err(Error::ScopeOutOfBounds {
99+
current_collection_name: self.current_table.source.name_for_alias(),
100+
tables_in_scope_names: self
101+
.tables_in_scope
102+
.iter()
103+
.map(|c| c.source.name_for_alias())
104+
.collect(),
105+
scope: *scope,
106+
})
107+
}
108+
} else {
109+
Ok(&self.current_table)
110+
}
111+
}
60112
}
61113

62114
/// For a table in the query, We'd like to track what is its reference in the query

crates/query-engine/translation/src/translation/mutation/v2/delete.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,7 @@ pub fn translate(
146146
let predicate_expression = filtering::translate(
147147
env,
148148
state,
149-
&helpers::RootAndCurrentTables {
150-
root_table: table_name_and_reference.clone(),
151-
current_table: table_name_and_reference,
152-
},
149+
&helpers::TableScope::new(table_name_and_reference),
153150
&predicate,
154151
)?;
155152

crates/query-engine/translation/src/translation/mutation/v2/insert.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,7 @@ pub fn translate(
232232
let predicate_expression = filtering::translate(
233233
env,
234234
state,
235-
&helpers::RootAndCurrentTables {
236-
root_table: table_name_and_reference.clone(),
237-
current_table: table_name_and_reference,
238-
},
235+
&helpers::TableScope::new(table_name_and_reference),
239236
&predicate,
240237
)?;
241238

crates/query-engine/translation/src/translation/mutation/v2/update.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,7 @@ pub fn translate(
151151
})
152152
.collect::<Result<Vec<sql::ast::Expression>, Error>>()?;
153153

154-
let root_and_current_tables = helpers::RootAndCurrentTables {
155-
root_table: table_name_and_reference.clone(),
156-
current_table: table_name_and_reference,
157-
};
154+
let root_and_current_tables = helpers::TableScope::new(table_name_and_reference);
158155

159156
// Build the `pre_constraint` argument boolean expression.
160157
let pre_predicate =

0 commit comments

Comments
 (0)