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

Reply via email to