[core] Refactor Variant Parquet reads onto a unified shredding read path by lxy-9602 · Pull Request #8392 · apache/paimon

lxy-9602 · 2026-06-30T08:23:00Z

Purpose

This is a subtask of MAP shared-shredding support.

Description

This PR introduces a shared shredding read framework to align the read path with the existing shredding write plan abstraction. A ShreddingReadPlanFactory can now create a logical-to-physical read plan and a batch assembler, while ShreddingFormatReader wraps the underlying format reader and converts physical batches back to logical batches before returning rows upstream.

This also migrates Parquet Variant reads onto the new read-plan framework. Variant physical schema conversion and batch assembly are moved out of the Parquet column vector internals into VariantShreddingReadPlanFactory, so Variant follows the same model intended for future MAP shared-shredding support: logical schema -> physical read schema -> physical batch -> logical batch.

Introduces a common shredding read-plan abstraction with ShreddingReadPlanFactory, ShreddingReadPlan, and ShreddingBatchAssembler.
Adds ShreddingFormatReader to wrap format readers and assemble logical batches from physical batches.
Refactors Parquet Variant reads onto the new shredding read-plan path.
Moves Variant physical schema conversion and batch assembly out of Parquet column-vector internals.
Adds format-level field metadata support through SupportsFieldMetadata.
Exposes field metadata from ORC and Parquet file formats.
Updates Arrow schema metadata compatibility handling and adds coverage for rich format metadata.
Removes obsolete Variant-specific helper/vector wrapper code replaced by the new read-plan path.

Tests

Tests

mvn -pl paimon-format -am -DfailIfNoTests=false clean test
mvn -pl paimon-common -am -Pfast-build -DfailIfNoTests=false -Dtest=ShreddingReadPlanFactoriesTest test
mvn -pl paimon-arrow -am -Pfast-build -DfailIfNoTests=false -Dtest=ArrowSchemaMetadataCompatibilityTest test
mvn -pl paimon-core -am -Pfast-build -DfailIfNoTests=false -Dtest=SchemaEvolutionTest#testAddVariantFieldWithShredding test

JingsongLi · 2026-07-01T03:37:27Z

