You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/11/29 07:57:25 UTC
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10399: IGNITE-17599 SQL Calcite: Support insertion of LocalDateTime, LocalDate and LocalTime
alex-plekhanov commented on code in PR #10399:
URL: https://github.com/apache/ignite/pull/10399#discussion_r1034405381
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java:
##########
@@ -348,16 +363,41 @@ else if (val instanceof Number && storageType != val.getClass()) {
return val;
}
+ /** Converts temporal objects to long.
+ *
+ * @param ctx Data context.
+ * @param val Temporal value.
+ * @return Millis value.
+ */
+ private static long toLong(DataContext ctx, Object val) {
+ if (val instanceof LocalDateTime)
+ return SqlFunctions.toLong(Timestamp.valueOf((LocalDateTime)val), DataContext.Variable.TIME_ZONE.get(ctx));
+
+ if (val instanceof LocalDate)
+ return SqlFunctions.toLong(DateValueUtils.convertToSqlDate((LocalDate)val), DataContext.Variable.TIME_ZONE.get(ctx));
+
+ if (val instanceof LocalTime)
+ return SqlFunctions.toLong(DateValueUtils.convertToSqlTime((LocalTime)val), DataContext.Variable.TIME_ZONE.get(ctx));
+
+ return SqlFunctions.toLong((java.util.Date)val, DataContext.Variable.TIME_ZONE.get(ctx));
+ }
+
/** */
public static Object fromInternal(DataContext ctx, Object val, Type storageType) {
if (val == null)
return null;
else if (storageType == java.sql.Date.class && val instanceof Integer)
return new java.sql.Date(fromLocalTs(ctx, (Integer)val * DateTimeUtils.MILLIS_PER_DAY));
+ else if (storageType == LocalDate.class && val instanceof Integer)
+ return Instant.ofEpochMilli((Integer)val * DateTimeUtils.MILLIS_PER_DAY).atZone(ZoneOffset.UTC).toLocalDate();
Review Comment:
As far as I remember `ofEpochMilli` return different values than `Date` constructor for dates before `GregorianCalendar#gregorianCutoverYearJulian`, can you please check it?
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LocalDateTimeSupportTest.java:
##########
@@ -0,0 +1,272 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.integration;
+
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Date;
+import java.util.List;
+import java.util.function.Function;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.util.typedef.G;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import static java.util.Collections.singletonList;
+import static org.apache.ignite.internal.cache.query.index.sorted.inline.types.DateValueUtils.convertToSqlDate;
+import static org.apache.ignite.internal.cache.query.index.sorted.inline.types.DateValueUtils.convertToSqlTime;
+import static org.apache.ignite.internal.cache.query.index.sorted.inline.types.DateValueUtils.convertToTimestamp;
+
+/** */
+@RunWith(Parameterized.class)
+public class LocalDateTimeSupportTest extends AbstractBasicIntegrationTest {
+ /** */
+ @Parameterized.Parameter()
+ public boolean isValidationEnabled;
+
+ /** */
+ @Parameterized.Parameter(1)
+ public String sqlType;
+
+ /** */
+ @Parameterized.Parameter(2)
+ public Class<?> colType;
+
+ /** */
+ @Parameterized.Parameter(3)
+ public Class<?> objType;
+
+ /** */
+ @Parameterized.Parameter(4)
+ public Function<Object, Object> sqlTypeConverter;
+
+ /** */
+ @Parameterized.Parameters(name = "isValidationEnabled={0}, sqlType={1}, columnCls={2}, testObjCls={3}")
+ public static Collection<Object[]> parameters() {
+ Collection<Object[]> params = new ArrayList<>();
+
+ for (boolean isV : Arrays.asList(true, false)) {
+ params.add(new Object[] {
+ isV, "TIMESTAMP", null, LocalDateTime.class, f(ts -> convertToTimestamp((LocalDateTime)ts))
+ });
+ params.add(new Object[] {
+ isV, "TIMESTAMP", null, Date.class, f(ts -> new Timestamp(((Date)ts).getTime()))
+ });
+ params.add(new Object[] {
+ isV, "TIMESTAMP", null, java.sql.Date.class, f(ts -> new Timestamp(((Date)ts).getTime()))
+ });
+
+ for (Class<?> testObjCls : Arrays.asList(Timestamp.class, LocalDateTime.class, Date.class, java.sql.Date.class)) {
+ params.add(new Object[] {
+ isV, null, Timestamp.class, testObjCls, f(ts -> {
+ if (ts instanceof LocalDateTime)
+ return convertToTimestamp((LocalDateTime)ts);
+ return ts;
+ })
+ });
+
+ params.add(new Object[] {
+ isV, null, Date.class, testObjCls, f(ts -> {
+ if (testObjCls == LocalDateTime.class)
+ return new Date(convertToTimestamp((LocalDateTime)ts).getTime());
+ else if (testObjCls == Timestamp.class)
+ return new Date(((Timestamp)ts).getTime());
+ return ts;
+ })
+ });
+
+ params.add(new Object[] {
+ isV, null, LocalDateTime.class, testObjCls, f(ts -> {
+ if (testObjCls == Timestamp.class)
+ return ((Timestamp)ts).toLocalDateTime();
+ else if (testObjCls == java.util.Date.class)
+ return new Timestamp(((Date)ts).getTime()).toLocalDateTime();
+ else if (testObjCls == java.sql.Date.class)
+ return ((java.sql.Date)ts).toLocalDate().atStartOfDay();
+ else
+ return ts;
+ })
+ });
+ }
+
+ params.add(new Object[] {
+ isV, "DATE", null, LocalDate.class, f(d -> convertToSqlDate((LocalDate)d))
+ });
+
+ for (Class<?> testObjCls : Arrays.asList(LocalDate.class, java.sql.Date.class)) {
+ params.add(new Object[] {
+ isV, null, java.sql.Date.class, testObjCls, f(ts -> {
+ if (testObjCls == LocalDate.class)
+ return convertToSqlDate((LocalDate)ts);
+ return ts;
+ })
+ });
+
+ params.add(new Object[] {
+ isV, null, LocalDate.class, testObjCls, f(ts -> {
+ if (testObjCls == java.sql.Date.class)
+ return ((java.sql.Date)ts).toLocalDate();
+ return ts;
+ })
+ });
+ }
+
+ params.add(new Object[] {
+ isV, "TIME", null, LocalTime.class, f(t -> convertToSqlTime((LocalTime)t))
+ });
+
+ for (Class<?> testObjCls : Arrays.asList(LocalTime.class, java.sql.Time.class)) {
+ params.add(new Object[] {
+ isV, null, java.sql.Time.class, testObjCls, f(ts -> {
+ if (testObjCls == LocalTime.class)
+ return convertToSqlTime((LocalTime)ts);
+ return ts;
+ })
+ });
+
+ params.add(new Object[] {
+ isV, null, LocalTime.class, testObjCls, f(ts -> {
+ if (testObjCls == java.sql.Time.class)
+ return ((java.sql.Time)ts).toLocalTime();
+ return ts;
+ })
+ });
+ }
+ }
+
+ return params;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.getSqlConfiguration().setValidationEnabled(isValidationEnabled);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void beforeTest() throws Exception {
+ if (!G.allGrids().isEmpty())
+ return;
+
+ startGrids(nodeCount());
+
+ client = startClientGrid("client");
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+ }
+
+ /** */
+ @Test
+ public void testTemporalTypes() {
+ createTable();
+
+ executeSql("CREATE INDEX DATA_IDX ON DATA(data DESC);");
+
+ Object testObj = generateTestObject(objType);
+
+ executeSql("INSERT INTO DATA(_key, id, data) values(?, ?, ?)", 0, 0, testObj);
+
+ grid(0).cache(DEFAULT_CACHE_NAME).put(1, new Data(1, testObj));
+
+ grid(0).cache(DEFAULT_CACHE_NAME).put(2, grid(0).binary().toBinary(new Data(2, testObj)));
+
+ List<List<?>> selectData = executeSql("SELECT data FROM DATA");
+
+ Object sqlObj = sqlTypeConverter.apply(testObj);
+
+ selectData.get(0).forEach(d -> assertEquals(sqlObj, d));
Review Comment:
`selectData.forEach(d -> assertEquals(sqlObj, d.get(0)));`
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryUtils.java:
##########
@@ -1823,4 +1832,64 @@ public KeyOrValProperty(boolean key, String name, Class<?> cls) {
return -1;
}
}
+
+ /** */
+ private enum TemporalTypes {
+ /** Corresponds to SQL DATE. */
+ DATE(java.sql.Date.class, java.time.LocalDate.class),
+
+ /** Corresponds to SQL TIME. */
+ TIME(java.sql.Time.class, java.time.LocalTime.class),
+
+ /** Corresponds to SQL TIMESTAMP. */
+ TIMESTAMP(java.sql.Timestamp.class, java.util.Date.class, java.time.LocalDateTime.class),
+
+ /** Corresponds to non-temporal types. */
+ NONE();
+
+ /** */
+ private final Set<Class<?>> clsSet = new HashSet<>();
+
+ /** */
+ TemporalTypes(Class<?>... types) {
+ if (types == null)
+ return;
+
+ Collections.addAll(clsSet, types);
+ }
+
+ /** */
+ public static TemporalTypes getKind(Class<?> type) {
+ if (DATE.contains(type))
Review Comment:
Why do we need it additional class if we still use `if/elseif`? It will be simplier and faster to just inline the same if/endif into `isConvertibleTypes` method
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org