klsince commented on code in PR #15283:
URL: https://github.com/apache/pinot/pull/15283#discussion_r1999550448


##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java:
##########
@@ -2587,4 +2625,4 @@ public void testNoRGRealtimeTable() {
 
     
Assert.assertFalse(TableConfigUtils.isTableUsingInstancePoolAndReplicaGroup(tableConfig));
   }
-}
+}

Review Comment:
   unnecessary change?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -1199,6 +1216,8 @@ private static void validateFieldConfigList(TableConfig 
tableConfig, @Nullable S
     }
     assert indexingConfig != null;
 
+

Review Comment:
   empty lines added by accident? and there seems to be linter check error



##########
pinot-segment-local/pom.xml:
##########
@@ -136,4 +136,17 @@
       <artifactId>clp-ffi</artifactId>
     </dependency>
   </dependencies>
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <source>23</source>
+          <target>23</target>
+          <compilerArgs>--enable-preview</compilerArgs>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>

Review Comment:
   is this change required?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -1243,6 +1262,12 @@ private static void validateFieldConfigList(TableConfig 
tableConfig, @Nullable S
       // Validate the forward index disabled compatibility with other indexes 
if enabled for this column
       validateForwardIndexDisabledIndexCompatibility(columnName, fieldConfig, 
indexingConfig, schema, tableType);
 
+      // Validate bloom filter is not added to boolean column
+      if (fieldConfig.getIndexes() != null && 
fieldConfig.getIndexes().has("bloom") ) {

Review Comment:
   could this check be moved inside the switch-case block?



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to