fm100 commented on PR #2791: URL: https://github.com/apache/iceberg-rust/pull/2791#issuecomment-5107854824
> Thanks for opening this PR, @fm100. > > I'm not super fond of this change - I am concerned that it is introducing complexity and uncertainty into the spec module of iceberg-rust, for a specific edge case. With this change, we add (albeit relatively small) edge cases and blank out the schema _only_ for unpartitioned tables where DuckDB wrote the manifests erroneously before it was fixed (release TBD). > > I'd rather entertain discussions on how either DuckDB or the Iceberg project can facilitate operations that recover the table in this scenario. For instance, why does the DuckDB project not own providing a procedure to rewrite these manifests tolerating bad schema bytes in the manifests' metadata? I'm aware that I'm pushing work back on to the DuckDB community, although I do question if it is a better fit where they can own the bug in terms of both the report, fix, and the mitigation for manifests written with this malformed metadata. They can also decide when to no longer maintain code required to address the bug. > > What do you think, @fm100 @kevinjqliu? Happy to be pushed back on. I know there's other circumstances we'd rather tolerate smaller inconsistencies (such as the Decimal(S, P) serialization), so there's a balance to be found here. Thank you very much for the opinion. I think DuckDB might be able to make a fix on their side, particularly to rewrite metadata that is broken, but I still prefer the reader side tolerance regardless of that. As iceberg is an open table format, users have a freedom of choice of the engine. It is highly likely possible that two or more users using different version of duckdb from their terminal, and I don't think it's ideal to expect them to be aware of this issue and rewrite when things broken. The owner of the table won't see an issue because they read and write with duckdb, but the problem will happen for someone else using DataFusion backed by iceberg-rust. From my perspective, the top four popular query engines are Spark, DuckDB, polars, and DataFusion, and the Spark and DuckDB are already tolerant for this somehow (I haven't tested polars). It will give the users impression that simply iceberg-datafusion integration is unstable rather than having the fix with DuckDB. So, Duck DB community making a fix is one way to push of course, but being tolerant from the iceberg-rust side also super important. -- 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]
