zhongyujiang commented on pull request #3777: URL: https://github.com/apache/iceberg/pull/3777#issuecomment-998408677
> This is only called from test code, so I think the motivation was to speculative since other classes had similar updates. I don't think that we should move forward with this since it isn't actually used. I didn't notice that this is only used in test, and no, I am not speculating. If you read this [issue](https://github.com/apache/iceberg/issues/3604) and this [pr](https://github.com/apache/iceberg/pull/3605), you can find that I found the problem of ApplyNameMapping change custom parquet schema before another merged pr, I checked other classes extend ParquetTypeVisitor and found RemoveIds has the same problem. I fixed both in this [pr](https://github.com/apache/iceberg/pull/3605), but no one reviewed. Yesterday I found the issue of ApplyNameMapping is solved in another pr and already merged, so I closed my last pr and extracted the part of RemoveIds here. That's my motivation. -- 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]
