pythaac commented on code in PR #10562:
URL: https://github.com/apache/gravitino/pull/10562#discussion_r3088069186


##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -1265,12 +1271,18 @@ private List<Index> getSecondaryIndexes(
           String name = resultSet.getString("name");
           String type = resultSet.getString("type");
           String expression = resultSet.getString("expr");
+          String granularity = resultSet.getString("granularity");
           try {
             String[][] fields = parseIndexFields(expression);
             if (ArrayUtils.isEmpty(fields)) {
               continue;
             }
-            secondaryIndexes.add(Indexes.of(getClickHouseIndexType(type), 
name, fields));
+            Map<String, String> indexProperties = ImmutableMap.of();
+            if (StringUtils.isNotBlank(granularity)) {
+              indexProperties = ImmutableMap.of(GRANULARITY, granularity);
+            }
+            secondaryIndexes.add(
+                Indexes.of(getClickHouseIndexType(type), name, fields, 
indexProperties));

Review Comment:
   From this [clickhouse 
document](https://clickhouse.com/docs/engines/table-engines/mergetree-family/mergetree#data-skipping-indexes),
 there's no blank granularity in here since the granularity has default value.
   
   > The GRANULARITY clause can be omitted, the default value of 
granularity_value is 1.
   
   - It checks blank granularity just in case
   - Added warning when granularity is blank in 
https://github.com/apache/gravitino/pull/10562/changes/de2623b8d3a3d09a0e5577506c50dbc7bb9c96ca



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

Reply via email to