github-actions[bot] commented on code in PR #66481:
URL: https://github.com/apache/doris/pull/66481#discussion_r3748171388
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiScanPlanProvider.java:
##########
@@ -636,13 +636,17 @@ static Optional<List<ConnectorScanRange>>
incrementalRanges(IncrementalRelation
}
/**
- * The Hudi table-config partition-field names (byte-faithful to legacy
{@code HudiScanNode:391-393}), the
- * source the incremental MOR path parses per-slice partition values
against — NOT the HMS-sourced
- * handle partition keys the snapshot path uses (the two coincide only for
hive-synced tables).
+ * The Hudi table-config partition-field names, canonicalized to Hudi's
lower-case Doris-column convention.
+ * This is the source the incremental MOR path parses per-slice partition
values against — NOT the
+ * HMS-sourced handle partition keys the snapshot path uses (the two
coincide only for hive-synced tables).
*/
private static List<String> partitionFieldNames(HoodieTableMetaClient
metaClient) {
Option<String[]> fields =
metaClient.getTableConfig().getPartitionFields();
- return fields.isPresent() ? Arrays.asList(fields.get()) :
Collections.emptyList();
+ return fields.isPresent()
+ ? Arrays.stream(fields.get())
+ .map(name -> name.toLowerCase(Locale.ROOT))
Review Comment:
Please apply this canonicalization to the COW `@incr` producer as well.
`incrementalRanges` uses `partitionFieldNames` only for MOR; the COW arm
returns `COWIncrementalRelation.collectSplits()`, which rereads raw
`getPartitionFields()` and stores (for example) `City` as the range-map key.
`HudiScanRange.populateRangeParams` then overwrites `columns_from_path_keys`
with that raw key while the tuple slot and scan-level key are `city`; BE's
exact lookup discards it, so the partition slot is not filled. Reuse the
canonicalized names there and add a COW-incremental test that reaches
`populateRangeParams`.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiScanPlanProvider.java:
##########
@@ -842,24 +846,29 @@ static Map<String, String> parsePartitionValues(
String[] fragments = partitionPath.split("/");
if (fragments.length != partKeyNames.size()) {
if (partKeyNames.size() == 1) {
- String prefix = partKeyNames.get(0) + "=";
- String value = partitionPath.startsWith(prefix)
- ? partitionPath.substring(prefix.length()) :
partitionPath;
+ String value = stripPartitionColumnPrefix(partitionPath,
partKeyNames.get(0));
values.put(partKeyNames.get(0), unescapePathName(value));
return values;
}
throw new DorisConnectorException(
"Failed to parse partition values of path: " +
partitionPath);
}
for (int i = 0; i < fragments.length; i++) {
- String prefix = partKeyNames.get(i) + "=";
- String raw = fragments[i].startsWith(prefix)
- ? fragments[i].substring(prefix.length()) : fragments[i];
+ String raw = stripPartitionColumnPrefix(fragments[i],
partKeyNames.get(i));
values.put(partKeyNames.get(i), unescapePathName(raw));
}
return values;
}
+ /** Strips an optional Hive-style {@code column=} prefix using Hive/Hudi's
case-insensitive identifiers. */
+ private static String stripPartitionColumnPrefix(String fragment, String
columnName) {
+ int separator = fragment.indexOf('=');
+ if (separator > 0 && fragment.substring(0,
separator).equalsIgnoreCase(columnName)) {
Review Comment:
This case-insensitive prefix heuristic is ambiguous for Hudi's default
positional layout. With `hive_style_partitioning=false` and URL encoding
disabled, column `city` can have the literal partition value `City=Beijing`,
producing the positional fragment `City=Beijing`. The old exact `city=` check
preserved that value; this branch now mistakes it for Hive-style syntax and
materializes/prunes on `Beijing`. Please make parsing layout-aware (or
otherwise carry unambiguous path provenance) and add a positional `=`
regression while retaining the mixed-case Hive-style case.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergSchemaUtils.java:
##########
@@ -275,26 +276,26 @@ static TSchema buildCurrentSchema(Schema schema,
List<String> requestedLowerName
* AUTHORITATIVE, so every field carries an explicit (possibly empty)
per-field mapping (see
* {@link #buildField}).
*/
- static TSchema buildCurrentSchema(Schema schema, List<String>
requestedLowerNames,
+ static TSchema buildCurrentSchema(Schema schema, List<String>
requestedNames,
Map<Integer, List<String>> nameMapping, boolean hasNameMapping,
boolean enableTimestampTz) {
- return buildCurrentSchema(schema, requestedLowerNames, nameMapping,
hasNameMapping, false,
+ return buildCurrentSchema(schema, requestedNames, nameMapping,
hasNameMapping, false,
enableTimestampTz);
}
/** Build the dictionary using the same Doris scalar mappings as the
connector's catalog columns. */
- static TSchema buildCurrentSchema(Schema schema, List<String>
requestedLowerNames,
+ static TSchema buildCurrentSchema(Schema schema, List<String>
requestedNames,
Map<Integer, List<String>> nameMapping, boolean hasNameMapping,
boolean enableVarbinary,
boolean enableTimestampTz) {
TSchema tSchema = new TSchema();
tSchema.setSchemaId(CURRENT_SCHEMA_ID);
TStructField root = new TStructField();
- if (requestedLowerNames == null || requestedLowerNames.isEmpty()) {
+ if (requestedNames == null || requestedNames.isEmpty()) {
for (Types.NestedField field : schema.columns()) {
- addField(root, buildField(field,
field.name().toLowerCase(Locale.ROOT), nameMapping,
+ addField(root, buildField(field, field.name(), nameMapping,
Review Comment:
Preserving the pinned schema's name here leaves the adjacent partition
carriers on a different schema generation. For a historical pin before a
case-only rename (`City` -> `CITY`), the tuple and dictionary use `City`, but
`getIdentityPartitionColumns` and `getIdentityPartitionInfoMap` still call
`table.schema().findColumnName(sourceId)` and emit latest-name `CITY`. FE
therefore does not classify `City` as the partition slot and BE ignores the
`CITY` range value; an imported identity-partitioned file without the physical
column is then materialized as NULL/default instead of its manifest value.
Please resolve these identity names against the same pinned `scanSchema` and
thread them through both scan-level and per-file keys, with a
historical-rename/path-fill test.
--
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]
