Spark: Follow name mapping while importing ORC tables

[aokolnychyi]

If we add support for name mapping in ORC, we should follow it while importing tables through SparkTableUtil.

[aokolnychyi]

FYI, @rdsr @edgarRd

[edgarRd]

Thanks for creating this, @aokolnychyi - I think I had a similar issue in #1206. The name mapping support for ORC has been merged #1208, so I'm planning on working on this issue this week.

[edgarRd]

While working on this I noticed that we only compute the NameMapping when it is set in the table properties in: https://github.com/apache/iceberg/blob/master/spark/src/main/java/org/apache/iceberg/spark/SparkTableUtil.java#L577 otherwise we set as null. As per https://github.com/apache/iceberg/pull/1167/files/eadac8d5da4f7911e4734f101c8756ee12f2aea9#diff-ea7008dc637345e7a36b5e1b5a579e13 - this will break since there's no IDs in the ORC data, so there's a couple of questions:

1. Should we have a default NameMapping built with the target Iceberg table schema?
2. Should we change https://github.com/apache/iceberg/pull/1167/files/eadac8d5da4f7911e4734f101c8756ee12f2aea9#diff-ea7008dc637345e7a36b5e1b5a579e13 to not fail if there's no IDs found?

What do you think, @rdsr @aokolnychyi ?

[aokolnychyi]

Internally, we started assigning a default name mapping when migrating existing tables to Iceberg. I am not sure a name mapping makes sense when we create a fresh Iceberg table. My assumption was that we would need to assign a name mapping in the import/snapshot commands.

Also, we ignore Parquet metrics for columns we could not determine ids for in SparkTableUtil. Should we do the same for ORC?

[edgarRd]

Internally, we started assigning a default name mapping when migrating existing tables to Iceberg. I am not sure a name mapping makes sense when we create a fresh Iceberg table.

Yeah, I don't think a name mapping makes sense for Iceberg tables that won't import any data. For migrated tables, a default name mapping equivalent to the columns of the target table should be fine. That's what I did, but programmatically in https://github.com/apache/iceberg/pull/1399/files#diff-531684170cfef0f1493ced85a0c8dbfcR544

My assumption was that we would need to assign a name mapping in the import/snapshot commands.

I think the common case would be that when importing a table, users would want to import all the columns, so the default name mapping should be the same of the target table schema. This is consistent with the default you mentioned and my changed mentioned above if I understand correctly.

Also, we ignore Parquet metrics for columns we could not determine ids for in SparkTableUtil. Should we do the same for ORC?

Yes, this behavior should be covered by my change in https://github.com/apache/iceberg/pull/1399/files#diff-ea7008dc637345e7a36b5e1b5a579e13L264 and the ORC metrics implementation already skips columns that do not have an Iceberg field mapped by the id: https://github.com/apache/iceberg/pull/1399/files#diff-ea7008dc637345e7a36b5e1b5a579e13L116