+        }
+
+        @Override
+        public ColumnVector[] getChildren() {


This vector is still exposed through the vectorized read path: ShreddingFormatReader copies the delegate VectorizedRowIterator, so callers such as ArrowVectorizedBatchConverter can consume the returned batch directly. The Arrow map writer calls mapColumnVector.getChildren()[0/1], so returning null here makes a shared-shredding MAP column fail with an NPE when exported through the vectorized Arrow path. Could we either materialize key/value child vectors for this wrapper or force this reader to fall back to a row iterator for shared-shredding maps?

Thanks for the suggestion! Updated MapSharedShreddingReadPlan to materialize MAPs columnarly with key/value child vectors. Also added an e2e append read/write test covering shared-shredding MAP values with the supported nested types.

JingsongLi · 2026-07-03T11:10:26Z

+            RowType logicalRowType, GroupType parquetType, boolean caseSensitive) {
+        List<DataField> fields = new ArrayList<>();
+        for (DataField field : logicalRowType.getFields()) {
+            Type fileType = matchParquetField(parquetType, field.name(), caseSensitive);


This read plan is created whenever the requested row type contains any Variant field, but buildPhysicalRowType resolves every top-level field directly from the Parquet file schema. If schema evolution adds a non-Variant column after older files were written, that column is absent from fileSchema; the previous Parquet reader path let clipParquetSchema synthesize a missing column and fill NULLs, while this throws here before requested schema creation. Please preserve the missing-column path, for example by keeping the logical field type when the file field is absent and letting createRequestedSchema mark it missing, and add a test with a Variant column plus a newly added field read from old Parquet files.

Thanks for pointing this out. I have updated the read plan to be defensive here.

One note: in the normal table read path, schema-evolution missing columns should already be handled before reaching the format reader. FormatReaderMapping builds the actual format read type from the file data schema, so fields absent from an old file are not included in the actualReadRowType passed to Parquet/ORC. Those fields are then represented by indexMapping = -1 and filled as NULL by DataFileRecordReader / VectorMappingUtils.

That said, keeping the missing-field handling in the Variant read plan makes the format-level code more robust and avoids surprises if this plan is used from a more direct format path.

JingsongLi · 2026-07-03T11:10:27Z

        }
+
+        if (hasSharedShredding) {
+            validateMapSharedShreddingFileFormats(options);


Schema validation accepts shared-shredding tables with file.compression=snappy or other Parquet/ORC codecs, but MapSharedShreddingWritePlan.fieldMetadata(compression) passes that same codec into normalizeFieldDictCompression, which only allows zstd, lz4, or none. Such a table can be created successfully and then fail only when the writer closes after producing data. Please either validate the relevant data/changelog/per-level compression options here for shared-shredding, or decouple field-dictionary compression from the data-file codec.

Thanks for the review, fixed now.

JingsongLi · 2026-07-03T11:10:27Z

+    }
+
+    private static void validateNoVariantWithMapSharedShredding(TableSchema schema) {
+        if (containsVariantFields(new RowType(schema.fields()))) {


The shared-shredding validation only rejects Variant recursively, but BLOB can still be nested in a shredding MAP value because the existing BLOB checks only inspect top-level fields. A schema such as MAP<STRING, BLOB> passes validation and then fails when the read plan creates RowToColumnConverter.createElementConverter(mapType.getValueType()), because BlobType currently throws UnsupportedOperationException. Please reject BLOB recursively for shared-shredding MAP values or implement BLOB materialization, and add a validation test for MAP<STRING, BLOB>.

Thanks for the review! I’ve updated the validation according to the comments.

JingsongLi · 2026-07-03T13:30:41Z

+            this.keyConverter =
+                    RowToColumnConverter.createElementConverter(this.mapType.getKeyType());
+            this.valueConverter =
+                    RowToColumnConverter.createElementConverter(this.mapType.getValueType());


This path allows MAP<STRING, CHAR> values to reach RowToColumnConverter. The shared converter currently handles CharType with row.getByte(...) and WritableByteVector, while ColumnVectorUtils.createWritableColumnVector creates a HeapBytesVector for CHAR (same as VARCHAR). As a result, materializing a shared-shredding MAP whose value type contains CHAR will fail or use the wrong accessor. Please either fix CharType in RowToColumnConverter to use the string/bytes path and add a regression, or reject CHAR in the shared-shredding validation until it is supported.

JingsongLi · 2026-07-03T13:31:29Z

+        if (hasSharedShredding) {
+            validateMapSharedShreddingFileFormats(options);
+            validateMapSharedShreddingCompressions(options);
+            validateNoVariantOrBlobWithMapSharedShredding(schema);


This validation still allows shared-shredding maps whose value type contains MULTISET, for example MAP<STRING, MULTISET<INT>>. The write converter can carry that value as an InternalMap, but read materialization later calls RowToColumnConverter.createElementConverter(valueType), and RowToColumnConverter.visit(MultisetType) currently throws UnsupportedOperationException. That means such a table can be created and written but fails when the shared-shredding map is read back. Please either implement multiset conversion in the row-to-column path or reject MultisetType recursively here with the other unsupported types.

Thanks a lot for your review and time! I learned a lot from the feedback and have made a few more fixes.

JingsongLi · 2026-07-04T02:06:35Z

This PR has become too large, can we split some PRs out.

lxy-9602 · 2026-07-07T04:38:26Z

This PR has become too large, can we split some PRs out.

Thanks for the detailed review. To make the changes easier to review, I have split the original PR into smaller independent parts:

Refactor the Variant read path to use ShreddingReadPlan, so Variant materialization is no longer tightly coupled with the Parquet reader (this pr).
Add MAP shared-shredding schema validation and common utility updates ([core] Validate unsupported MAP shared-shredding configurations #8487).
Add append-table MAP shared-shredding read/write support, including the full append E2E test coverage.

This should make each part more focused and avoid mixing framework refactoring, validation, and the MAP shared-shredding read/write implementation in one large PR. Would be grateful if you could take another look when you have time.

lxy-9602 mentioned this pull request Jun 30, 2026

[core] Support append read write for MAP shared-shredding #8355

Closed

lxy-9602 added 3 commits June 30, 2026 17:46

Support append read/write for MAP shared-shredding

4fd70b7

fix compile

6269e8b

add more tests

82dec42

lxy-9602 force-pushed the append-shredding-write2 branch from 48b8d94 to 82dec42 Compare June 30, 2026 09:46

JingsongLi reviewed Jul 1, 2026

View reviewed changes

lxy-9602 added 2 commits July 1, 2026 15:29

fix review

cb67ed7

fix little

2be2d91

JingsongLi reviewed Jul 3, 2026

View reviewed changes

fix review: with more validate

b0b5441

JingsongLi reviewed Jul 3, 2026

View reviewed changes

lxy-9602 added 2 commits July 3, 2026 22:11

fix char type and more validate

917bdf4

add more tests

96e0ac5

lszskye mentioned this pull request Jul 6, 2026

[spark] prepare MAP selected-key pushdown read type #8478

Open

[core] Refactor Variant Parquet reads onto a unified shredding read path

3f40452

lxy-9602 changed the title ~~[core] Support append read/write for MAP shared-shredding~~ [core] Refactor Variant Parquet reads onto a unified shredding read path Jul 7, 2026

lxy-9602 added 3 commits July 7, 2026 10:45

fix little

ed936e4

add tests

686cdc0

fix tests

3cb1599

Uh oh!

Conversation

lxy-9602 commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Tests

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lxy-9602 Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JingsongLi commented Jul 4, 2026

Uh oh!

lxy-9602 commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lxy-9602 commented Jun 30, 2026 •

edited

Loading

lxy-9602 Jul 3, 2026 •

edited

Loading