You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/11/27 04:29:03 UTC
[GitHub] [calcite] liyafan82 opened a new pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
liyafan82 opened a new pull request #2282:
URL: https://github.com/apache/calcite/pull/2282
Some simple arithmetic operations can be simplified, for example:
```
a + 0 = a
a - 0 = a
a * 1 = a
a / 1 = a
```
In our product, we found such scenarios are fairly frequently encountered. For example, some SQL programmers use a * 1.0 as a convenient way to cast `a` to floating point type.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
NobiGo commented on pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#issuecomment-874421020
LGTM
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r531835205
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -329,6 +336,93 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ // if any operand is null, the result will be null
+ if (RexUtil.isNullLiteral(e.operands.get(0), true)
+ || RexUtil.isNullLiteral(e.operands.get(1), true)) {
+ return rexBuilder.makeNullLiteral(e.type);
+ }
Review comment:
It might not be true. Sometimes users may expect it can throw an exception.
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -329,6 +336,93 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ // if any operand is null, the result will be null
+ if (RexUtil.isNullLiteral(e.operands.get(0), true)
+ || RexUtil.isNullLiteral(e.operands.get(1), true)) {
+ return rexBuilder.makeNullLiteral(e.type);
+ }
+
+ switch (e.getKind()) {
+ case PLUS:
+ return simplifyPlus(e);
+ case MINUS:
+ return simplifyMinus(e);
+ case TIMES:
+ return simplifyMultiply(e);
+ case DIVIDE:
+ return simplifyDivide(e);
+ default:
+ throw new IllegalArgumentException("Unsupported arithmeitc operation " + e.getKind());
+ }
+ }
+
+ private RexNode simplifyPlus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex >= 0) {
+ // return the other operand
+ RexNode other = e.getOperands().get((zeroIndex + 1) % 2);
+ return other.getType().equals(e.getType())
+ ? other : rexBuilder.makeCast(e.getType(), other);
+ }
+ return simplifyGenericNode(e);
+ }
+
+ private RexNode simplifyMinus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex == 1) {
+ RexNode leftOperand = e.getOperands().get(0);
+ return leftOperand.getType().equals(e.getType())
+ ? leftOperand : rexBuilder.makeCast(e.getType(), leftOperand);
Review comment:
Can `a - a` be simplified to 0 if a is not nullable?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#issuecomment-739113313
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r532346934
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -329,6 +336,93 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ // if any operand is null, the result will be null
+ if (RexUtil.isNullLiteral(e.operands.get(0), true)
+ || RexUtil.isNullLiteral(e.operands.get(1), true)) {
+ return rexBuilder.makeNullLiteral(e.type);
+ }
+
+ switch (e.getKind()) {
+ case PLUS:
+ return simplifyPlus(e);
+ case MINUS:
+ return simplifyMinus(e);
+ case TIMES:
+ return simplifyMultiply(e);
+ case DIVIDE:
+ return simplifyDivide(e);
+ default:
+ throw new IllegalArgumentException("Unsupported arithmeitc operation " + e.getKind());
+ }
+ }
+
+ private RexNode simplifyPlus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex >= 0) {
+ // return the other operand
+ RexNode other = e.getOperands().get((zeroIndex + 1) % 2);
+ return other.getType().equals(e.getType())
+ ? other : rexBuilder.makeCast(e.getType(), other);
+ }
+ return simplifyGenericNode(e);
+ }
+
+ private RexNode simplifyMinus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex == 1) {
+ RexNode leftOperand = e.getOperands().get(0);
+ return leftOperand.getType().equals(e.getType())
+ ? leftOperand : rexBuilder.makeCast(e.getType(), leftOperand);
Review comment:
Thanks for the suggestion.
There are some special cases for which this is not true. For example,
```
NaN - NaN = NaN
Inf - Inf = NaN
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r533010891
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -332,6 +339,87 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
Review comment:
Add . to the end of the sentence.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r533011251
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -332,6 +339,87 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
Review comment:
Could you mark it that it only supports numeric type?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r659086697
##########
File path: core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
##########
@@ -3224,4 +3224,25 @@ private SqlSpecialOperatorWithPolicy(String name, SqlKind kind, int prec, boolea
@Test void testSimplifyVarbinary() {
checkSimplifyUnchanged(cast(cast(vInt(), tVarchar(true, 100)), tVarbinary(true)));
}
+
+ @Test void testSimplifySimpleArithmetic() {
+ RexNode a = vIntNotNull(1);
+ RexNode zero = literal(0);
+ RexNode one = literal(1);
+
+ RexNode b = vDecimalNotNull(2);
+ RexNode half = literal(new BigDecimal(0.5), b.getType());
+
+ checkSimplify(add(a, zero), "?0.notNullInt1");
Review comment:
Could you please add a test like `add(zero, sub(vInt(0), vInt(0)))`?
The expression should evaluate to null in case `vInt(0)` is null.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r659474347
##########
File path: core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
##########
@@ -3224,4 +3224,25 @@ private SqlSpecialOperatorWithPolicy(String name, SqlKind kind, int prec, boolea
@Test void testSimplifyVarbinary() {
checkSimplifyUnchanged(cast(cast(vInt(), tVarchar(true, 100)), tVarbinary(true)));
}
+
+ @Test void testSimplifySimpleArithmetic() {
+ RexNode a = vIntNotNull(1);
+ RexNode zero = literal(0);
+ RexNode one = literal(1);
+
+ RexNode b = vDecimalNotNull(2);
+ RexNode half = literal(new BigDecimal(0.5), b.getType());
+
+ checkSimplify(add(a, zero), "?0.notNullInt1");
Review comment:
Good point. Thanks. @vlsi
I have added some test cases concerning null. Please check.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#issuecomment-874411309
Dear reviewers, thanks a lot for your comments.
If there are no more comments, I want to merge it in a few days.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r533098574
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -332,6 +339,87 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
Review comment:
Sure. I've made it explicit in the JavaDoc.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r533010204
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -332,6 +339,87 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ switch (e.getKind()) {
+ case PLUS:
+ return simplifyPlus(e);
+ case MINUS:
+ return simplifyMinus(e);
+ case TIMES:
+ return simplifyMultiply(e);
+ case DIVIDE:
+ return simplifyDivide(e);
+ default:
+ throw new IllegalArgumentException("Unsupported arithmeitc operation " + e.getKind());
+ }
+ }
+
+ private RexNode simplifyPlus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex >= 0) {
+ // return the other operand
+ RexNode other = e.getOperands().get((zeroIndex + 1) % 2);
+ return other.getType().equals(e.getType())
+ ? other : rexBuilder.makeCast(e.getType(), other);
+ }
+ return simplifyGenericNode(e);
+ }
+
+ private RexNode simplifyMinus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex == 1) {
+ RexNode leftOperand = e.getOperands().get(0);
+ return leftOperand.getType().equals(e.getType())
+ ? leftOperand : rexBuilder.makeCast(e.getType(), leftOperand);
+ }
+ return simplifyGenericNode(e);
+ }
+
+ private RexNode simplifyMultiply(RexCall e) {
+ int oneIndex = findLiteralIndex(e.operands, 1L);
+ if (oneIndex >= 0) {
Review comment:
Make it final since it's our convention.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo commented on pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
NobiGo commented on pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#issuecomment-874421020
LGTM
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#issuecomment-868438881
Dear reviewers, thanks a lot for your good comments.
If there are no other comments, I am going to merge this in a few days.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r532346549
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -329,6 +336,93 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ // if any operand is null, the result will be null
+ if (RexUtil.isNullLiteral(e.operands.get(0), true)
+ || RexUtil.isNullLiteral(e.operands.get(1), true)) {
+ return rexBuilder.makeNullLiteral(e.type);
+ }
Review comment:
Thanks for your feedback. It makes sense. This logic is removed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#issuecomment-874411309
Dear reviewers, thanks a lot for your comments.
If there are no more comments, I want to merge it in a few days.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r533098400
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -332,6 +339,87 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ switch (e.getKind()) {
+ case PLUS:
+ return simplifyPlus(e);
+ case MINUS:
+ return simplifyMinus(e);
+ case TIMES:
+ return simplifyMultiply(e);
+ case DIVIDE:
+ return simplifyDivide(e);
+ default:
+ throw new IllegalArgumentException("Unsupported arithmeitc operation " + e.getKind());
+ }
+ }
+
+ private RexNode simplifyPlus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex >= 0) {
+ // return the other operand
+ RexNode other = e.getOperands().get((zeroIndex + 1) % 2);
+ return other.getType().equals(e.getType())
+ ? other : rexBuilder.makeCast(e.getType(), other);
+ }
+ return simplifyGenericNode(e);
+ }
+
+ private RexNode simplifyMinus(RexCall e) {
+ int zeroIndex = findLiteralIndex(e.operands, 0L);
+ if (zeroIndex == 1) {
+ RexNode leftOperand = e.getOperands().get(0);
+ return leftOperand.getType().equals(e.getType())
+ ? leftOperand : rexBuilder.makeCast(e.getType(), leftOperand);
+ }
+ return simplifyGenericNode(e);
+ }
+
+ private RexNode simplifyMultiply(RexCall e) {
+ int oneIndex = findLiteralIndex(e.operands, 1L);
+ if (oneIndex >= 0) {
Review comment:
Revised. Thanks for the good suggestion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 merged pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 merged pull request #2282:
URL: https://github.com/apache/calcite/pull/2282
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r533098452
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -332,6 +339,87 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
Review comment:
Done. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] chunweilei commented on a change in pull request #2282: [CALCITE-4420] Some simple arithmetic operations can be simplified
Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2282:
URL: https://github.com/apache/calcite/pull/2282#discussion_r531835205
##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -329,6 +336,93 @@ private RexNode simplifyGenericNode(RexCall e) {
return rexBuilder.makeCall(e.getType(), e.getOperator(), operands);
}
+ /**
+ * Try to find a literal with the given value in the input list.
+ */
+ private int findLiteralIndex(List<RexNode> operands, long value) {
+ for (int i = 0; i < operands.size(); i++) {
+ if (operands.get(i).isA(SqlKind.LITERAL)) {
+ Comparable comparable = ((RexLiteral) operands.get(i)).getValue();
+ if (comparable instanceof BigDecimal && ((BigDecimal) comparable).longValue() == value) {
+ return i;
+ }
+ }
+ }
+ return -1;
+ }
+
+ private RexNode simplifyArithmetic(RexCall e) {
+ if (e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC
+ || e.getOperands().stream()
+ .anyMatch(o -> e.getType().getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)) {
+ // we only support simplifying numeric types
+ return simplifyGenericNode(e);
+ }
+
+ assert e.getOperands().size() == 2;
+
+ // if any operand is null, the result will be null
+ if (RexUtil.isNullLiteral(e.operands.get(0), true)
+ || RexUtil.isNullLiteral(e.operands.get(1), true)) {
+ return rexBuilder.makeNullLiteral(e.type);
+ }
Review comment:
It might not be true. Sometimes users may want it to throw an exception.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org