zstan commented on code in PR #5515: URL: https://github.com/apache/ignite-3/pull/5515#discussion_r2032427595
########## modules/platforms/dotnet/Apache.Ignite.Tests/Proto/ColocationHashTests.cs: ########## @@ -96,10 +96,11 @@ public class ColocationHashTests : IgniteTestsBase LocalTime.Noon, LocalDateTime.FromDateTime(DateTime.UtcNow).TimeOfDay, default(LocalTime), - new LocalDateTime(year: 1, month: 1, day: 1, hour: 1, minute: 1, second: 1, millisecond: 1), + + // Minimum allowed DATETIME value. + new LocalDateTime(year: 1, month: 1, day: 1, hour: 18, minute: 0, second: 0, millisecond: 0), Review Comment: Seems "Maximum allowed DATETIME value" need to be present too ########## modules/api/src/testFixtures/java/org/apache/ignite/table/AbstractImmutableTupleTest.java: ########## @@ -440,4 +436,13 @@ private static int tailFactor(int precision) { throw new IllegalArgumentException("Unsupported type: " + type); } } + + private static Instant generateInstant(Random rnd) { Review Comment: do we really need such logic here ? These values are not passed through validation at all. i.e. do we really need some bounds mathematics here, it`s hard to understand in future or i miss smth ? ########## modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueViewSimpleSchemaApiTest.java: ########## @@ -537,6 +540,167 @@ public void putGetAllTypes(AllTypesTestCase testCase) { } } + @ParameterizedTest + @MethodSource("dateBoundaryTestCases") + @SuppressWarnings("ThrowableNotThrown") + public void putDateBoundaryValues(AllTypesTestCase testCase) { + try { + KeyValueView<Long, Object> view = testCase.view(); + + // Put min and max allowed values. + view.put(null, 1L, SchemaUtils.DATE_MIN); + view.put(null, 2L, SchemaUtils.DATE_MAX); + + // Verify them using KV API. + { + assertEquals(SchemaUtils.DATE_MIN, view.get(null, 1L)); + assertEquals(SchemaUtils.DATE_MAX, view.get(null, 2L)); + } + + // Verify them using SQL API. + { + String query = "SELECT VAL, VAL::VARCHAR FROM " + testCase.tableName + " WHERE id = ?"; + { + List<List<Object>> res = sql(query, 1); + + assertEquals(SchemaUtils.DATE_MIN, res.get(0).get(0)); + assertEquals("0001-01-01", res.get(0).get(1)); Review Comment: ```suggestion assertEquals(SchemaUtils.DATE_MIN.toString(), res.get(0).get(1)); ``` ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteMethod.java: ########## @@ -59,6 +59,15 @@ public enum IgniteMethod { /** See {@link IgniteSqlFunctions#subtractTimeZoneOffset(long, TimeZone)}. **/ SUBTRACT_TIMEZONE_OFFSET(IgniteSqlFunctions.class, "subtractTimeZoneOffset", long.class, TimeZone.class), + /** See {@link IgniteSqlFunctions#toDateExact(int)}. **/ + TO_DATE_EXACT(IgniteSqlFunctions.class, "toDateExact", int.class), + + /** See {@link IgniteSqlFunctions#toTimestampExact(Object)}. **/ + TO_TIMESTAMP_EXACT(IgniteSqlFunctions.class, "toTimestampExact", long.class), + + /** See {@link IgniteSqlFunctions#toTimestampLtzExact(Object)}. **/ + TO_TIMESTAMP_WLTZ_EXACT(IgniteSqlFunctions.class, "toTimestampLtzExact", Object.class), Review Comment: ```suggestion TO_TIMESTAMP_LTZ_EXACT(IgniteSqlFunctions.class, "toTimestampLtzExact", Object.class), ``` ########## modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItKeyValueViewSimpleSchemaApiTest.java: ########## @@ -537,6 +540,167 @@ public void putGetAllTypes(AllTypesTestCase testCase) { } } + @ParameterizedTest + @MethodSource("dateBoundaryTestCases") + @SuppressWarnings("ThrowableNotThrown") + public void putDateBoundaryValues(AllTypesTestCase testCase) { + try { + KeyValueView<Long, Object> view = testCase.view(); + + // Put min and max allowed values. + view.put(null, 1L, SchemaUtils.DATE_MIN); + view.put(null, 2L, SchemaUtils.DATE_MAX); + + // Verify them using KV API. + { + assertEquals(SchemaUtils.DATE_MIN, view.get(null, 1L)); + assertEquals(SchemaUtils.DATE_MAX, view.get(null, 2L)); + } + + // Verify them using SQL API. + { + String query = "SELECT VAL, VAL::VARCHAR FROM " + testCase.tableName + " WHERE id = ?"; + { + List<List<Object>> res = sql(query, 1); + + assertEquals(SchemaUtils.DATE_MIN, res.get(0).get(0)); + assertEquals("0001-01-01", res.get(0).get(1)); + } + { + List<List<Object>> res = sql(query, 2); + + assertEquals(SchemaUtils.DATE_MAX, res.get(0).get(0)); + assertEquals("9999-12-31", res.get(0).get(1)); Review Comment: ```suggestion assertEquals(SchemaUtils.DATE_MAX.toString(), res.get(0).get(1)); ``` ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java: ########## @@ -45,6 +50,15 @@ * Ignite SQL functions. */ public class IgniteSqlFunctions { + private static final int DATE_MIN_INTERNAL = (int) TypeUtils.toInternal(SchemaUtils.DATE_MIN, LocalDate.class); Review Comment: TypeUtils.toInternal - already deprecated method, you need to avoid it`s usage ########## modules/marshaller-common/src/testFixtures/java/org/apache/ignite/internal/marshaller/testobjects/TestObjectWithAllTypes.java: ########## @@ -63,8 +62,7 @@ public static TestObjectWithAllTypes randomObject(Random rnd) { obj.dateCol = LocalDate.ofYearDay(1990 + rnd.nextInt(50), 1 + rnd.nextInt(360)); obj.timeCol = LocalTime.of(rnd.nextInt(24), rnd.nextInt(60)); obj.dateTimeCol = LocalDateTime.of(obj.dateCol, obj.timeCol); - obj.timestampCol = Instant.ofEpochMilli(rnd.nextLong()).truncatedTo(ChronoUnit.SECONDS) - .plusNanos(normalizeNanos(rnd.nextInt(1_000_000_000), 6)); + obj.timestampCol = (Instant) SchemaTestUtils.generateRandomValue(rnd, NativeTypes.timestamp(6)); Review Comment: I wonder - why do we need additional random objects generator ? Can we rewrite all to SchemaTestUtils ? Also why do we need both: obj.primitiveBooleanCol = rnd.nextBoolean(); and obj.booleanCol = rnd.nextBoolean(); -- 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