Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[multistage][bugfix] canonicalize SqlKind.OTHERS as well as SqlKind.OTHER_FUNCTIONS #12025

Merged
merged 1 commit into from Nov 17, 2023
Merged

[multistage][bugfix] canonicalize SqlKind.OTHERS as well as SqlKind.OTHER_FUNCTIONS #12025

merged 1 commit into from Nov 17, 2023

Conversation

walterddr
Copy link
Contributor

@walterddr walterddr commented Nov 17, 2023 •
edited

Fix the following issues:

When compiling the RexExpression

  • SqlKind.OTHERS should extract actual function name inside obj instead of getting the toString functionName "others"
  • SqlKind.OTHERS/OTHER_FUNCTIONS should canonicalized function name during parsing instead of rely on downstream operator to parse and interpret them.
  • Support concat as || operator

@walterddr walterddr changed the title [multistage] canonicalize SqlKind.OTHERS as well as SqlKind.OTHER_FUNCTIONS [multistage][bugfix] canonicalize SqlKind.OTHERS as well as SqlKind.OTHER_FUNCTIONS Nov 17, 2023
Jackie-Jiang
Jackie-Jiang reviewed Nov 17, 2023
View reviewed changes
pinot-query-planner/src/main/java/org/apache/pinot/query/parser/CalciteRexExpressionParser.java
@@ -223,12 +226,13 @@ private static Expression compileFunctionExpression(RexExpression.FunctionCall r
return compileAndExpression(rexCall, pinotQuery);
case OR:
return compileOrExpression(rexCall, pinotQuery);
case OTHER:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does OTHER always have function name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes.
in our RexExpression context. RexExpression type FunctionCall stems from RexCall which encapsulates an SqlOperator#SqlKind and SqlOperator#name --> thus it should always have a name

@Jackie-Jiang Jackie-Jiang added the multi-stage Related to the multi-stage query engine label Nov 17, 2023
@walterddr walterddr merged commit 1d8f2b6 into apache:master Nov 17, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jackie-Jiang Jackie-Jiang Jackie-Jiang approved these changes

Assignees
No one assigned
Labels
bugfix enhancement multi-stage Related to the multi-stage query engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@walterddr @Jackie-Jiang