Derive name mapping from table schema when iceberg.id is missing #24795
Conversation
weijiii
force-pushed
the
iceberg-orc-infer-name-mapping
branch
from
ef0d3db to
d5f34c2
Compare
| @@ -87,7 +87,7 @@ public static Metrics fileMetrics(TrinoInputFile file, MetricsConfig metricsConf | |||
| Footer footer = reader.get().getFooter(); | |||
|
|
|||
| // use name mapping to compute missing Iceberg field IDs | |||
| Optional<NameMapping> nameMapping = Optional.of(MappingUtil.create(schema)); | |||
| NameMapping nameMapping = MappingUtil.create(schema); | |||
There was a problem hiding this comment.
So if I'm understanding correctly, we have two different codepaths that eventually land at fileColumnsByIcebergId. One from IcebergPageSourceProvider, and one here in OrcMetrics. Prior to this PR, the latter would use MappingUtil to derive the schema, but the former would only use the table properties. And in this PR, we are standardizing so that both codepaths use MappingUtil. Do I have that right?
I'm curious, doesn't this OrcMetrics codepath also need to handle the case where the name mapping is set as a JSON in the table properties? Like why aren't these codepaths completely unified in how they derive a NameMapping from the combination of ORC files / table metadata / table schema?
There was a problem hiding this comment.
IIUC OrcMetrics::fileMetrics is only used when migrating table to Iceberg, therefore the Iceberg table metadata is not yet created as well as the table properties. Column Ids are expected to be missing in the ORC files so we are using the name mapping derived from table schema to fill the gap.
There was a problem hiding this comment.
Makes sense, thanks for clarifying!
weijiii
force-pushed
the
iceberg-orc-infer-name-mapping
branch
from
d5f34c2 to
d90decc
Compare
|
Thanks @weijiii for this PR and for adding the additional context from Iceberg. +1 to this change after adding the tests. |
Description
iceberg.idis present for any file columns including nested ones from the ORC data files, the name mapping from table metadata would be ignored. If the ORC data file is malformed where noiceberg.idis present, querying the table would incorrectly yieldNULLfor all columns from the data files. Name mapping can be derived from the table schema. Ificeberg.idis present in the file columns, name mapping would not affect anything. Ificeberg.idis missing in all file columns, the name mapping from table properties should be used to set the missingiceberg.idattributes. If name mapping is not configured in the table properties, one that is derived from table schema would be used as alternative.Additional context and related issues
Release notes
(v) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: