JingsongLi commented on code in PR #9532:
URL: https://github.com/apache/paimon/pull/9532#discussion_r3911805771
##########
paimon-common/src/main/java/org/apache/paimon/data/variant/InferVariantShreddingSchema.java:
##########
@@ -654,6 +658,43 @@ private DataType finalizeAdaptiveSchema(
return selectScalarType(combined, current, previousSelected);
}
+ /**
+ * Carries a previously selected schema forward for a node the current
file has no evidence for.
+ * The selection is already final, so nothing is re-thresholded, but its
nodes still consume the
+ * shared width budget. The entry unit for this node has already been
spent by the caller, so
+ * this mirrors what finalizeAdaptiveSchema does from that point on: a
child is entered only
+ * while budget remains, entering it spends one unit, and a child that
exhausts the budget
+ * becomes VARIANT while its field or array container is still kept.
+ */
+ private DataType retainSelectedSchema(DataType selected, MaxFields
maxFields) {
+ if (selected instanceof RowType) {
+ List<DataField> fields = new ArrayList<>();
+ for (DataField field : ((RowType) selected).getFields()) {
+ if (maxFields.remaining <= 0) {
+ break;
+ }
+ maxFields.remaining--;
+ DataType retained =
+ maxFields.remaining <= 0
+ ? DataTypes.VARIANT()
+ : retainSelectedSchema(field.type(),
maxFields);
+ fields.add(new DataField(fields.size(), field.name(),
retained));
+ }
+ return fields.isEmpty() ? DataTypes.VARIANT() : new
RowType(fields);
+ }
+ if (selected instanceof ArrayType) {
+ maxFields.remaining--;
+ DataType element =
+ maxFields.remaining <= 0
+ ? DataTypes.VARIANT()
+ : retainSelectedSchema(
+ ((ArrayType) selected).getElementType(),
maxFields);
+ return new ArrayType(element);
+ }
+ maxFields.remaining--;
Review Comment:
[P2] Do not charge retained VARIANT leaves twice
The caller already spends the node entry unit before `retainSelectedSchema`
is reached. For a `VariantType`, there is no typed-value child to spend another
unit on; the normal `finalizeAdaptiveSchema` path likewise returns `VARIANT`
after only the entry debit. This branch therefore overcharges retained
`VARIANT` leaves.
A minimal boundary case reproduces the regression: use two top-level variant
columns with `maxSchemaWidth = 3`, infer/commit `[null, 5]`, then infer `[null,
6]`. The first retained `VARIANT` should cost one unit and leave two for the
second `BIGINT`, but this extra decrement exhausts the budget and the second
column becomes untyped `VARIANT`. I confirmed that assertion fails against this
head while the existing 27 tests pass. Please special-case `VariantType` here
and add the shared-budget regression.
--
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]
