rubenada commented on PR #3640: URL: https://github.com/apache/calcite/pull/3640#issuecomment-1935544748
Thanks for your patience and the thorough explanation @HanumathRao , I really appreciate it. I understand a bit more the situation now... and I beg to differ. IMHO fix 2 is just "sweeping the problem under the rug", we could avoid the problem in `Prepare.java`, but the underlying issue still remains. Plus, before removing this [Chesterton's fence](https://en.wiktionary.org/wiki/Chesterton%27s_fence), we would need to understand the reasons why field trimming was included in there in the first place. Also, removing the trimming can have undesired consequences (e.g. some projects might have fields which are expensive to retrieve/compute, so they should only be part of the final plan if they are really used, otherwise they should be trimmed; so this change would lead to a performance regression). I think fix 1 is the right way to go (although I understand it will be harder). Field trimming is quite a powerful tool (not without some bugs), which can be used by downstream projects (either via SqlToRelConverter#trimUnusedFields or directly via RelFieldTrimmer, since both are public); so if we find a bug on it, I think it would be better to try to fix it. -- 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]
