You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "deniskuzZ (via GitHub)" <gi...@apache.org> on 2025/09/22 15:05:59 UTC

[PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

deniskuzZ opened a new pull request, #14153:
URL: https://github.com/apache/iceberg/pull/14153

   (no comment)


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379042090


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -266,4 +279,37 @@ public void testInvalidUpperBoundBinary() throws Exception {
     assertThat(dataFile.lowerBounds()).as("Should have a valid lower bound").containsKey(3);
     assertThat(dataFile.upperBounds()).as("Should have a null upper bound").doesNotContainKey(3);
   }
+
+  @Test
+  public void testDataWriterWithVariantShredding() throws IOException {

Review Comment:
   Consider adding tests for edge cases:
   
     1. **Null shredding function**: What happens when `variantShreddingFunc(null)` is called?
     2. **No shredding function set**: Default behavior when the method is never called
     3. **Exception handling**: If the shredding function throws an exception during writing
   



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3335800369

   Added an open question to the File Format API doc: https://docs.google.com/document/d/1sF_d4tFxJsZWsZFCyCL9ZE7YuI7-P3VrzMLIrrTIxds/edit?pli=1&tab=t.0#heading=h.svg0lmr788x2
   
   We will discuss this on the File Format API sync on Monday. I would be happy to talk to you about it there.
   
   Thanks,
   Peter


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379441672


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   added



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3338236549

   > > If our goal is to support all V3 features across all file formats
   > 
   > what if variant shredding won't ever be supported by orc or avro?
   
   That’s a fair question. When I proposed introducing new file formats like Vortex, the first concern raised was that we must ensure feature parity across all supported formats. Otherwise, users would have to navigate a complex matrix of supported vs. unsupported features — which is a valid usability concern.
   
   To me, that implies we either implement these features across all formats or reconsider the long-term viability of formats that can’t support them.
   
   > > You might want to check out the File Format API proposal, since it will impact Hive integration as well.
   > 
   > yes, I intended to, but never found the time. Now it seems I can’t postpone it any longer.
   
   Totally understandable — keeping up with multiple open proposals is no small task. Sorry for adding to the pile!
    
   > > the API has planned to expose an inputSchema(S type) method, allowing engines to pass in the input schema — which implicitly defines the shredding configuration.
   > 
   > Is this API available, and can we use it right now and pass shredding configuration?
   
   Not yet. I’d like to finalize the design first. In the meantime, I’ve been maintaining a PoC implementation to test the concepts — you can find it here: #12298.
   Note: variant handling hasn’t been added to it yet.
   
   > > while we do need to expose this functionality, we might want to rethink how we expose it.
   > 
   > That’s totally fine, we’ve only just begun work on variant type support.
   
   👍 
   Thanks for all the info, and understanding!
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2377284529


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   @huaxingao i'm OK if we want to do that, I think we'd have to read the footer and get the schema from that and extract that the variant has a typed_value etc....it's not too much additional work and does give us confidence that the parquet file is shredded as expected. 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379332825


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {

Review Comment:
   Yeah I think this one is pretty clear



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379390099


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   @deniskuzZ Logical round‑trip passes even without shredding, so the test will succeed without calling `.variantShreddingFunc(variantShreddingFunc)` (false positive). That’s why we should also assert the Parquet footer shows a shredded schema . If you don’t have time to add this, I can have a followup PR for this.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379177230


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {
+      appenderBuilder.variantShreddingFunc(func);

Review Comment:
   there are no such check in other DataWriteBuilder methods



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379202074


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -266,4 +279,37 @@ public void testInvalidUpperBoundBinary() throws Exception {
     assertThat(dataFile.lowerBounds()).as("Should have a valid lower bound").containsKey(3);
     assertThat(dataFile.upperBounds()).as("Should have a null upper bound").doesNotContainKey(3);
   }
+
+  @Test
+  public void testDataWriterWithVariantShredding() throws IOException {

Review Comment:
   This PR does not cover the implementation of the Variant type writer. The related tests can be found in TestVariantWriters



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2370003089


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {
+      appenderBuilder.variantShreddingFunc(func);
+      return this;
+    }

Review Comment:
   Could we add a test to `TestParquetDataWriter`? 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379034525


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {

Review Comment:
   Since it is public method, consider adding JavaDoc to document the purpose and usage of this method



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379441672


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   added. @huaxingao, please check whether this aligns with what you expected



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379220347


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   i think that is out of scope of this PR as it doesn't cover the Variant type writer implementation. Suggested test should be added in TestVariantWriters



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379177230


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {
+      appenderBuilder.variantShreddingFunc(func);

Review Comment:
   Other DataWriteBuilder methods don’t perform such checks. For example, see createWriterFunc



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3337165331

   > How will this method be used?
   
   This method is intended for use in HiveFileWriterFactory#configureDataWrite(Parquet.DataWriteBuilder builder). The HiveFileWriterFactory itself extends BaseFileWriterFactory<Record>.
   
   > What is the plan to provide the same functionality for Avro and ORC?
   
   I’m not aware of any such plans. At the moment, variant shredding is only supported in Parquet.
   
   > I have checked the PR, and I'm afraid, that we need to revisit exposing this change. This is a File Format specific method
   
   I don’t see how this change could interfere with anything. It’s just a simple setter for the variant shredding function in the Parquet FileAppender, and indeed, it’s a file format specific method.
   
   How else can we take advantage of the variant shredding feature?
   
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3338043741

   > If our goal is to support all V3 features across all file formats
   
   what if variant shredding won't ever be supported by orc or avro?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3324805175

   also cc @aihuaxu 


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2372777308


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -266,4 +278,40 @@ public void testInvalidUpperBoundBinary() throws Exception {
     assertThat(dataFile.lowerBounds()).as("Should have a valid lower bound").containsKey(3);
     assertThat(dataFile.upperBounds()).as("Should have a null upper bound").doesNotContainKey(3);
   }
+
+  @Test
+  public void testDataWriterWithVariantShredding() throws IOException {
+    Schema variantSchema =
+        new Schema(
+            ImmutableList.<Types.NestedField>builder()
+                .addAll(SCHEMA.columns())
+                .add(Types.NestedField.optional(4, "variant", Types.VariantType.get()))
+                .build());
+
+    ByteBuffer testMetadataBuffer =
+        VariantTestUtil.createMetadata(ImmutableList.of("a", "b"), true);
+
+    ByteBuffer testObjectBuffer =
+        VariantTestUtil.createObject(
+            testMetadataBuffer,
+            ImmutableMap.of(
+                "a", Variants.of(123456789),
+                "b", Variants.of("string")));
+
+    Variant testVariant =
+        Variant.of(
+            Variants.metadata(testMetadataBuffer),
+            Variants.value(Variants.metadata(testMetadataBuffer), testObjectBuffer));

Review Comment:
   removed the `test` prefix



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379177230


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {
+      appenderBuilder.variantShreddingFunc(func);

Review Comment:
   there are no such checks in other DataWriteBuilder methods. Take a look at createWriterFunc for example



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379185954


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {

Review Comment:
   this is a builder method that is self-explanatory. i can't find other builder methods having a javadoc



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379672300


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   LGTM. Thanks for adding this check!



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2372694582


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   This change in the assertion looks good to me, it's the same approach for how we validate variants are what we expect in `TestVariantWriters`



##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -266,4 +278,40 @@ public void testInvalidUpperBoundBinary() throws Exception {
     assertThat(dataFile.lowerBounds()).as("Should have a valid lower bound").containsKey(3);
     assertThat(dataFile.upperBounds()).as("Should have a null upper bound").doesNotContainKey(3);
   }
+
+  @Test
+  public void testDataWriterWithVariantShredding() throws IOException {
+    Schema variantSchema =
+        new Schema(
+            ImmutableList.<Types.NestedField>builder()
+                .addAll(SCHEMA.columns())
+                .add(Types.NestedField.optional(4, "variant", Types.VariantType.get()))
+                .build());
+
+    ByteBuffer testMetadataBuffer =
+        VariantTestUtil.createMetadata(ImmutableList.of("a", "b"), true);
+
+    ByteBuffer testObjectBuffer =
+        VariantTestUtil.createObject(
+            testMetadataBuffer,
+            ImmutableMap.of(
+                "a", Variants.of(123456789),
+                "b", Variants.of("string")));
+
+    Variant testVariant =
+        Variant.of(
+            Variants.metadata(testMetadataBuffer),
+            Variants.value(Variants.metadata(testMetadataBuffer), testObjectBuffer));

Review Comment:
   Minor: I don't think the `test` prefix in these variable names is particularly useful since they already exist in the context of a test. I'd also inline `testMetadataBuffer` instead of having a separate variable.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3338003415

   > > How will this method be used?
   > 
   > This method is intended for use in `HiveFileWriterFactory#configureDataWrite(Parquet.DataWriteBuilder builder)`. The `HiveFileWriterFactory` itself extends `org.apache.iceberg.data.BaseFileWriterFactory<Record>`.
   
   Nice — I had honestly forgotten how we implemented writes in Hive.
   You might want to check out the File Format API proposal, since it will impact Hive integration as well.
   
   > > What is the plan to provide the same functionality for Avro and ORC?
   > 
   > I’m not aware of any such plans. At the moment, variant shredding is only supported in Parquet.
   
   If we want this feature to be available across all supported file formats, we’ll eventually need a more generic interface.
   
   > > I have checked the PR, and I'm afraid, that we need to revisit exposing this change. This is a File Format specific method
   > 
   > I don’t see how this change could interfere with anything. It’s just a simple setter for the variant shredding function in the Parquet FileAppender, and indeed, it’s a file format specific method.
   > 
   > How else can we take advantage of the variant shredding feature?
   
   If our goal is to support all V3 features across all file formats, we’ll need consistent functionality in every WriteBuilder / DataWriteBuilder. That’s exactly what the File Format API proposal aims to address.
   
   So far, the API has planned to expose an inputSchema(S type) method, allowing engines to pass in the input schema — which implicitly defines the shredding configuration. The VariantShreddingFunction offers an alternative, and we’ll be discussing this in Monday’s File Format API Sync. But even if we go with the function-based approach, we may need to revise the signature to make it more generic. It could return an engine type or an Iceberg type, but tying it to a format-specific type would defeat the purpose of the API — namely, providing a file format–independent interface for users.
   
   Whichever direction we choose, I expect changes in how shredding is handled within writers. That’s why I mentioned that while we do need to expose this functionality, we might want to rethink how we expose it.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3335767743

   I have checked the PR, and I'm afraid, that we need to revisit exposing this change.
   This is a File Format specific method (see the `Type` generic parameter for the BiFunction), and the File Format API proposal aims to generalize them between File Formats.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#issuecomment-3335734295

   How will this method be used?
   
   What is the plan to provide the same functionality for Avro and ORC?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2372881583


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   The logical round‑trip assertions look good. To guard the intended behavior, could we also verify that the physical Parquet schema is shredded when `variantShreddingFunc` is set?



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379390099


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   Logical round‑trip passes even without shredding, so the test will succeed without calling `.variantShreddingFunc(variantShreddingFunc)` (false positive). That’s why we should also assert the Parquet footer shows a shredded schema . If you don’t have time to add this, I can have a followup PR for this.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379985894


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   np, thanks for the thorough review!



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379185954


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {

Review Comment:
   This builder method is self-explanatory, and I couldn’t find Javadocs on other builder methods either.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379328889


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetDataWriter.java:
##########
@@ -113,13 +121,17 @@ public void testDataWriter() throws IOException {
     List<Record> writtenRecords;
     try (CloseableIterable<Record> reader =
         Parquet.read(file.toInputFile())
-            .project(SCHEMA)
-            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(SCHEMA, fileSchema))
+            .project(schema)
+            .createReaderFunc(fileSchema -> GenericParquetReaders.buildReader(schema, fileSchema))
             .build()) {
       writtenRecords = Lists.newArrayList(reader);
     }
 
-    assertThat(writtenRecords).as("Written records should match").isEqualTo(records);
+    assertThat(writtenRecords).hasSameSizeAs(records);
+
+    for (int i = 0; i < records.size(); i++) {
+      InternalTestHelpers.assertEquals(schema.asStruct(), records.get(i), writtenRecords.get(i));

Review Comment:
   @deniskuzZ Yeah I think you're right. At this level of abstraction it's probably not the right assertions to add. TestVariantWriters should verify this in its `writeAndRead` at some point.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar merged PR #14153:
URL: https://github.com/apache/iceberg/pull/14153


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2379032543


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {
+      appenderBuilder.variantShreddingFunc(func);

Review Comment:
   Should we add null check? something like 
   Preconditions.checkArgument(func != null, "Variant shredding function cannot be null");



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Expose variantShreddingFunc() in Parquet.DataWriteBuilder [iceberg]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #14153:
URL: https://github.com/apache/iceberg/pull/14153#discussion_r2371956009


##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -827,6 +827,11 @@ public DataWriteBuilder createWriterFunc(
       return this;
     }
 
+    public DataWriteBuilder variantShreddingFunc(VariantShreddingFunction func) {
+      appenderBuilder.variantShreddingFunc(func);
+      return this;
+    }

Review Comment:
   thanks for checking the PR, @amogh-jahagirdar ! I've added the test.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org