github-actions[bot] commented on code in PR #66345:
URL: https://github.com/apache/doris/pull/66345#discussion_r3703657219
##########
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] Keep partition freshness on the resolved-empty generation. With a
positive table-cache TTL but `meta.cache.iceberg.partition_view.enable=false`
(or zero TTL/capacity), a REST catalog using vended credentials can retain the
pre-append `-1` in `latestSnapshotCache` while its deliberately uncached table
load sees a newly committed S1. This method records the resolved-empty pin, but
`materializeLatest()` then calls `getMvccPartitionView()`, whose negative-id
path treats live S1 as current. The same MTMV snapshot therefore scans no rows
because of the new empty fence but advertises S1 partitions/freshness, so a
refresh can persist S1 as synchronized with its rows missing. Please make the
partition view honor `isResolvedEmptySnapshot()` (or retain the pinned table
generation) and add a first-append MTMV test with the derived view cache
disabled.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWritePlanProvider.java:
##########
@@ -230,6 +235,124 @@ public ConnectorSinkPlan planWrite(ConnectorSession
session, ConnectorWriteHandl
}
}
+ private void validateBoundWriteColumns(Table table, ConnectorWriteHandle
handle,
+ WriteOperation writeOperation) {
+ List<ConnectorColumn> boundTargetColumns =
handle.getBoundTargetColumns();
+ if (!boundTargetColumns.isEmpty()
+ && (writeOperation == WriteOperation.REWRITE
+ || writeOperation == WriteOperation.UPDATE
+ || writeOperation == WriteOperation.MERGE)) {
+ long boundLineageColumns = boundTargetColumns.stream()
+ .filter(ConnectorColumn::isReservedPassthrough)
+ .count();
+ long currentLineageColumns =
IcebergWriterHelper.getFormatVersion(table) >= 3 ? 2 : 0;
+ // Format version changes the physical sink arity without changing
table.schema(); the reserved
+ // columns are the bind-time witness that the output and v3
schema-json belong to one generation.
+ if (boundLineageColumns != currentLineageColumns) {
+ throw new DorisConnectorException(
+ "Iceberg write metadata changed after the write was
bound; retry the statement");
+ }
+ }
+ // V3 row-lineage columns are engine-generated metadata, not fields in
table.schema(). Excluding
+ // their neutral marker keeps the comparison on the complete user
schema for INSERT and MERGE.
+ List<ConnectorColumn> boundColumns = boundTargetColumns.stream()
+ .filter(column -> !column.isReservedPassthrough())
+ .collect(Collectors.toList());
+ if (writeOperation == WriteOperation.DELETE || boundColumns.isEmpty())
{
+ return;
+ }
+ List<NestedField> currentColumns = table.schema().columns();
+ boolean hasSyntheticRowId = boundColumns.size() ==
currentColumns.size() + 1
Review Comment:
[P1] Do not accept the synthetic row locator for REWRITE.
`ConnectorRewriteGroupTask` clones the caller's session variables, so with
`show_hidden_columns=true` both the source scan and `getWriteSchemaSnapshot()`
append `__DORIS_ICEBERG_ROWID_COL__`; rewrite binding retains invisible columns
and this exemption lets that extra output through. `buildRewriteSink()`
advertises only `table.schema()` on v2, or that schema plus the two real v3
lineage fields, and `VIcebergTableWriter::open()` requires the output count to
equal that schema count. Every group therefore fails with one surplus column.
Please exclude the request-scoped locator from REWRITE while retaining the v3
`reservedPassthrough` fields, and cover v2/v3 rewrites with
`show_hidden_columns=true`.
--
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]
