Skip to content

Commit 56141d9

Browse files
authored
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 f93a5b3 commit 56141d9

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
@@ -156,10 +156,7 @@ pub fn translate(
156156
let predicate_expression = filtering::translate(
157157
env,
158158
state,
159-
&helpers::RootAndCurrentTables {
160-
root_table: table_name_and_reference.clone(),
161-
current_table: table_name_and_reference,
162-
},
159+
&helpers::TableScope::new(table_name_and_reference),
163160
&predicate,
164161
)?;
165162

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,10 +245,7 @@ pub fn translate(
245245
let predicate_expression = filtering::translate(
246246
env,
247247
state,
248-
&helpers::RootAndCurrentTables {
249-
root_table: table_name_and_reference.clone(),
250-
current_table: table_name_and_reference,
251-
},
248+
&helpers::TableScope::new(table_name_and_reference),
252249
&predicate,
253250
)?;
254251

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
// Set default constrainst
160157
let default_constraint = default_constraint();

0 commit comments

Comments
 (0)