asolimando commented on code in PR #5345:
URL: https://github.com/apache/calcite/pull/5345#discussion_r4357362719
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2785,6 +2770,66 @@ && sameTypeOrNarrowsNullability(e.getType(),
intExpr.getType())) {
}
}
+ /** Returns whether {@code CAST(CAST(x AS inner) AS outer)} can be
simplified to
+ * {@code CAST(x AS outer)}, where x has type {@code source} and the cast
from
+ * {@code source} to {@code inner} is lossless. */
+ private static boolean canRemoveInnerCast(RelDataType source, RelDataType
inner,
+ RelDataType outer) {
+ // x :: y below means cast(x as y)
+ // Removing the inner cast changes the type of the operand of the outer
cast
+ // from inner to source, but not its value. The result of the outer cast
can
+ // still change, because a cast to a character or binary type formats its
+ // operand according to the operand's type, and a cast of a character
operand
+ // parses its text.
+ // Cases A and B are in simplifyCast.
+ // C) The inner cast can be removed:
+ // C.1) char2expression :: char(4) :: char(5) => char2expression :: char(5)
+ // C.2) char2expression :: char(10) :: char(5) => char2expression ::
char(5)
+ // C.3) char2expression :: varchar(10) :: char(5) => char2expression ::
char(5)
+ // C.4) char6expression :: varchar(10) :: char(5) => char6expression ::
char(5)
+ // C.5) smallintExpr :: int :: varchar => smallintExpr :: varchar
+ // 20 => '20' in both cases
+ // C.6) intExpr :: varchar :: int => intExpr :: int
+ // 20 => 20 in both cases
+ // C.7) intExpr :: varchar :: bigint => intExpr :: bigint
+ // 7369 => 7369 in both cases; the two casts to tinyint give an error
in both cases
+ // D) The inner cast cannot be removed (see also B.2 and B.3 in
simplifyCast):
+ // D.1) intExpr :: double :: varchar => must not be changed
+ // 20 => right: '20.0', wrong: '20'
+ // realExpr :: double :: varchar => must not be changed
+ // 266.66666 => right: '266.6666564941406', wrong: '266.66666'
+ // D.2) intExpr :: decimal(12, 2) :: varchar => must not be changed
+ // 20 => right: '20.00', wrong: '20'
+ // D.3) smallintExpr :: bigint :: varbinary => must not be changed
+ // 20 => right: X'0000000000000014', wrong: X'0014'
+ // D.4) intExpr :: varchar :: timestamp => must not be changed
+ // 7369 => right: error, wrong: 1970-01-01 00:00:07
+ if (SqlTypeUtil.isBinary(outer)) {
+ // D.3
+ return false;
+ }
+ if (SqlTypeUtil.inCharFamily(inner)) {
+ if (inner.getSqlTypeName() == SqlTypeName.CHAR
+ && outer.getSqlTypeName() != SqlTypeName.CHAR) {
+ // B.2 and B.3 from simplifyCast
+ return false;
+ }
+ if (!SqlTypeUtil.inCharFamily(source)
+ && !SqlTypeUtil.inCharFamily(outer)
+ && outer.getSqlTypeName() != source.getSqlTypeName()
Review Comment:
That would be equal to ask "no bugs" in code, that wasn't my point, and as I
said the PR is already a solid improvement that I will take.
I was interested to hear if you had opinions on how to test this more
broadly, possibly with the help of agents. I know that in your stack you have
DataFusion (Arrow types) and a Rust DataFlow runtime, I was wondering if there
was a way to come up with a test harness for an agent to test all combinations
with runtime values and spot more exceptions, like trying to find a counter
example (something like https://arxiv.org/pdf/2403.03193 is doing to "validate"
rewrite rules, finding a minimal counter example).
If we can solve this for one given runtime (say even DF and Arrow), we can
write down what was done and users with different runtimes can do the same and
open tickets/PRs to refine what we have.
It's a way broader discussion beyond this PR, feel free to disregard if you
don't have time, I will approve the PR regardless as it's a solid improvement,
just curious to hear your thoughts on this.
--
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]
