github-actions[bot] commented on code in PR #66538:
URL: https://github.com/apache/doris/pull/66538#discussion_r3874256681
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -355,6 +1099,13 @@ Status
IcebergTableReader::materialize_virtual_columns(Block* table_block) {
break;
}
}
+ for (size_t column_idx = 0; column_idx < mappings.size(); ++column_idx) {
+ if (!requires_required_field_validation(mappings[column_idx])) {
Review Comment:
[P2] Keep footer aggregates behind Iceberg requiredness validation
This new check runs only after normal table materialization, but the
inherited `COUNT(column)`/MINMAX eligibility never inspects
`reject_null_value`, which is where this reader records requiredness because FE
external types remain nullable. An old nullable file mapped to a current
required field can therefore return a footer aggregate instead of the `Required
Iceberg field ... contains NULL` error produced by a normal scan. The generic
nullable-to-required COUNT test uses a nonnullable Doris `table_type`, so it
misses this Iceberg representation. Disable aggregate pushdown when the
selected mapping or any child requires this validation, and add an Iceberg
evolution case.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalUtil.java:
##########
@@ -214,7 +271,7 @@ private static TField getExternalSchema(Type columnType,
Column dorisColumn,
Column subColumn = subNameToSubColumn.get(subField.getName());
fieldPtr.setFieldPtr(getExternalSchema(
subField.getType(), subColumn, nameMapping,
hasNameMapping,
- base64InitialDefaults));
+ initialDefaults, binaryLikeFieldIds,
requiredFieldIds));
Review Comment:
[P1] Preserve duplicate-name field identities in the schema carrier
`getSchemaFieldsForScan()` intentionally merges a dropped nested equality
key by ID into the current struct. After that name is re-added with a new ID,
the synthetic struct has two same-name children with different IDs. This map
keeps only the appended historical `Column`, so both `StructField` entries
recurse with its metadata and the current ID disappears from the query-wide
Thrift carrier. The BE's new field-ID/path checks cannot recover that identity
and can bind the delete key to the replacement value. Serialize these children
positionally/by field identity and cover a same-name re-add.
--
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]
