Gabriel39 commented on code in PR #65851:
URL: https://github.com/apache/doris/pull/65851#discussion_r3697533489
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -427,7 +427,7 @@ private static Plan normalizePlanWithoutLock(LogicalPlan
plan, TableIf table,
}
for (int i = 0; i < values.size(); i++) {
Column sameNameColumn = null;
- for (Column column : table.getBaseSchema(true)) {
+ for (Column column : connectorWriteSchema(table,
true)) {
Review Comment:
[P1] Preserve connector write defaults for explicit DEFAULT in VALUES
This loop now selects the request-scoped connector column, whose persisted
`defaultValue` is intentionally null while `getDefaultValueSql()` carries the
Iceberg write default. The downstream `generateDefaultExpression()` still
branches only on `getDefaultValue() == null`, so it emits a NULL literal for a
nullable field instead of parsing the connector default. The current External
Regression confirms this: `INSERT ... (default_int) VALUES (DEFAULT)` writes
NULL in both Parquet and ORC, while omitting the same column writes 35.
Please make explicit DEFAULT consume `getDefaultValueSql()` (or route it
through the connector default helper) and retain the Parquet/ORC regression as
a gate.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -1728,57 +1754,399 @@ private static List<String>
requestedLowerNames(List<ConnectorColumnHandle> colu
return names;
}
+ private static boolean mayHaveEqualityDeletes(Snapshot snapshot) {
+ if (snapshot == null) {
+ return false;
+ }
+ String equalityDeletes =
snapshot.summary().get(TOTAL_EQUALITY_DELETES);
+ // A missing counter is unknown (replace/cherry-pick snapshots can
omit it), so retain the bounded
+ // schema-history carrier. The exact task/delete binding is still
decided by Iceberg during split planning.
+ return equalityDeletes == null || !equalityDeletes.equals("0");
+ }
+
/**
- * Ensure the schema-evolution dict carries the table's equality-delete
KEY columns even when the query
- * does not project them (#65502). Equality-delete keys are hidden scan
dependencies: BE resolves a key
- * that is missing from an OLD data file by looking its field id up in
this dict to get the column type +
- * iceberg initial default; without the entry BE materializes the key as
NULL and mis-applies the delete.
- * The keys are the table's declared identifier fields (what
equality-delete writers key on) -> a few
- * columns, DCHECK-safe superset (BE looks up only its own scan slots; the
pin/top-N branches already ship
- * the full schema). If the table declares NO identifier yet the scan
carries equality deletes (whose
- * equality_ids we cannot cheaply enumerate here), fall back to the full
schema. Non-identifier /
- * append-only / position-delete-only tables are unaffected (the pruned
dict is returned verbatim).
+ * Build a schema carrier that can resolve any equality key reachable
before the selected schema without
+ * enumerating data files, manifests, or byte-split tasks. Its retained
state is bounded by table schema
+ * history rather than scan cardinality. At execution time BE looks fields
up by the exact IDs on each
+ * {@link FileScanTask#deletes()}; unrelated carrier fields never
participate in delete matching.
+ *
+ * <p>The selected snapshot lineage wins when a field was renamed. The
metadata schema list, in its actual
+ * chronology up to the selected schema (schema IDs are identifiers, not a
sequence), fills schema-only
+ * changes and expired ancestors. Current fields remain first, so a
dropped/re-added name still resolves the
+ * projected current field by name while a historical equality key
resolves by its stable field ID.</p>
*/
- private List<String> withEqualityDeleteKeyColumns(Table table,
List<String> requested) {
- if (requested.isEmpty()) {
- // An empty requested list already makes buildCurrentSchema fall
back to the FULL schema (every
- // top-level column) — a superset that covers every
equality-delete key — so there is nothing to
- // force-include. Returning early also preserves that all-columns
fallback (a non-empty identifier
- // set would otherwise prune it to identifier-only) and skips the
table.schema()/currentSnapshot()
- // probe when it cannot change the result.
- return requested;
- }
- Schema schema = table.schema();
- Set<Integer> identifierFieldIds = schema.identifierFieldIds();
- if (identifierFieldIds.isEmpty()) {
- return hasEqualityDeletes(table) ? Collections.emptyList() :
requested;
- }
- Set<String> present = new HashSet<>();
- for (String name : requested) {
- present.add(name.toLowerCase(Locale.ROOT));
- }
- List<String> result = new ArrayList<>(requested);
- for (int fieldId : identifierFieldIds) {
- Types.NestedField field = schema.findField(fieldId);
+ private static Schema schemaForPotentialEqualityDeletes(
+ Table table, TableScan scan, Schema scanSchema) {
+ List<Schema> history = potentialEqualityDeleteSchemaHistory(table,
scan, scanSchema);
+ Set<Integer> missing = new HashSet<>();
+ for (Schema schema : history) {
+ for (NestedField field :
TypeUtil.indexById(schema.asStruct()).values()) {
+ if (field.type().isPrimitiveType()) {
+ missing.add(field.fieldId());
+ }
+ }
+ }
+ missing.removeAll(TypeUtil.indexById(scanSchema.asStruct()).keySet());
+ if (missing.isEmpty()) {
+ return scanSchema;
+ }
+
+ List<NestedField> fields = new ArrayList<>(scanSchema.columns());
+ for (Schema historicalSchema : history) {
+ addHistoricalEqualityFields(fields, missing, historicalSchema);
+ }
+ if (!missing.isEmpty()) {
+ throw new IllegalStateException(
+ "Iceberg historical primitive fields are absent from
schema history: " + missing);
+ }
+ return new Schema(scanSchema.schemaId(), fields);
Review Comment:
[P1] Avoid constructing an invalid carrier after drop and same-name re-add
This gathers every historical primitive field ID, not only IDs used by
applicable equality deletes. If `x` with field ID 1 is dropped and a new `x`
with field ID 2 is added, `fields` starts with current `x` (ID 2), then
`mergeHistoricalEqualityFields()` appends historical `x` (ID 1). The `new
Schema(...)` call rejects that valid evolution with `Invalid schema: multiple
fields for name x`. Any scan for which `mayHaveEqualityDeletes` is true can
therefore fail even when `x` was never an equality key.
Please encode historical identities without constructing a duplicate-name
Iceberg Schema, or narrow the carrier to the exact applicable equality IDs
through a bounded/lazy mechanism. Add top-level and nested
drop/re-add-same-name coverage with an unrelated retained equality delete and
with a missing snapshot summary.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExternalRowLevelMergePlanBuilder.java:
##########
@@ -275,18 +279,12 @@ private List<Expression>
buildInsertProjection(MergeNotMatchedClause clause,
}
}
if (value == null) {
- if (column.getDefaultValueSql() != null) {
- Expression unboundDefaultValue = new NereidsParser()
- .parseExpression(column.getDefaultValueSql());
- if (unboundDefaultValue instanceof UnboundAlias) {
- unboundDefaultValue = unboundDefaultValue.child(0);
- }
- value = unboundDefaultValue;
- } else if (column.isAllowNull()) {
- value = new
NullLiteral(DataType.fromCatalogType(column.getType()));
- } else {
- throw new AnalysisException("Column has no default value,
column=" + column.getName());
- }
+ value = ConnectorWriteSchemaUtils.resolveDefault(column);
Review Comment:
[P1] Preserve NULL semantics for an omitted nullable MERGE column
When a `WHEN NOT MATCHED THEN INSERT` clause has an explicit target-column
list and omits a nullable column with no write default, `value` is null here.
`resolveDefault()` rejects every column whose `getDefaultValueSql()` is null,
so a valid omission now fails with `Column has no default value`. The previous
code produced a typed `NullLiteral` for exactly this case.
Please distinguish omission from an explicit DEFAULT: use the write default
when present, synthesize typed NULL when the omitted column is nullable, and
reject only a required column without a default. Add a MERGE regression that
omits one nullable no-default field.
--
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]
