github-actions[bot] commented on code in PR #65851:
URL: https://github.com/apache/doris/pull/65851#discussion_r3669378588
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -530,30 +594,646 @@ void enableCurrentIcebergScanSemantics() {
params.setIcebergScanSemanticsVersion(ICEBERG_SCAN_SEMANTICS_VERSION);
}
+ /**
+ * Build the schema metadata carrier used by both scanners and
equality-delete readers.
+ *
+ * <p>Batch-mode delete files are planned asynchronously after scan
parameters are sent to BE.
+ * The authenticated manifest preflight therefore supplies the live
equality field IDs before
+ * the schema carrier is serialized. Only historical fields referenced by
those delete files are
+ * added, so an unrelated dropped type cannot make an otherwise supported
scan fail.
+ */
+ @VisibleForTesting
+ List<NestedField> getSchemaFieldsForScan(
+ Schema scanSchema, Set<Integer> equalityDeleteFieldIds) throws
UserException {
+ List<NestedField> fields = new ArrayList<>(scanSchema.columns());
+ if (isSystemTable || equalityDeleteFieldIds.isEmpty()) {
+ return fields;
+ }
+
+ Set<Integer> missingFieldIds = new HashSet<>(equalityDeleteFieldIds);
+
missingFieldIds.removeAll(TypeUtil.indexById(scanSchema.asStruct()).keySet());
Review Comment:
[P1] Preserve the ancestor tree for nested equality-delete keys
`TypeUtil.indexById` treats a current nested key as present here, but
`fields` contains only the schema roots, while both V1 `_find_schema_field` and
V2 `_find_table_column_by_field_id` search only `root_field.fields`. Thus a
valid key such as `optional struct s<required int k>` reaches BE with `k` below
`s`, where both scanner paths fail to resolve it (and their delete-file readers
reject the containing complex field). For a dropped nested key,
`addHistoricalEqualityFields` instead appends just the leaf as a synthetic
root, losing both its physical path and the optional parent's null semantics.
Iceberg permits primitive equality keys nested in structs, so these scans are
rejected even though the table is valid. Please carry the ancestor chain and
resolve equality IDs recursively, with forced V1/V2 Parquet/ORC tests for
current and dropped nested keys.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -505,22 +525,66 @@ private List<String> getOrderedPathPartitionKeys() {
}
public void createScanRangeLocations() throws UserException {
- super.createScanRangeLocations();
+ Schema scanSchema = getQuerySchema();
+ Optional<Map<Integer, List<String>>> nameMapping =
extractNameMapping();
+ Set<Integer> equalityDeleteFieldIds = Collections.emptySet();
+ if (!isSystemTable) {
+ checkNameMappingBackendCompatibility(
+ scanSchema, nameMapping, backendPolicy.getBackends());
Review Comment:
[P2] Scope the name-collision fence to resolved paths
`scanSchema` is the complete query schema, and this helper recursively scans
every descendant without consulting `desc.getSlots()` or nested access paths.
During a smooth upgrade, a query such as `SELECT id` is therefore rejected
merely because an unrelated, unprojected `root.payload` subtree has a
rename/re-add alias collision, even though neither reader resolves that
subtree. This turns the necessary fence for a projected colliding path into an
availability regression for every query on the table. Please make the collision
check projection/access-path aware while still including hidden applicable
equality keys, and add an unrelated-projection test alongside the existing
positive collision case.
##########
be/src/format/transformer/vorc_transformer.cpp:
##########
@@ -318,6 +320,30 @@ std::unique_ptr<orc::Type>
VOrcTransformer::_build_orc_type(
}
}
if (nested_field != nullptr) {
+ const PrimitiveType primitive_type = data_type->get_primitive_type();
+ const auto use_iceberg_binary_type = [&](std::string_view binary_type)
{
+ DORIS_CHECK(is_string_type(primitive_type) ||
is_varbinary(primitive_type) ||
+ primitive_type == TYPE_BINARY);
+ type = orc::createPrimitiveType(orc::BINARY);
+ type->setAttribute(ICEBERG_BINARY_TYPE, std::string(binary_type));
+ };
+ switch (nested_field->field_type()->type_id()) {
+ case iceberg::TypeID::UUID:
+ use_iceberg_binary_type("UUID");
+ break;
Review Comment:
[P1] Convert legacy UUID text before writing ORC binary
With `enable.mapping.varbinary=false`, an Iceberg UUID is still carried to
BE as a Doris `STRING`. This change correctly advertises the ORC field as
`binary`/`iceberg.binary-type=UUID`, but `write()` continues to use the generic
string SerDe, which copies a normal 36-character UUID literal and its length
unchanged. That produces 36 ASCII bytes where Iceberg requires the UUID's
16-byte representation; the Parquet writer already has an explicit
32/36-character-to-16-byte conversion for this same carrier. Please add
schema-aware recursive conversion/validation before populating the ORC batch
(and validate `FIXED` lengths at the same boundary), with an explicit-value
legacy-mapping interoperability test rather than only the omitted default,
which is already raw bytes.
--
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]
