[GitHub] [iceberg] pvary commented on a diff in pull request #6407: Flink: use SerializableTable for source

[GitBox <gi...@apache.org>]

pvary commented on code in PR #6407:
URL: https://github.com/apache/iceberg/pull/6407#discussion_r1046703137


##########
flink/v1.16/flink/src/main/java/org/apache/iceberg/flink/source/IcebergSource.java:
##########
@@ -357,13 +358,10 @@ public IcebergSource<T> build() {
       if (readerFunction == null) {
         RowDataReaderFunction rowDataReaderFunction =
             new RowDataReaderFunction(
+                (SerializableTable) SerializableTable.copyOf(table),
                 flinkConfig,
-                table.schema(),
                 context.project(),
-                context.nameMapping(),

Review Comment:
   I remember that when we were implementing historical queries in Hive (AS OF VERSION, AS OF TIMESTAMP) then we found that the namemapping is bound to the table, and not bound to the version. So if some specific schema evolution happens (migrate a table, rename a column, add back an old column - or something like this - sadly I do not remember the specifics) then we were not able to restore the original mapping from the current one, and we do not able to query the data.
   Being able to provide a namemapping could help here.
   
   Arguably, this is a rare case, and the correct fix would be a spec change, still I thought it worth to mention.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org