morrySnow commented on code in PR #21171:
URL: https://github.com/apache/doris/pull/21171#discussion_r1261934280
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PredicatePropagation.java:
##########
@@ -70,19 +72,44 @@ public Expression visit(Expression expr, Void context) {
@Override
public Expression visitComparisonPredicate(ComparisonPredicate cp,
Void context) {
- if (cp.left().isSlot() && cp.right().isConstant()) {
- return replaceSlot(cp);
- } else if (cp.left().isConstant() && cp.right().isSlot()) {
- return replaceSlot(cp);
+ if (isExpressionSlotCoveredByCast(cp.left()) &&
(cp.right().isConstant())) {
+ return replaceSlot(cp,
getDatatypeCoveredByCast(cp.left()));
+ } else if (isExpressionSlotCoveredByCast(cp.right()) &&
cp.left().isConstant()) {
+ return replaceSlot(cp,
getDatatypeCoveredByCast(cp.right()));
}
return super.visit(cp, context);
}
- private Expression replaceSlot(Expression expr) {
+ private boolean isTwoExpressionEqualWithCast(Expression left,
Expression right) {
+ return
getSlotCoveredByCast(left).equals(getSlotCoveredByCast(right));
+ }
+
+ private boolean canCompareType(DataType dataType) {
+ return dataType.isBigIntType() || dataType.isIntegerType() ||
dataType.isSmallIntType()
+ || dataType.isTinyIntType() ||
dataType.isLargeIntType();
+ }
+
+ private boolean isOriginDataTypeBigger(DataType originDataType,
Expression expr) {
+ if
(canCompareType(leftSlotEqualToRightSlot.child(0).getDataType())
+ &&
canCompareType(leftSlotEqualToRightSlot.child(1).getDataType())
+ && canCompareType(originDataType)) {
+ // infer filter can not be lower than original datatype,
or dataset would be wrong
+ if (originDataType.width() >
leftSlotEqualToRightSlot.child(0).getDataType().width()
+ || originDataType.width() >
leftSlotEqualToRightSlot.child(1).getDataType().width()) {
Review Comment:
use width not a good idea. maybe u should add widerThan or similar method to
IntegralType
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PredicatePropagation.java:
##########
@@ -92,13 +119,48 @@ private Expression replaceSlot(Expression expr) {
}, null);
}
+ /**
+ * judge if expression is slot covered by cast
+ * example: cast(cast(table.columnA))
+ */
+ private boolean isExpressionSlotCoveredByCast(Expression expr) {
+ if (expr instanceof Cast) {
+ return isExpressionSlotCoveredByCast(((Cast) expr).child());
+ }
+ return expr instanceof SlotReference;
+ }
+
+ /**
+ * get slot covered by cast
+ * example: input: cast(cast(table.columnA)) output: columnA.datatype
+ *
+ */
+ private DataType getDatatypeCoveredByCast(Expression expr) {
+ if (expr instanceof Cast) {
+ return getDatatypeCoveredByCast(((Cast) expr).child());
+ }
+ return expr.getDataType();
+ }
+
+ /**
+ * get slot covered by cast
+ * example: input: cast(cast(table.columnA)) output: table.columnA
+ */
+ private Expression getSlotCoveredByCast(Expression cast) {
+ if (cast instanceof Cast) {
+ return getSlotCoveredByCast(((Cast) cast).child());
+ }
+ return cast;
+ }
Review Comment:
move it into ExpressionUtils
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PredicatePropagation.java:
##########
@@ -92,13 +119,48 @@ private Expression replaceSlot(Expression expr) {
}, null);
}
+ /**
+ * judge if expression is slot covered by cast
+ * example: cast(cast(table.columnA))
+ */
+ private boolean isExpressionSlotCoveredByCast(Expression expr) {
+ if (expr instanceof Cast) {
+ return isExpressionSlotCoveredByCast(((Cast) expr).child());
+ }
+ return expr instanceof SlotReference;
+ }
Review Comment:
move it into ExpressionUtils
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PredicatePropagation.java:
##########
@@ -70,19 +72,44 @@ public Expression visit(Expression expr, Void context) {
@Override
public Expression visitComparisonPredicate(ComparisonPredicate cp,
Void context) {
- if (cp.left().isSlot() && cp.right().isConstant()) {
- return replaceSlot(cp);
- } else if (cp.left().isConstant() && cp.right().isSlot()) {
- return replaceSlot(cp);
+ if (isExpressionSlotCoveredByCast(cp.left()) &&
(cp.right().isConstant())) {
Review Comment:
add comments to explain why need `isExpressionSlotCoveredByCast` and
`getDatatypeCoveredByCast `
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PredicatePropagation.java:
##########
@@ -70,19 +72,44 @@ public Expression visit(Expression expr, Void context) {
@Override
public Expression visitComparisonPredicate(ComparisonPredicate cp,
Void context) {
- if (cp.left().isSlot() && cp.right().isConstant()) {
- return replaceSlot(cp);
- } else if (cp.left().isConstant() && cp.right().isSlot()) {
- return replaceSlot(cp);
+ if (isExpressionSlotCoveredByCast(cp.left()) &&
(cp.right().isConstant())) {
+ return replaceSlot(cp,
getDatatypeCoveredByCast(cp.left()));
+ } else if (isExpressionSlotCoveredByCast(cp.right()) &&
cp.left().isConstant()) {
+ return replaceSlot(cp,
getDatatypeCoveredByCast(cp.right()));
}
return super.visit(cp, context);
}
- private Expression replaceSlot(Expression expr) {
+ private boolean isTwoExpressionEqualWithCast(Expression left,
Expression right) {
+ return
getSlotCoveredByCast(left).equals(getSlotCoveredByCast(right));
+ }
Review Comment:
move it into ExpressionUtils
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PredicatePropagation.java:
##########
@@ -70,19 +72,44 @@ public Expression visit(Expression expr, Void context) {
@Override
public Expression visitComparisonPredicate(ComparisonPredicate cp,
Void context) {
- if (cp.left().isSlot() && cp.right().isConstant()) {
- return replaceSlot(cp);
- } else if (cp.left().isConstant() && cp.right().isSlot()) {
- return replaceSlot(cp);
+ if (isExpressionSlotCoveredByCast(cp.left()) &&
(cp.right().isConstant())) {
+ return replaceSlot(cp,
getDatatypeCoveredByCast(cp.left()));
+ } else if (isExpressionSlotCoveredByCast(cp.right()) &&
cp.left().isConstant()) {
+ return replaceSlot(cp,
getDatatypeCoveredByCast(cp.right()));
}
return super.visit(cp, context);
}
- private Expression replaceSlot(Expression expr) {
+ private boolean isTwoExpressionEqualWithCast(Expression left,
Expression right) {
+ return
getSlotCoveredByCast(left).equals(getSlotCoveredByCast(right));
+ }
+
+ private boolean canCompareType(DataType dataType) {
Review Comment:
if u want get all integer type , u could use `instance of IntegralType`
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
