github-actions[bot] commented on code in PR #66345:
URL: https://github.com/apache/doris/pull/66345#discussion_r3705850040
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -2140,9 +2143,6 @@ public ConnectorTableHandle
applySnapshot(ConnectorSession session,
}
String ref = snapshot.getProperties().get(REF_PROPERTY);
long snapshotId = snapshot.getSnapshotId();
- if (snapshotId < 0 && ref == null) {
- return iceHandle;
- }
return iceHandle.withSnapshot(snapshotId, ref, snapshot.getSchemaId());
Review Comment:
[P1 follow-up] The RANGE case is now fenced, but resolved-empty non-RANGE
tables still fall through to live LIST metadata. For an identity, bucket,
truncate, or multi-field spec, `buildResolvedEmptyMvccPartitionView()` returns
UNPARTITIONED; `PluginDrivenMvccExternalTable.materializeLatest()` then calls
`listLatestPartitions()` with the original unpinned handle. A concurrent first
append can therefore inject S1 partition names/counts into an otherwise
empty-pinned snapshot, and a metadata-only spec evolution can derive RANGE
eligibility from a post-pin generation. Please short-circuit the generic LIST
fallback for resolved-empty handles (or retain the pinned table/spec
generation), and cover identity/bucket plus spec-evolution cases.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWritePlanProvider.java:
##########
@@ -265,24 +395,89 @@ public List<ConnectorWriteSortColumn>
getWriteSortColumns(ConnectorSession sessi
// unconditional setSortInfo inside the isSorted() branch even
when no identity column resolves.
return null;
}
- List<NestedField> columns = table.schema().columns();
+ Map<Integer, Integer> positionsByFieldId = new HashMap<>();
+ if (boundTargetColumns.isEmpty()) {
+ List<NestedField> currentColumns = table.schema().columns();
+ for (int i = 0; i < currentColumns.size(); i++) {
+ positionsByFieldId.put(currentColumns.get(i).fieldId(), i);
+ }
+ } else {
+ for (int i = 0; i < boundTargetColumns.size(); i++) {
+
positionsByFieldId.put(boundTargetColumns.get(i).getUniqueId(), i);
+ }
+ }
List<ConnectorWriteSortColumn> result = new ArrayList<>();
for (SortField sortField : sortOrder.fields()) {
if (!sortField.transform().isIdentity()) {
continue;
}
- for (int i = 0; i < columns.size(); i++) {
- if (columns.get(i).fieldId() == sortField.sourceId()) {
- result.add(new ConnectorWriteSortColumn(i,
- sortField.direction() == SortDirection.ASC,
- sortField.nullOrder() == NullOrder.NULLS_FIRST));
- break;
- }
+ Integer position = positionsByFieldId.get(sortField.sourceId());
+ if (position != null) {
+ // Resolve against the bound field id, never a newly refreshed
live ordinal; otherwise
+ // schema reorder can sort one output expression using another
column's ordering contract.
+ result.add(new ConnectorWriteSortColumn(position,
+ sortField.direction() == SortDirection.ASC,
+ sortField.nullOrder() == NullOrder.NULLS_FIRST));
}
}
return result;
}
+ @Override
+ public String getWriteMetadataIdentity(ConnectorSession session,
ConnectorTableHandle tableHandle) {
+ return writeMetadataIdentity(resolveTable(session,
(IcebergTableHandle) tableHandle));
+ }
+
+ static String writeMetadataIdentity(Table table) {
Review Comment:
[P1] Include the row-level mode contract in this fence. Admission reads
`write.{delete,update,merge}.mode` from the statement-frozen/cached read table,
but this identity omits all three, so a property-only transition to
`copy-on-write` passes the checks before and after `newTransaction()` and opens
Doris's explicitly unsupported RowDelta path on the current table. Please fence
these properties from bind through begin, or carry and revalidate the actual
analyzed operation after refresh (UPDATE is later encoded as MERGE, so checking
only the provider operation is insufficient), with property-only tests for all
three statements.
--
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]
