ibzib commented on pull request #13934: URL: https://github.com/apache/beam/pull/13934#issuecomment-780863264
> Is there any explantation (one pager or JIRA) to explain the intention of this refactoring? Especially what is the follow up steps after this PR? (this solely refactoring does not look very useful) @amaliujia There are two main reasons for this PR. One is to address review comments on previous UDF PRs, and improve the structure of our code. Currently, we create the ZetaSQL representation of UDFs in SqlAnalyzer, while we create a Beam representation of UDFs in the PlannerImpl. Really, we should not be doing any low level operations like handling UDFs in PlannerImpl. The refactor gives better object oriented design by merging both of those operations into a single method. Also, the new class introduced in this PR better represents the 1:1 relationship between the SimpleCatalog and the SchemaPlus. I don't think SqlAnalyzer is the right place to handle catalog tasks because populating the catalog and analyzing the query are logically separate tasks. The other reason is to make registerUdf work with ZetaSQL. We will need to extract UDFs from the SchemaPlus. Currently, because the two operations mentioned above are separate, we we would have to extract the UDF from SchemaPlus twice. So in summary this PR will make the implementation of registerUdf much simpler and cleaner. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
