SQL-3293: Implement algebrization for HigherOrderFunction expressions - #181
SQL-3293: Implement algebrization for HigherOrderFunction expressions#181mattChiaravalloti wants to merge 14 commits into
Conversation
…gebrizer and tracking variable schema
| expression_context = ExpressionContext::default() | ||
| .with_higher_order_function_arg_ctx(HigherOrderFunctionCtx::Map), |
There was a problem hiding this comment.
This mimics how algebrization within the context of a Map expression works. It's saying "if the ExpressionContext indicates we're in a Map, how would we algebrize this unqualified Identifier"?
| variables = map! { | ||
| "this" => Schema::Atomic(Atomic::Integer), | ||
| }, |
There was a problem hiding this comment.
Keep an eye on the variables presence/value in these tests! If it is present and does not indicate the variable is nullable, is_nullable is false. If it is not present or is present and indicates the variable is nullable, is_nullable is true.
| allow_order_by_missing_columns: bool, | ||
| clause_type: RefCell<ClauseType>, | ||
| expression_context: ExpressionContext, | ||
| variables: BTreeMap<&'static str, schema::Schema>, |
There was a problem hiding this comment.
Similar to SchemaInferenceState, we need to track which variables are in scope. I considered putting this in the ExpressionContext but ultimately decided against it for reasons I can no longer justify. I think it may make more sense to put this ExpressionContext but I'm not sure... Curious to hear your thoughts!
1 participant
This PR implements algebrization for
HigherOrderFunctionexpressions (including the disambiguation of unqualified identifiers intoVariables), as described by the design doc. This builds off of the refactor that was previously merged in #179 .This PR touched 10 files, but 6 of those files are simply incidental changes because of updated test macro apis and imports.
The relevant changes are in
algebrizer/definitions.rs, where the actual algebrization implementations are written, and inalgebrizer/tests/expressions/{higher_order_functions.rs, identifier_and_subpath.rs}, where the unit tests for all new algebrization is written.Similar to schema-checking for higher order functions, this implementation introduces a
HigherOrderFunctionWrappererror variant that wraps algebrizer errors in some additional context to help users understand (1) that an issue happened within a higher order function and (2) what specific component of the higher order function caused it. Fortunately, wrapping errors in the algebrizer is much more straightforward than in the schema checking module.Building off of #179 , I added an additional field to the
ExpressionContextthat tracks if we are in a higher order function argument context and, if so, which one. Recall from the design doc that since we are not allowing users to specify variable names (they must usethisandvalue) we also implicitly accepted that shadowing will happen if higher order functions are nested within each other. Keep that in mind when you observe uses of the new context field! Notice that we never actually "restore" previous context since it will never really matter! A higher order function function argument is algebrized with a new algebrizer with properly set state, so subsequent algebrization afterwards is unaffected by its state since it uses a different algebrizer struct.