-
Notifications
You must be signed in to change notification settings - Fork 51
feat: Allow local arithmetic execution in hybrid engine #1906
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
Conversation
dd71fac
to
a082510
Compare
a082510
to
74c3051
Compare
): | ||
rarg = ops.AsTypeOp(to_type=dtypes.DATETIME_DTYPE).as_expr(rarg) | ||
|
||
return expr.op.as_expr(larg, rarg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the function throw an error if an argument has an invalid type? Or you just let Polar throws errors?
@@ -140,7 +140,8 @@ class AddOp(base_ops.BinaryOp): | |||
def output_type(self, *input_types): | |||
left_type = input_types[0] | |||
right_type = input_types[1] | |||
if all(map(dtypes.is_string_like, input_types)) and len(set(input_types)) == 1: | |||
# TODO: Binary/bytes addition requires impl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please create a bug for this TODO? It can be part of pandas top-100 hostlist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created issue 432495312
@@ -37,26 +37,259 @@ def lower(self, expr: expression.OpExpression) -> expression.Expression: | |||
return expr.op.as_expr(larg, rarg) | |||
|
|||
|
|||
class LowerAddRule(op_lowering.OpLoweringRule): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand why it's calling "lower" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw the term used in some other compiler-related things: https://stackoverflow.com/questions/20252876/wanted-good-definition-of-the-term-lowering-in-the-context-of-compilers/21295297#21295297
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