Vladsz83 commented on code in PR #11822:
URL: https://github.com/apache/ignite/pull/11822#discussion_r1993565473
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java:
##########
@@ -62,6 +74,97 @@ public IgniteRexBuilder(RelDataTypeFactory typeFactory) {
return super.makeLiteral(bd.setScale(type.getScale(),
RoundingMode.HALF_UP), type, typeName);
}
+ // Adjust temporal literals so that dates before 15.10.1582 would
match internal long representation.
+ // `RexLiteral#getValueAs(Class type)` uses
`DateTimeUtils#ymdToUnixDate(int year, int month, int day)` directly
+ // calculating epoch days. We use `new Date(long)` and `new
Timestampt(long)` in `TypeUtils#fromInternal(...)` and
+ // and `Date#getTime()` in `TypeUtils#toInternal(...)`. Classic
temporal types process the long value differently
+ // for dates before Gregorian calendar. `DateTimeUtils` does not. This
may cause +several weeks for old date literals.
+ if (o instanceof DateString)
Review Comment:
Yes, because literal misses calls like from _TypeUtils_ at all. It goes into
_RexLiteral#getValueAs()_:
`getValueAs:1097, RexLiteral (org.apache.calcite.rex)
getValue:953, RexLiteral (org.apache.calcite.rex)
convertLiteral:1978, SqlToRelConverter (org.apache.calcite.sql2rel)
convertLiteralInValuesList:1959, SqlToRelConverter
(org.apache.calcite.sql2rel)
convertRowValues:1885, SqlToRelConverter (org.apache.calcite.sql2rel)
convertValuesImpl:4849, SqlToRelConverter (org.apache.calcite.sql2rel)
convertValues:4825, SqlToRelConverter (org.apache.calcite.sql2rel)
convertValues:117, IgniteSqlToRelConvertor
(org.apache.ignite.internal.processors.query.calcite.prepare)
convertQueryRecursive:3819, SqlToRelConverter (org.apache.calcite.sql2rel)
convertQueryRecursive:87, IgniteSqlToRelConvertor
(org.apache.ignite.internal.processors.query.calcite.prepare)
convertInsert:3889, SqlToRelConverter (org.apache.calcite.sql2rel)
convertInsert:94, IgniteSqlToRelConvertor
(org.apache.ignite.internal.processors.query.calcite.prepare)
convertQueryRecursive:3805, SqlToRelConverter (org.apache.calcite.sql2rel)
convertQueryRecursive:87, IgniteSqlToRelConvertor
(org.apache.ignite.internal.processors.query.calcite.prepare)
convertQuery:613, SqlToRelConverter (org.apache.calcite.sql2rel)
rel:339, IgnitePlanner
(org.apache.ignite.internal.processors.query.calcite.prepare)
optimize:76, PlannerHelper
(org.apache.ignite.internal.processors.query.calcite.prepare)`
I think we should refactor dates conversion in future. We should use smth.
like DateTimeUtils instead own methods. But this could affect already stored
dates.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
