search

apache / iceberg-python

Apache PyIceberg
https://py.iceberg.apache.org/
Apache License 2.0
463 stars 168 forks source link

NameMapping flattens the names and causes `a.b` field to collide with child `b` field of field `a` #935

Closed sungwy closed 2 months ago

sungwy commented 3 months ago

Apache Iceberg version

None

Please describe the bug 🐞

According to the Iceberg documentation on Column Projection:

A name may contain . but this refers to a literal name, not a nested field. For example, a.b refers to a field named a.b, not child field b of field a.

The current implementation of NameMapping flattens the name by joining the parent child relationships with a .. This causes name collisions issues with fields that should not collide with each other.

For example, this flat map causes a.b field to collide with child b field of field a.

We should update _field_by_name() and find() methods of NameMapping to use a tree structure instead of a flat dict, and traverse the tree in order to retrieve MappedField of the provided name.

https://github.com/apache/iceberg-python/blob/e27cd9095503cfe9fa7e0a806ba25d42920c68c5/pyiceberg/table/name_mapping.py#L73-L82

Fokko commented 3 months ago

Thanks for tracking this @syun64, can I pick this one up? :)

sungwy commented 3 months ago

Yes of course!

Just a note that's hopefully helpful: while working on covering more cases for #921 , I realized this may require a bit more work than I originally thought. We currently rely on a flat name mapping in many places throughout the repository, including when we aggregate stats from the parquet files:

https://github.com/apache/iceberg-python/blob/0f2e19e49eb8b859cfcd7f89cd182461a61f15a7/pyiceberg/io/pyarrow.py#L2027-L2031

So I think we will need to build a tree representation of the Name to ID mapping for a given pyarrow schema as well.

https://github.com/apache/iceberg-python/blob/0f2e19e49eb8b859cfcd7f89cd182461a61f15a7/pyiceberg/io/pyarrow.py#L1934-L1936

sungwy commented 3 months ago

Hi @Fokko - I wanted to make note of this Rest Catalog Open API Spec PR, where the community may be weighing the pros and cons of flattening the nested field names in our APIs: