dylanhz commented on code in PR #27613: URL: https://github.com/apache/flink/pull/27613#discussion_r2851273416
########## flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala: ########## @@ -129,6 +129,9 @@ class CodeGeneratorContext( // string_constant -> reused_term private val reusableStringConstants: mutable.Map[String, String] = mutable.Map[String, String]() + // set of function instance term that will be added only once + private val reusableFunctionTerms: mutable.HashSet[String] = mutable.HashSet[String]() + Review Comment: > how can we be sure in this? The dedup key (fieldTerm) is derived from `functionIdentifier()`, which is the same key used by `addReusableObjectInternal` to generate member/init statements — so the dedup is consistent with existing behavior. > Can we add a test with custom udf and counter inside checking that it was invoked once? I added unit tests in `CodeGeneratorContextTest` that directly asserts `references.size` after calling `addReusableFunction` with the same/different functions, covering stateless dedup, stateful dedup, and stateful non-dedup cases. As for a counter-based test, `addReusableObjectInternal` creates instances via `InstantiationUtil.deserializeObject` that bypasses constructors, and `open()` was already deduplicated by LinkedHashSet before this fix, so neither counter can distinguish the before/after behavior. Let me know if you have better idea. -- 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]
