Skip to content

Fix some issues in SQL function#16365

Merged
rongrong merged 4 commits intoprestodb:masterfrom
rongrong:sql-function
Jul 12, 2021
Merged

Fix some issues in SQL function#16365
rongrong merged 4 commits intoprestodb:masterfrom
rongrong:sql-function

Conversation

@rongrong
Copy link
Copy Markdown
Contributor

@rongrong rongrong commented Jul 1, 2021

Test plan - unit test

In facebook: T74900887

== RELEASE NOTES ==

General Changes
* Fix issues in SQL functions where query might fail with compiler error when function implementation includes lambda variable that has the same name as column names or function input names.

rongrong added 4 commits July 6, 2021 15:28
ExpressionInterpreter and RowExpressionInterpreter are used to evaluate
constant expressions, thus we should not return the function expression
when the expression does not evaluate to constant.
When we inline SQL functions, SQL functions are equivalent to syntax sugar.
Thus moving the optimization to the begining of plan optimizer rules together
with all other expression desugaring rules.
Rewrite lambda expressions in SQL function implementation to make sure
each lambda argument is unique in the rewritten expression.
Comment on lines -350 to -354
new IterativeOptimizer(
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
new InlineSqlFunctions(metadata, sqlParser).rules()),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to remove this rule?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not removed. Moved to the beginning of optimizer rules with other desugar expression rules because inline sql functions is conceptually a desugar operation and doing it early would allow more optimizations.

@rongrong rongrong merged commit fc1a5ac into prestodb:master Jul 12, 2021
@rongrong rongrong deleted the sql-function branch July 12, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants