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.

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

Extend expression pushdown support #9506

Open
assaf2 opened this issue Oct 5, 2021 · 7 comments
Open

Extend expression pushdown support #9506

assaf2 opened this issue Oct 5, 2021 · 7 comments
Assignees

Comments

@assaf2
Copy link
Member

assaf2 commented Oct 5, 2021

At ConnectorExpressionTranslator, support conversion of expressions such Cast, NotExpression, IsNullPredicate, IsNotNullPredicate, ArithmeticBinaryExpression. These expressions can be represented as function calls rather than new types of ConnectorExpression.

EDIT: GenericLiteral, LogicalExpression, ComparisonExpression and Cast were implemented at #11086.

@assaf2 assaf2 self-assigned this Oct 5, 2021
@slhmy
Copy link

slhmy commented Dec 21, 2021

@assaf2 I'm trying to implement visitArithmeticBinary for ArithmeticBinaryExpression.
Then I found when the left and right expression of ArithmeticBinaryExpression are in different type, they are CastExpression wrapped outside them.

CAST(DECIMAL '1.0' AS decimal(2, 1))

So I think CastExpression also needed conversion.

@slhmy
Copy link

slhmy commented Dec 22, 2021

Also, does this issue means we need to implement Functions like AND, OR and so on?
I tried to pushdown these expression down as Call, then I found they can not be handled like $like_pattern.

I found ArithmeticBinaryExpression and ComparisonExpression will not have the above problem, they have implemented ScalarOperators.
But LogicalExpression is different, it has an unsure params count, and don't have ScalarOperator/Function to handle them.

My solution is making a nested call for LogicalExpression which make every logical call have two parameters, then implementing a function for AND and OR.
Obviously it's not a good solution..

@assaf2
Copy link
Member Author

assaf2 commented Dec 22, 2021

@slhmy Thanks for sharing this, I'll check it when I'll start working on this issue, immediately after #7994 is merged.

@findepi
Copy link
Member

findepi commented Mar 14, 2022

NotExpression, IsNullPredicate, IsNotNullPredicate, ArithmeticBinaryExpression. These expressions can be represented as function calls rather than new types of ConnectorExpression.

i think the biggest cost related to adding support for these is to define how they should be modeled, and the rest is just executing on the plan

I propose

  • NotExpression -> Call("$not")
  • IsNullPredicate -> Call("$is_null")
  • IsNotNullPredicate -> Call("$not", Call("$is_null"))
  • ArithmeticBinaryExpression -> Call with $plus, $minus, $multiply, $divide function names

(The reason for using $ in all names is to avoid collisions with existing, or potentially existing, functions with similar names.)

@martint please comment or upvote this.

cc @hashhar @wendigo @losipiuk

@findepi
Copy link
Member

findepi commented Mar 16, 2022

ArithmeticBinaryExpression -> Call with $plus, $minus, $multiply, $divide function names

#11511

  • NotExpression -> Call("$not")
  • IsNullPredicate -> Call("$is_null")
  • IsNotNullPredicate -> Call("$not", Call("$is_null"))

#11514

@assaf2
Copy link
Member Author

assaf2 commented Mar 27, 2022

CAST: #11551
BETWEEN translation: #11684 (already pushed down as $and(val >= min, val <= max))
COALESCE #11535

@assaf2
Copy link
Member Author

assaf2 commented Mar 29, 2022

IF and CASE #11699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants