hubgeter commented on code in PR #69188:
URL: https://github.com/apache/doris/pull/69188#discussion_r4292806162
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPartitionUtils.java:
##########
@@ -138,6 +139,46 @@ static List<String> getIdentityPartitionColumns(Table
table) {
return new ArrayList<>(partitionColumns);
}
+ /**
+ * Rewrites a static-partition spec ({@code INSERT [OVERWRITE] ...
PARTITION (col = val)}) so every key is the
+ * matching partition FIELD name, the identifier both downstream consumers
address: the BE sink matches
+ * {@code static_partition_values} against {@code PartitionField#name()},
and the commit row filter resolves
+ * the field to its source column.
+ *
+ * <p>The engine's PARTITION clause names the identity partition's SOURCE
column instead — the identifier Spark
+ * resolves for a static partition clause ({@code
Analyzer.partitionColumnNames} takes the identity transform's
+ * field reference) and the one {@code SHOW CREATE TABLE} renders — so the
two spellings must be bridged once,
+ * here, before either consumer sees the spec. A key that matches no field
of the current spec is kept verbatim
+ * and left to the callers' fail-loud guards.</p>
+ */
+ static Map<String, String> toPartitionFieldKeys(Map<String, String>
staticPartitions, Schema schema,
+ PartitionSpec spec) {
+ if (staticPartitions == null || staticPartitions.isEmpty() || spec ==
null) {
+ return staticPartitions;
+ }
+ Map<String, String> canonical = new HashMap<>(staticPartitions.size());
+ for (Map.Entry<String, String> entry : staticPartitions.entrySet()) {
+ canonical.put(toPartitionFieldName(schema, spec, entry.getKey()),
entry.getValue());
+ }
+ return canonical;
+ }
+
+ /**
+ * The partition field a static-partition name addresses: its SOURCE
column name, or the field name itself.
+ * Iceberg refuses to name a partition field after a column other than its
own source
+ * ({@code PartitionSpec.Builder#checkAndAddPartitionName}), so the two
lookups cannot disagree.
+ */
+ private static String toPartitionFieldName(Schema schema, PartitionSpec
spec, String name) {
+ for (PartitionField field : spec.fields()) {
+ NestedField source = schema.findField(field.sourceId());
+ if ((source != null && source.name().equalsIgnoreCase(name))
Review Comment:
Fixed in 1ff3d44d061. `toPartitionFieldName` now resolves the identity
partition over the top-level SOURCE column first and only falls back to a
field-name match, so the alias cannot capture the key. Your example is pinned
by the new tests
`validateStaticPartitionColumnsPrefersTheIdentitySourceOverACaseFoldedAlias`
and `staticPartitionKeysPreferTheIdentitySourceOverACaseFoldedFieldAlias` (the
spec builder does accept `identity(other, "ID")` next to `identity(id,
"id_identity")`, as you noted).
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -1911,23 +1919,60 @@ public void
validateStaticPartitionColumns(ConnectorSession session, ConnectorTa
throw new DorisConnectorException(String.format(
"Table %s is not partitioned, cannot use static partition
syntax", tableName));
}
- Map<String, PartitionField> partitionFieldMap = new HashMap<>();
+ for (String colName : staticPartitionColumnNames) {
+ validateStaticPartitionColumn(table.schema(), spec, colName,
tableName);
+ }
+ }
+
+ /** The single-name rule of {@link #validateStaticPartitionColumns}. */
+ private static void validateStaticPartitionColumn(Schema schema,
PartitionSpec spec, String colName,
+ String tableName) {
for (PartitionField field : spec.fields()) {
- partitionFieldMap.put(field.name(), field);
+ Types.NestedField source = schema.findField(field.sourceId());
+ if (source == null || !source.name().equalsIgnoreCase(colName)) {
Review Comment:
Fixed in 1ff3d44d061. Static targets must now resolve to a TOP-LEVEL source
column (`IcebergPartitionUtils#topLevelSourceColumnName`); `NestedField#name()`
is no longer treated as a column name, so `PARTITION(id=2)` over
`identity(payload.id)` reads as unknown and the field name itself reports "its
source is a nested field ... the engine can only materialize the literal into a
top-level column". Covered by
`validateStaticPartitionColumnsRejectsNestedPartitionSources`.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -1911,23 +1919,60 @@ public void
validateStaticPartitionColumns(ConnectorSession session, ConnectorTa
throw new DorisConnectorException(String.format(
"Table %s is not partitioned, cannot use static partition
syntax", tableName));
}
- Map<String, PartitionField> partitionFieldMap = new HashMap<>();
+ for (String colName : staticPartitionColumnNames) {
+ validateStaticPartitionColumn(table.schema(), spec, colName,
tableName);
+ }
+ }
+
+ /** The single-name rule of {@link #validateStaticPartitionColumns}. */
+ private static void validateStaticPartitionColumn(Schema schema,
PartitionSpec spec, String colName,
+ String tableName) {
for (PartitionField field : spec.fields()) {
- partitionFieldMap.put(field.name(), field);
+ Types.NestedField source = schema.findField(field.sourceId());
+ if (source == null || !source.name().equalsIgnoreCase(colName)) {
+ continue;
+ }
+ // The SOURCE column of a partition field: a legal static target
only for an identity transform,
+ // because a transformed partition value is not the column value
and cannot be written through it.
+ if (field.transform().isIdentity()) {
+ return;
+ }
+ throw new DorisConnectorException(String.format(
Review Comment:
Fixed in 1ff3d44d061. The identity partition over the source is searched
across ALL fields before any non-identity error is raised, and the key rewrite
shares that lookup, so a spec that declares `bucket(8, id)` before
`identity(id)` keeps its static writes. Pinned by
`validateStaticPartitionColumnsFindsIdentityAfterANonIdentityFieldOnTheSameSource`
and `staticPartitionKeysFindTheIdentityFieldRegardlessOfFieldOrder`.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -1911,23 +1919,60 @@ public void
validateStaticPartitionColumns(ConnectorSession session, ConnectorTa
throw new DorisConnectorException(String.format(
"Table %s is not partitioned, cannot use static partition
syntax", tableName));
}
- Map<String, PartitionField> partitionFieldMap = new HashMap<>();
+ for (String colName : staticPartitionColumnNames) {
+ validateStaticPartitionColumn(table.schema(), spec, colName,
tableName);
Review Comment:
Fixed in 1ff3d44d061. Validation now uses the statement's pinned writer
schema (`IcebergStatementScope.activeWriteSchema`) - the same copy binding,
planning and the BE sink consume, registered by `pinConnectorWriteSchema`
before analysis - and only falls back to the loaded table when no pin exists.
`validateStaticPartitionColumnsUsesThePinnedBranchWriterSchema` pins your
branch case: the pinned (`@branch(old)`) statement accepts `id`, while
validating against the renamed current schema would reject it and advise `key`.
--
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]
