github-actions[bot] commented on code in PR #69536:
URL: https://github.com/apache/doris/pull/69536#discussion_r4366960335
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -2327,8 +2341,55 @@ private static List<Schema> metadataSchemaHistory(Table
table) {
return new ArrayList<>(table.schemas().values());
}
- private static void addHistoricalEqualityFields(
- List<NestedField> fields, Set<Integer> missingFieldIds, Schema
historicalSchema) {
+ /**
+ * A writer commits with the schema that is current at commit, so a key
dropped before its delete commits loses
+ * the schema-only changes made before the drop from every snapshot. Field
IDs are never reused and a field
+ * only relaxes compatibly, by promotion or from required to optional, so
each field of the selected definition
+ * takes the widest promotion of its type and the optionality it has in
any retained schema; neither depends on
+ * the order of the schemas. A field required in every retained schema
stays required.
+ */
+ private static List<NestedField> withRetainedRelaxations(List<NestedField>
fields, List<Schema> schemas) {
+ List<NestedField> relaxed = new ArrayList<>(fields.size());
+ for (NestedField field : fields) {
+ Type type = field.type();
+ boolean optional = field.isOptional();
+ for (Schema schema : schemas) {
+ NestedField retained = schema.findField(field.fieldId());
+ if (retained == null) {
+ continue;
+ }
+ optional |= retained.isOptional();
+ if (type.isPrimitiveType() && retained.type().isPrimitiveType()
+ && isWideningPromotion(type.asPrimitiveType(),
retained.type().asPrimitiveType())) {
+ type = retained.type();
+ }
+ }
+ if (type.isStructType()) {
+ type =
Types.StructType.of(withRetainedRelaxations(type.asStructType().fields(),
schemas));
+ }
+ relaxed.add(type.equals(field.type()) && optional ==
field.isOptional()
+ ? field :
Types.NestedField.from(field).ofType(type).isOptional(optional).build());
Review Comment:
[P2] Convert DATE defaults before rebuilding a promoted carrier
A non-null DATE default makes this rebuild fail. Iceberg 1.12.0's
`NestedField.from(field)` copies the typed default literals, and the
constructor converts them to the new type; `DateLiteral.to` accepts only DATE,
so TIMESTAMP/TIMESTAMP_NS produces `Cannot cast default value to timestamp...`.
See the pinned [builder/default
conversion](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/api/src/main/java/org/apache/iceberg/types/Types.java)
and
[DateLiteral](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/api/src/main/java/org/apache/iceberg/expressions/Literals.java).
For an unpartitioned v3 table, give optional `k DATE` initial default
`2024-01-02`, append S1, publish a schema-only TIMESTAMP promotion with the
equivalent midnight default, prepare an equality delete, drop k, then commit
the pending delete. As in the new temporal tests, publish the promotion through
`TableMetadata.Builder`. The lineage selects S1's DATE definition; retained
widening then reaches this builder and aborts FE planning before either reader
runs. The previous metadata lookup selected the already-typed TIMESTAMP
definition. A copied DATE write default also triggers this.
Convert both defaults into the promoted temporal type before rebuilding and
add a default-bearing test. This differs from the existing temporal thread's
fixed no-default comparison failure.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -2327,8 +2341,55 @@ private static List<Schema> metadataSchemaHistory(Table
table) {
return new ArrayList<>(table.schemas().values());
}
- private static void addHistoricalEqualityFields(
- List<NestedField> fields, Set<Integer> missingFieldIds, Schema
historicalSchema) {
+ /**
+ * A writer commits with the schema that is current at commit, so a key
dropped before its delete commits loses
+ * the schema-only changes made before the drop from every snapshot. Field
IDs are never reused and a field
+ * only relaxes compatibly, by promotion or from required to optional, so
each field of the selected definition
+ * takes the widest promotion of its type and the optionality it has in
any retained schema; neither depends on
+ * the order of the schemas. A field required in every retained schema
stays required.
+ */
+ private static List<NestedField> withRetainedRelaxations(List<NestedField>
fields, List<Schema> schemas) {
+ List<NestedField> relaxed = new ArrayList<>(fields.size());
+ for (NestedField field : fields) {
+ Type type = field.type();
+ boolean optional = field.isOptional();
+ for (Schema schema : schemas) {
+ NestedField retained = schema.findField(field.fieldId());
+ if (retained == null) {
+ continue;
+ }
+ optional |= retained.isOptional();
+ if (type.isPrimitiveType() && retained.type().isPrimitiveType()
+ && isWideningPromotion(type.asPrimitiveType(),
retained.type().asPrimitiveType())) {
+ type = retained.type();
Review Comment:
[P2] Preserve the historical DATE domain across future nanos promotions
Scanning every retained schema also imports promotions committed after a
time-travel target. DATE-to-TIMESTAMP_NS has a smaller representable range: the
[v3
specification](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/format/spec.md#schema-evolution)
requires out-of-range values to fail at runtime, so this promotion is not
harmless for an earlier DATE read.
Use an unpartitioned v3 table with optional `k DATE`, a row at `1000-01-01`,
and a DATE equality delete for `1001-01-01`, without column metrics. Drop k and
append an id-only file to establish target T. Later restore k's old nullable
schema with `TableMetadata.Builder.setCurrentSchema(existingId)` and publish a
TIMESTAMP_NS schema. Nullable-field deletion rollback is allowed by the same
spec, and the [metadata
builder](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/core/src/main/java/org/apache/iceberg/TableMetadata.java)
accepts this history.
Reading T should still compare DATEs and retain both rows. Instead, its
lineage's DATE carrier is widened by that future schema. V2
`_resolve_equality_delete_fields` chooses TIMESTAMP_NS and
`_load_equality_delete_file` uses `EvolutionCast`, whose DATE-to-nanos strict
conversion rejects the year-1001 tombstone. The previous lookup stopped at T's
schema and kept DATE. Preserve historical comparison domains for
range-restricting promotions and add this pinned-read case. This is distinct
from the existing temporal thread: the problematic promoted schema is after T,
and no defaults are needed.
--
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]
