Gabriel39 commented on code in PR #67687:
URL: https://github.com/apache/doris/pull/67687#discussion_r3976180025
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/iceberg/DeleteFileIndex.java:
##########
@@ -406,6 +410,11 @@ Builder afterSequenceNumber(long seq) {
return this;
}
+ Builder schemasById(Map<Integer, Schema> newSchemasById) {
Review Comment:
Fixed in d10663e032. The cache-backed delete index now receives
table.schemas(), and the persisted-table regression drops an equality-delete
key, reloads the table, and verifies lazy cache planning still returns the data
file and delete.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPartitionUtils.java:
##########
@@ -805,11 +803,9 @@ private static IcebergRawPartition
generateRawPartition(Table table, StructLike
for (int i = 0; i < partitionSpec.fields().size(); ++i) {
PartitionField partitionField = partitionSpec.fields().get(i);
Class<?> fieldClass = partitionSpec.javaClasses()[i];
- int fieldId = partitionField.fieldId();
- // Iceberg partition field id starts at PARTITION_DATA_ID_START,
so the index into partitionData is
- // fieldId - PARTITION_DATA_ID_START.
- int index = fieldId - PARTITION_DATA_ID_START;
- Object o = partitionData.get(index, fieldClass);
+ // A spec's partition struct is compact even when evolved field
IDs have gaps, so index by the
+ // field's position in this spec rather than by its table-global
partition field ID.
+ Object o = partitionData.get(i, fieldClass);
Review Comment:
Fixed across d10663e032 and 3f775f5c76. Representable unified partition
fields are read by field ID. If Iceberg 1.11 removes an orphaned historical
field from the unified struct, Doris now uses the safe empty partition display
instead of fabricating null or collapsed values; the regression covers two
distinct old buckets with exact pre-drop names.
##########
fe/pom.xml:
##########
@@ -360,7 +360,7 @@ under the License.
<!-- ATTN: avro version must be consistent with Iceberg version -->
<!-- Please modify iceberg.version and avro.version together,
you can find avro version info in iceberg mvn repository -->
- <iceberg.version>1.10.1</iceberg.version>
+ <iceberg.version>1.11.0</iceberg.version>
Review Comment:
Fixed in d10663e032. New FE output pins the known Iceberg 1.11 Schema
descriptor to the 1.10.1 UID, and new BE input accepts the corresponding legacy
descriptor. The compatibility bridge is intentionally limited to this verified
1.10.1/1.11 pair rather than introducing a general wire-protocol redesign:
StaticDataTask, StructType, NestedField, and StaticDataTask.Row retain their
UIDs, while unknown future layouts fail closed. Tests cover an actual 1.10.1
task fixture and planned current-version system-table tasks.
##########
fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java:
##########
@@ -1832,6 +1859,52 @@ public void
planScanPinnedToOlderSnapshotReadsOnlyThatSnapshotsFiles() {
Assertions.assertTrue(pinned.get(0).getPath().get().endsWith("f1.parquet"));
}
+ @Test
+ public void planScanHistoricalPredicateSurvivesColumnRename() {
+ assertHistoricalPredicatePlansAfterSchemaEvolution(false);
+ }
+
+ @Test
+ public void planScanHistoricalPredicateSurvivesColumnDrop() {
+ assertHistoricalPredicatePlansAfterSchemaEvolution(true);
+ }
+
+ private void assertHistoricalPredicatePlansAfterSchemaEvolution(boolean
dropColumn) {
+ Schema historicalSchema = new Schema(
+ Types.NestedField.optional(1, "x", Types.IntegerType.get()),
+ Types.NestedField.optional(2, "y", Types.IntegerType.get()),
+ Types.NestedField.optional(3, "part",
Types.IntegerType.get()));
+ Table table = createTable(
+ "historical_predicate_after_" + (dropColumn ? "drop" :
"rename"),
+ historicalSchema, PartitionSpec.unpartitioned(),
+ Collections.singletonMap(TableProperties.FORMAT_VERSION, "2"));
+ table.newFastAppend()
+ .appendFile(dataFile(table.spec(),
"s3://b/db/historical.parquet", 1024, null, null))
+ .commit();
+ long historicalSnapshotId = table.currentSnapshot().snapshotId();
+ int historicalSchemaId = table.currentSnapshot().schemaId();
+
+ if (dropColumn) {
+ table.updateSchema().deleteColumn("x").commit();
+ } else {
+ table.updateSchema().renameColumn("x", "renamed_x").commit();
+ }
+ table.newFastAppend()
Review Comment:
Fixed in d10663e032. The rename and drop regressions now plan the historical
predicate both immediately after the schema-only commit, while the snapshot IDs
are equal, and again after a later append advances the current snapshot.
--
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]
