github-actions[bot] commented on code in PR #68962:
URL: https://github.com/apache/doris/pull/68962#discussion_r4256949898
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -199,6 +199,50 @@ Status IcebergTableReader::validate_variant_file_mappings(
file_format_name(format));
}
+Status IcebergTableReader::annotate_file_schema(
+ std::vector<format::ColumnDefinition>* file_schema) {
+ if (file_schema == nullptr || file_schema->empty()) {
+ return Status::OK();
+ }
+ // Iceberg's positional fallback is a Parquet-reader-only compatibility
path. Its ORC/Avro
+ // readers instead derive a name mapping from the expected schema for
id-less files, which is
+ // exactly the current-name resurrection that v2 scan semantics
deliberately rejects (#65784),
+ // so ORC keeps its existing behavior and stays out of scope here.
+ if (_format != FileFormat::PARQUET) {
+ return Status::OK();
+ }
+ // Iceberg's `hasIds` contract is existential: a single real field id
keeps the file on the id
+ // path and, exactly as in Iceberg, skips both the name mapping and the
positional fallback.
+ if (schema_has_any_field_id(*file_schema)) {
+ return Status::OK();
+ }
+ // An old-FE plan keeps the legacy name fallback (rolling upgrade), and an
explicit table-level
+ // name mapping stays authoritative even when it does not cover this file.
+ if (!supports_iceberg_scan_semantics_v2(_scan_params) ||
+ _scan_has_any_authoritative_name_mapping()) {
+ return Status::OK();
+ }
+ // ParquetSchemaUtil.addFallbackIds: ordinals start at 1, are counted over
the physical
+ // top-level fields in file order, and are not propagated to nested
fields. Reader-synthesized
+ // carriers (row number / global row id) are not file fields and must not
consume an ordinal.
+ int32_t ordinal = 0;
+ for (auto& field : *file_schema) {
+ if (field.column_type != format::ColumnType::DATA_COLUMN) {
+ continue;
+ }
+ ++ordinal;
+ // ID-less Parquet fields carry their physical name as a STRING
identifier
+ // (ParquetReader::_fill_column_definition), so only a real integer
field id may skip
+ // stamping; overwriting the string identifier is what lets
FieldIdMatcher resolve the file.
+ if (field.has_identifier_field_id()) {
+ continue;
+ }
+ field.identifier = Field::create_field<TYPE_INT>(ordinal);
Review Comment:
[P1] Isolate condition cache when positional binding changes
With condition cache enabled, take an ID-less plain Parquet file with
physical old_b=[1,1] at ordinal 1 and old_a=[0,2] at ordinal 2, and table a (ID
1) authoritatively mapped to old_a. SELECT a FROM t WHERE a=1 keeps the row
group by min/max, finds no survivors and caches a false granule at EOF.
Removing schema.name-mapping.default and refreshing metadata now binds a to
old_b here, so the same query should return two rows. However,
TableReader::_init_reader_condition_cache keys by the unchanged
table-predicate/session digest and file path/mtime/size/range; its
localized_digest is only a cacheability check. The old false bitmap is reused
and Parquet skips both matching rows before the residual predicate can run.
Include the resolved physical binding in the cache identity, or disable
condition cache for positional fallback until that identity is represented, and
cover warm versus cold scans across the mapping change.
--
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]
