zstan commented on code in PR #3229:
URL: https://github.com/apache/ignite-3/pull/3229#discussion_r1497364710
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java:
##########
@@ -564,6 +556,14 @@ public static Object consumeFirstArgument(Object args0,
Object args1) {
return args1;
}
+ /** Returns the timestamp value minus the offset of the specified
timezone. */
+ public static Long localTimestampToUtc(Long timestamp, TimeZone timeZone) {
Review Comment:
why "local" is used here ? seems you need to change func name.
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java:
##########
@@ -197,45 +200,127 @@ public void testDateTime() {
assertQuery("SELECT date
'1992-01-19'").returns(sqlDate("1992-01-19")).check();
assertQuery("SELECT date '1992-01-18' + interval (1)
days").returns(sqlDate("1992-01-19")).check();
assertQuery("SELECT date '1992-01-18' + interval (24)
hours").returns(sqlDate("1992-01-19")).check();
+ assertQuery("SELECT timestamp '1992-01-18 02:30:00'")
+ .returns(sqlDateTime("1992-01-18T02:30:00")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-01-18
02:30:00'")
+ .returns(sqlTimestamp("1992-01-18T02:30:00")).check();
assertQuery("SELECT timestamp '1992-01-18 02:30:00' + interval (25)
hours")
.returns(sqlDateTime("1992-01-19T03:30:00")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-01-18
02:30:00' + interval (25) hours")
+ .returns(sqlTimestamp("1992-01-19T03:30:00")).check();
assertQuery("SELECT timestamp '1992-01-18 02:30:00' + interval (23)
hours")
.returns(sqlDateTime("1992-01-19T01:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-01-18
02:30:00' + interval (23) hours")
+ .returns(sqlTimestamp("1992-01-19T01:30:00.000")).check();
assertQuery("SELECT timestamp '1992-01-18 02:30:00' + interval (24)
hours")
.returns(sqlDateTime("1992-01-19T02:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-01-18
02:30:00' + interval (24) hours")
+ .returns(sqlTimestamp("1992-01-19T02:30:00.000")).check();
assertQuery("SELECT date
'1992-03-29'").returns(sqlDate("1992-03-29")).check();
assertQuery("SELECT date '1992-03-28' + interval (1)
days").returns(sqlDate("1992-03-29")).check();
assertQuery("SELECT date '1992-03-28' + interval (24)
hours").returns(sqlDate("1992-03-29")).check();
assertQuery("SELECT timestamp '1992-03-28 02:30:00' + interval (25)
hours")
.returns(sqlDateTime("1992-03-29T03:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-03-28
02:30:00' + interval (25) hours")
+ .returns(sqlTimestamp("1992-03-29T03:30:00.000")).check();
assertQuery("SELECT timestamp '1992-03-28 02:30:00' + interval (23)
hours")
.returns(sqlDateTime("1992-03-29T01:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-03-28
02:30:00' + interval (23) hours")
+ .returns(sqlTimestamp("1992-03-29T01:30:00.000")).check();
assertQuery("SELECT timestamp '1992-03-28 02:30:00' + interval (24)
hours")
.returns(sqlDateTime("1992-03-29T02:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-03-28
02:30:00' + interval (24) hours")
+ .returns(sqlTimestamp("1992-03-29T02:30:00.000")).check();
assertQuery("SELECT date
'1992-09-27'").returns(sqlDate("1992-09-27")).check();
assertQuery("SELECT date '1992-09-26' + interval (1)
days").returns(sqlDate("1992-09-27")).check();
assertQuery("SELECT date '1992-09-26' + interval (24)
hours").returns(sqlDate("1992-09-27")).check();
assertQuery("SELECT timestamp '1992-09-26 02:30:00' + interval (25)
hours")
.returns(sqlDateTime("1992-09-27T03:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-09-26
02:30:00'")
+ .returns(sqlTimestamp("1992-09-26T02:30:00.000")).check();
assertQuery("SELECT timestamp '1992-09-26 02:30:00' + interval (23)
hours")
.returns(sqlDateTime("1992-09-27T01:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-09-26
02:30:00' + interval (23) hours")
+ .returns(sqlTimestamp("1992-09-27T01:30:00.000")).check();
assertQuery("SELECT timestamp '1992-09-26 02:30:00' + interval (24)
hours")
.returns(sqlDateTime("1992-09-27T02:30:00.000")).check();
+ assertQuery("SELECT timestamp with local time zone '1992-09-26
02:30:00' + interval (24) hours")
Review Comment:
i think that numerous equal tests just with different dates are redundant
and hard to read, probably we need to reduce such tests, wdyt ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionContext.java:
##########
@@ -52,8 +54,6 @@
public class ExecutionContext<RowT> implements DataContext {
private static final IgniteLogger LOG =
Loggers.forClass(ExecutionContext.class);
- private static final TimeZone TIME_ZONE = TimeZone.getDefault(); // TODO
DistributedSqlConfiguration#timeZone
Review Comment:
but what about this comment ? seems we still need to discuss it ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/ExpressionFactoryImpl.java:
##########
@@ -316,6 +318,11 @@ public Iterable<RowT> values(List<RexLiteral> values,
RelDataType rowType) {
RexLiteral literal = values.get(i);
Object val = literal.getValueAs(types.get(field));
+ // Literal was parsed as UTC timestamp, now we need to adjust
it to the client's time zone.
+ if (val != null && literal.getTypeName() ==
SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE) {
+ val = IgniteSqlFunctions.localTimestampToUtc((Long) val,
(TimeZone) ctx.get(Variable.TIME_ZONE.camelName));
Review Comment:
no need to Long boxing ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteMethod.java:
##########
@@ -72,6 +73,9 @@ public enum IgniteMethod {
STRING_TO_TIMESTAMP(IgniteSqlFunctions.class, "timestampStringToNumeric",
String.class),
+ /** See {@link IgniteSqlFunctions#localTimestampToUtc(Long, TimeZone)}. **/
+ LOCAL_TIMESTAMP_TO_UTC(IgniteSqlFunctions.class, "localTimestampToUtc",
Long.class, TimeZone.class),
Review Comment:
```suggestion
LOCAL_TIMESTAMP_TO_UTC(IgniteSqlFunctions.class, "localTimestampToUtc",
long.class, TimeZone.class),
```
and further implementation :
public static Long localTimestampToUtc(**Long** timestamp
need to be changed, seems we no need boxing here ?
--
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]
