dawidwys commented on issue #11785: URL: https://github.com/apache/flink/pull/11785#issuecomment-616377734
Hi @WeiZhong94 thank you for working on this. Personally I would choose a different approach in a couple of places. Let me describe how I see it. The ultimate goal is to have: ``` private final Map<String, CatalogFunction> tempSystemFunctions = new LinkedHashMap<>(); private final Map<ObjectIdentifier, CatalogFunction> tempCatalogFunctions = new LinkedHashMap<>(); ``` To get there we should: 1. We should not instantiate `FunctionDefinitions` by default. Similarly as you are doing with the `UninstantiatedSystemFunction`. Instead of `UninstantiatedSystemFunction` we should use `CatalogFunctionImpl` though. 2. We should add a `CatalogFunction` that internally holds an instantiated `FunctionDefinition`. (for cases like `.select(new ScalarFunction(...))`) . We could call it maybe: `InlineCatalogFunction`. Better name welcome. It could look sth like this: ``` class InlineCatalogFunction implements CatalogFunction { private final FunctionDefinition definition; ... } ``` Having those we can use the maps as described in the beginning. ---------------------------------------------------------------- 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: us...@infra.apache.org