slfan1989 commented on PR #13913: URL: https://github.com/apache/iceberg/pull/13913#issuecomment-3251638167
> > @dramaticlly Thank you very much for reviewing the code and for your valuable suggestions! I have adopted most of your suggestions. Regarding the suggestion to modify the field types, my consideration is that this PR mainly focuses on improving input parameter parsing. Changing the field types might require adjustments to the execution logic, so in order to maintain consistency with the original author's setup, I would prefer not to make this change. Do you think this is acceptable? > > Thanks @slfan1989 , generally I think separate changes into multiple pulls are acceptable. However for Spark, we generally want to keep the supported Spark versions in sync, so we likely will need a back port PR to have same change for Spark 3.4 and 3.5. Many PRs with back port can fanout quickly > > These suggested change from Boolean to boolean shall be relative straightforward, and our existing unit tests shall catch the problem with right coverage. Otherwise there's bigger problem in our procedure tests. Please let me know if this changes your mind. @dramaticlly Thank you for your explanation! I believe it makes sense. I will try to improve the code based on your suggestions. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
