David-Banquet commented on PR #3390: URL: https://github.com/apache/iceberg-rust/pull/3390#issuecomment-6109549770
Thanks for picking this up again. This drops the older schemas entirely, so columns removed from the table also disappear from Glue. The Java catalog keeps them: `toColumns` in `IcebergToGlueConverter` writes the current schema first, then the older schemas with `iceberg.field.current=false`, and skips any name it has already added. The duplicates come from that missing name check, so keeping the loop and adding a `HashSet` of top-level names already written would fix #2749 and keep the Glue output the same as Java's. A table written by both Spark and iceberg-rust would otherwise see its Glue columns change depending on which engine committed last. If the maintainers prefer current-schema-only, that's a fair choice too, but it's a behavior change from Java and probably worth saying so in the PR description. Two smaller things: - The new test only adds a column, where both approaches give the same result. A case that drops a column (and one where a field keeps its name across schemas with a different type) would show which behavior is intended. - `builder.is_current = false;` and its comment can go: the fields are private and nothing reads them after `from_iceberg` returns. -- 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]
