JingsongLi commented on code in PR #10226:
URL: https://github.com/apache/paimon/pull/10226#discussion_r4283679597
##########
paimon-common/src/main/java/org/apache/paimon/data/variant/PaimonShreddingUtils.java:
##########
@@ -598,16 +613,24 @@ public static InternalRow assembleVariantStruct(
ByteBuffer topLevelMetadata = binaryBuffer(inputRow,
schema.topLevelMetadataIdx);
int numFields = fields.length;
GenericRow resultRow = new GenericRow(numFields);
+ if (numFields == 0) {
+ return resultRow;
+ }
+
+ Set<VariantMetadata> adopted = Collections.newSetFromMap(new
IdentityHashMap<>());
+ for (FieldToExtract field : fields) {
+ VariantMetadata metadata = field.reader().metadata();
+ if (adopted.add(metadata)) {
+ metadata.adopt(topLevelMetadata);
Review Comment:
[P2] Adopt metadata lazily for fully typed projections
This now compares the entire metadata buffer for every row even when all
requested fields are already present in typed_value. In the baseline,
projecting a non-null shredded INT goes straight to ScalarReader.getInt; it
does not parse or compare the unrelated dictionary. The Parquet pruner retains
the real metadata column for possible fallback, so this is also reached by
ordinary projected file reads through assembleVariantStructBatch.
I reproduced this with actual ParquetFileFormat shredding writes and
projected reads, using the same physical files and checking all 5,000 returned
INT values and their checksum. With 10,000 unprojected leftover keys (158,905
metadata bytes), reading only $.typed as INT takes a median 25.21 ms on this
head versus 2.31 ms on the exact merge-base
10cf6ebc7c4f0ba6066d32d8aed2b0b85e0b8349 (JDK 8/G1, warm local files, separate
JVMs; seven measured scans after warmup). With 1,000 keys it is 6.19 vs 3.89
ms. Both files return 5,000 rows and sum 615,000. Reversing JVM execution order
still gives 24.40 ms on HEAD versus 2.76 ms on the baseline for the 10,000-key
file (8.84x). A separate production-helper size sweep also confirms the new
per-row cost grows with dictionary size.
Please defer adoption until the current row first actually needs metadata
for fallback/rebuild/dictionary access, sharing that adoption across recursive
readers. Keep dynamic fallback for rows without a typed value and
backing-buffer reuse detection; a schema-only or buffer-identity shortcut would
be unsafe. Add a fully typed narrow-projection case with large unprojected
metadata so the cache optimization does not make that read linear in unrelated
dictionary size.
--
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]
