RussellSpitzer commented on a change in pull request #2642:
URL: https://github.com/apache/iceberg/pull/2642#discussion_r750641857



##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/ParquetWriter.java
##########
@@ -197,9 +198,11 @@ private void startRowGroup() {
 
     PageWriteStore pageStore = pageStoreCtorParquet.newInstance(
         compressor, parquetSchema, props.getAllocator(), 
this.columnIndexTruncateLength);
+    Preconditions.checkState(pageStore instanceof BloomFilterWriteStore,
+        "pageStore must be an instance of BloomFilterWriteStore");

Review comment:
       I'm a little confused on how the type might not match here? We get the 
class due to reflection so do we have a chance here of getting the wrong class?
   
   Seems like we may benefit from moving this into the DynamicConstructor code 
at the top of the file? I may misunderstand this but it seems like we should 
try to do this in the reflection itself rather than after we instantiate?




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