korlov42 commented on code in PR #3229:
URL: https://github.com/apache/ignite-3/pull/3229#discussion_r1500540782
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryProperty.java:
##########
@@ -32,6 +33,7 @@ public final class QueryProperty {
public static final Property<Set<SqlQueryType>> ALLOWED_QUERY_TYPES =
new Property<>("allowed_query_types", cast(Set.class));
public static final Property<String> DEFAULT_SCHEMA = new
Property<>("default_schema", String.class);
+ public static final Property<ZoneId> DEFAULT_TIME_ZONE_ID = new
Property<>("default_time_zone_id", ZoneId.class);
Review Comment:
why do you name it `DEFAULT_`? I mean, we have default properties, but the
property itself is not 'default' in that case
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlDdlParserTest.java:
##########
@@ -631,18 +630,14 @@ public void dropIndexIfExists() {
/**
* Ensures that the user cannot use the TIME_WITH_LOCAL_TIME_ZONE and
TIMESTAMP_WITH_LOCAL_TIME_ZONE types for table columns.
*/
- // TODO: Remove after https://issues.apache.org/jira/browse/IGNITE-19274
is implemented.
+ // TODO: Remove after https://issues.apache.org/jira/browse/IGNITE-21555
is implemented.
@Test
public void timestampWithLocalTimeZoneIsNotSupported() {
Review Comment:
does it make sense to rename this test?
(`timeWithLocalTimeZoneIsNotSupported `)
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionContext.java:
##########
@@ -115,14 +118,15 @@ public ExecutionContext(
this.localNode = localNode;
this.originatingNodeName = originatingNodeName;
this.txAttributes = txAttributes;
+ this.timeZoneId = Objects.requireNonNullElseGet(timeZoneId,
ZoneId::systemDefault);
Review Comment:
this looks dangerous. Let's define _default_ value only once in
SqlQueryProcessor
##########
modules/sql-engine/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -615,12 +615,13 @@ void AlterZoneOption(List<SqlNode> list) :
/**
* Parse datetime types: date, time, timestamp.
*
-* TODO Method doesn't recognize '*_WITH_LOCAL_TIME_ZONE' types and should be
removed after IGNITE-19274.
+* TODO Method doesn't recognize 'TIME_WITH_LOCAL_TIME_ZONE' types and should
be removed after IGNITE-21555.
Review Comment:
```suggestion
* TODO Method doesn't recognize 'TIME_WITH_LOCAL_TIME_ZONE' type and should
be removed after IGNITE-21555.
```
--
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]
