Add support for user-defined macro functions with arbitrary expressions#4318
Add support for user-defined macro functions with arbitrary expressions#4318hazefully wants to merge 13 commits into
Conversation
f3c3bee to
e60a7d9
Compare
e60a7d9 to
ad58abd
Compare
| return function.validateCall(arguments.toNamedArgumentInvocation()); | ||
| return function.validateCall(namedArguments); | ||
| } | ||
| // todo: this should go throw validateCall for unnamed arguments, however that function is not considering |
There was a problem hiding this comment.
please restore the todo (with the corrected rewording) this has to be consolidated and resolved at some point.
There was a problem hiding this comment.
I removed the todo because before this change validateCall was not being called for functions with unnamed arguments, but now it is. However, validateCall still doesn't check if the type of the arguments matches the expected function parameters types, as this is still delegated to the encapsulation logic, and the comment at L89 reflects that.
Is this something we want to change at a later time (i.e. move the type checking up from encapsulation time to function resolution time) or is this a design choice at this time?
There was a problem hiding this comment.
we're leaning more towards keeping the encapsulation logic intact and maybe move part of the type checking to the function resolution logic. this heavily depends on whether we want to support function overloading in the future or not.
This rule is only used in the grammar to avoid retyping its parts in the different rules it uses.
📊 Metrics Diff Analysis ReportSummary
ℹ️ About this analysisThis automated analysis compares query planner metrics between the base branch and this PR. It categorizes changes into:
The last category in particular may indicate planner regressions that should be investigated. New QueriesCount of new queries by file:
Plan and Metrics ChangedThese queries experienced both plan and metrics changes. This generally indicates that there was some planner change Total: 23 queries Statistical Summary (Plan and Metrics Changed)
There were no queries with significant regressions detected. Minor Changes (Plan and Metrics Changed)In addition, there were 23 queries with minor changes. |
This PR extends support for user-defined macro functions (user-defined functions which return a value instead of a relational expression) to support arbitrary expressions as the function's body instead of only field values. In addition, the following is also now supported for user-defined macro functions:
RETURNSRETURN, in addition to the already supported keywordAS. Both keywords are identical in how the body is handled.See the new tests added to the YAML test file
user-defined-macro-function-tests.yamsqlfor examples on what macro functions are now supported and how they can be used.In addition, this PR adds support for the YAML tests option
supported_versionto setup blocks to make it possible to only run setup blocks starting from some version in f880733. It also adds additional tests for macro functions to the filecase-sensitivity.yamsqlin 3b05bf8.