korlov42 commented on code in PR #5119:
URL: https://github.com/apache/ignite-3/pull/5119#discussion_r1940896368


##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ItSqlLogicTest.java:
##########
@@ -145,7 +145,7 @@
 @WithSystemProperty(key = "IMPLICIT_PK_ENABLED", value = "true")
 // The following is to make sure we unlock LWM on data nodes promptly so that 
dropped tables are destroyed fast.
 @WithSystemProperty(key = 
ResourceVacuumManager.RESOURCE_VACUUM_INTERVAL_MILLISECONDS_PROPERTY, value = 
"1000")
-@SqlLogicTestEnvironment(scriptsRoot = "src/integrationTest/sql/group1")
+@SqlLogicTestEnvironment(scriptsRoot = "src/integrationTest/sql/group1", regex 
= "test_timestamp_ms")

Review Comment:
   unrelated change



##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CatalogUtilsTest.java:
##########
@@ -186,6 +196,66 @@ void testClusterWideEnsuredActivationTimestamp() {
         assertEquals(expClusterWideActivationTs, 
clusterWideEnsuredActivationTimestamp(catalog.time(), 
TEST_MAX_CLOCK_SKEW_MILLIS));
     }
 
+    @ParameterizedTest
+    @MethodSource("columnTypesPrecision")
+    void testGetPrecision(ColumnType columnType, int min, int max) {
+        assertEquals(CatalogUtils.getMinPrecision(columnType), min, "min");
+        assertEquals(CatalogUtils.getMaxPrecision(columnType), max, "max");
+    }
+
+    private static Stream<Arguments> columnTypesPrecision() {
+        Stream<Arguments> types = Stream.of(
+                Arguments.of(DECIMAL, 1, 32767),
+                Arguments.of(TIME, 0, 9),
+                Arguments.of(TIMESTAMP, 0, 9)

Review Comment:
   should we add DATETIME, PERIOD, and DURATION?



##########
modules/system-view/src/main/java/org/apache/ignite/internal/systemview/utils/SystemViewUtils.java:
##########
@@ -141,8 +141,10 @@ private static ColumnParams 
systemViewColumnToColumnParams(SystemViewColumn<?, ?
             case STRING:
             case BYTES:
                 assert type instanceof VarlenNativeType : 
type.getClass().getCanonicalName();
+                VarlenNativeType varlenNativeType = (VarlenNativeType) type;
 
-                builder.length(((VarlenNativeType) type).length());
+                int maxLength = 
CatalogUtils.getMaxLength(typeSpec.asColumnType());
+                builder.length(Math.min(varlenNativeType.length(), maxLength));

Review Comment:
   I think, it's better to change NativeType



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -547,6 +582,133 @@ public static HybridTimestamp 
clusterWideEnsuredActivationTimestamp(long activat
         return defaultZone != null ? defaultZone.id() : null;
     }
 
+
+    /**
+     * Returns the maximum supported precision for given type or {@code -1} if 
the type does not support precision.
+     *
+     * @param columnType Column type
+     * @return Maximum precision
+     */
+    public static int getMaxPrecision(ColumnType columnType) {
+        if (!columnType.precisionAllowed()) {
+            return -MIN_DECIMAL_PRECISION;

Review Comment:
   it's better to introduce dedicated constant for this case



-- 
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