mihaibudiu commented on code in PR #5345:
URL: https://github.com/apache/calcite/pull/5345#discussion_r4354374903
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2690,52 +2690,37 @@ && sameTypeOrNarrowsNullability(e.getType(),
operand.getType())
}
if (RexUtil.isLosslessCast(operand)) {
// x :: y below means cast(x as y) (which is PostgreSQL-specific cast by
the way)
- // A) Remove lossless casts:
+ // A) Both casts can be removed:
// A.1) intExpr :: bigint :: int => intExpr
// A.2) char2Expr :: char(5) :: char(2) => char2Expr
- // B) There are cases when we can't remove two casts, but we could
probably remove inner one
- // B.1) char2expression :: char(4) :: char(5) -> char2expression ::
char(5)
- // B.2) char2expression :: char(10) :: char(5) -> char2expression ::
char(5)
- // B.3) char2expression :: varchar(10) :: char(5) -> char2expression ::
char(5)
- // B.4) char6expression :: varchar(10) :: char(5) -> char6expression ::
char(5)
- // C) Simplification is not possible:
- // C.1) char6expression :: char(3) :: char(5) -> must not be changed
+ // B) Simplification is not possible:
+ // B.1) char6expression :: char(3) :: char(5) => must not be changed
// the input is truncated to 3 chars, so we can't use
char6expression :: char(5)
- // C.2) varchar2Expr :: char(5) :: varchar(2) -> must not be changed
- // the input have to be padded with spaces (up to 2 chars)
- // C.3) char2expression :: char(4) :: varchar(5) -> must not be changed
+ // B.2) varchar2Expr :: char(5) :: varchar(2) => must not be changed
+ // the input has to be padded with spaces (up to 2 chars)
+ // B.3) char2expression :: char(4) :: varchar(5) => must not be changed
// would not have the padding
- // The approach seems to be:
- // 1) Ensure inner cast is lossless (see if above)
- // 2) If operand of the inner cast has the same type as the outer cast,
- // remove two casts except C.2 or C.3-like pattern (== inner cast is
CHAR)
- // 3) If outer cast is lossless, remove inner cast (B-like cases)
-
- // Here we try to remove two casts in one go (A-like cases)
RexNode intExpr = ((RexCall) operand).operands.get(0);
- // intExpr == CHAR detects A.1
- // operand != CHAR detects C.2
if ((intExpr.getType().getSqlTypeName() == SqlTypeName.CHAR
|| operand.getType().getSqlTypeName() != SqlTypeName.CHAR)
&& sameTypeOrNarrowsNullability(e.getType(), intExpr.getType())) {
+ // Remove both casts (A-like cases): the outer type is the source type,
+ // so the two casts are a round trip, except through a CHAR type from a
+ // non-CHAR source, as in int :: char(11) :: int: not every runtime
+ // parses the padded text back
return intExpr;
}
- // Here we try to remove inner cast (B-like cases)
if (RexUtil.isLosslessCast(intExpr.getType(), operand.getType())
- && (e.getType().getSqlTypeName() ==
operand.getType().getSqlTypeName()
- || e.getType().getSqlTypeName() == SqlTypeName.CHAR
- || operand.getType().getSqlTypeName() != SqlTypeName.CHAR)
- && SqlTypeCoercionRule.instance()
- .canApplyFrom(intExpr.getType().getSqlTypeName(),
e.getType().getSqlTypeName())) {
Review Comment:
The last test in keepInnerCastThatChangesResult covers this with a check
`VARCHAR(5) :: VARCHAR(10) :: GEOMETRY`
The original implementation would prevent optimizing `TINYINT :: INTEGER ::
BOOLEAN` for example.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
