You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "izveigor (via GitHub)" <gi...@apache.org> on 2023/03/08 19:59:37 UTC
[GitHub] [arrow-datafusion] izveigor opened a new pull request, #5516: feat: add the similar optimization function for bitwise negative
izveigor opened a new pull request, #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516
# Which issue does this PR close?
Follow on to https://github.com/apache/arrow-datafusion/pull/5423
# Rationale for this change
I propose to add the symmetric optimization function for bitwise negative.
Rules for bitwise negative in case of a binary expression are the same as the not clause:
~(A & B) ===> ~A | ~B
~(A | B) ===> ~A & ~B
~(~A) ===> A
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
New optimization method for binary expressions with bitwise negative.
# Are these changes tested?
After PR merge: https://github.com/apache/arrow-datafusion/pull/5511
# Are there any user-facing changes?
Should increase execution speed
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5516: feat: add the similar optimization function for bitwise negative
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516#discussion_r1131460414
##########
datafusion/optimizer/src/simplify_expressions/utils.rs:
##########
@@ -311,6 +311,38 @@ pub fn negate_clause(expr: Expr) -> Expr {
}
}
+/// bitwise negate a Negative clause
+/// input is the clause to be bitwise negated.(args for Negative clause)
+/// For BinaryExpr:
+/// ~(A & B) ===> ~A | ~B
+/// ~(A | B) ===> ~A & ~B
+/// ~(~A) ===> A
+/// For others, use Negative clause
+pub fn negative_clause(expr: Expr) -> Expr {
Review Comment:
I think this is basically applying https://en.wikipedia.org/wiki/De_Morgan%27s_laws
Maybe we can call it `distribute_negation`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #5516: feat: add the similar optimization function for bitwise negative
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516#issuecomment-1462594447
> Should we create a separate PR for this case to keep the symmetry?
Yes please. Or even better I wonder if there is a way to avoid the duplication.
Thank you @izveigor
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] izveigor commented on pull request #5516: feat: add the similar optimization function for bitwise negative
Posted by "izveigor (via GitHub)" <gi...@apache.org>.
izveigor commented on PR #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516#issuecomment-1460814020
Regarding tests: I have not noticed relevant tests for this segment of the code: https://github.com/apache/arrow-datafusion/blob/main/datafusion/optimizer/src/simplify_expressions/utils.rs#L249-#L273. Should we create a separate PR for this case to keep the symmetry?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb merged pull request #5516: feat: add the similar optimization function for bitwise negative
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] izveigor commented on a diff in pull request #5516: feat: add the similar optimization function for bitwise negative
Posted by "izveigor (via GitHub)" <gi...@apache.org>.
izveigor commented on code in PR #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516#discussion_r1129968496
##########
datafusion/optimizer/src/simplify_expressions/utils.rs:
##########
@@ -311,6 +311,38 @@ pub fn negate_clause(expr: Expr) -> Expr {
}
}
+/// bitwise negate a Negative clause
+/// input is the clause to be bitwise negated.(args for Negative clause)
+/// For BinaryExpr:
+/// ~(A & B) ===> ~A | ~B
+/// ~(A | B) ===> ~A & ~B
+/// ~(~A) ===> A
+/// For others, use Negative clause
+pub fn negative_clause(expr: Expr) -> Expr {
Review Comment:
I think, it deserves a better name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #5516: feat: add the similar optimization function for bitwise negative
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5516:
URL: https://github.com/apache/arrow-datafusion/pull/5516#issuecomment-1462597524
Converted to draft as it is waiting on other PRs and tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org