mihaibudiu commented on code in PR #5345:
URL: https://github.com/apache/calcite/pull/5345#discussion_r4362632945
##########
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:
Despite what that paper claims, I think that formal verification is rather
hopeless for real implementations of of real SQL because the SQL spec is rather
loose, and each dialect makes different choices. Perhaps if you fix ONE dialect
then you can do something by proving that some rewrites do not produce plans
with equivalent semantics, but Calcite is supposed to be very flexible and can
emulate features from many dialects based on various configuration flags.
Agents do change the equation, since they never get tired writing test cases
or debugging test failures; they are seemingly also good at writing proofs;
tying proofs to the actual code is another matter, though. I don't think Java
has good enough tooling for this; many people are using Verus these days for
Rust.
In my recent testing I have been asking agents to generate "all possible
combinations" of tests for some reduced test matrices and to compare our
results against postgres. This works very well when the Calcite semantics
aligns with postgres. I have also been comparing our Rust runtime with the
calcite constant folding implementation -- by evaluating the same expression on
constants or on runtime data -- by turning on and off the REDUCE_* passes in
our Calcite optimizer. Any discrepancy requires investigation: could be our bug
or a Calcite bug. In the case of this PR the agent figured out that these
rewrites happen even without the constant folding... so these are really bugs.
I am not sure how useful this answer is. I am still learning how to best do
things.
--
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]
