lsyldliu commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1444586092
########## flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGenUtils.scala: ########## @@ -124,14 +124,27 @@ object CodeGenUtils { private val nameCounter = new AtomicLong - def newName(name: String): String = { - s"$name$$${nameCounter.getAndIncrement}" + def newName(context: CodeGeneratorContext, name: String): String = { + if (context == null || context.getNameCounter == null) { + // Add an 'i' in the middle to distinguish from nameCounter in CodeGeneratorContext + // and avoid naming conflicts. + s"$name$$i${nameCounter.getAndIncrement}" + } else { + s"$name$$${context.getNameCounter.getAndIncrement}" + } } - def newNames(names: String*): Seq[String] = { + def newNames(context: CodeGeneratorContext, names: String*): Seq[String] = { require(names.toSet.size == names.length, "Duplicated names") - val newId = nameCounter.getAndIncrement - names.map(name => s"$name$$$newId") + if (context == null || context.getNameCounter == null) { + val newId = nameCounter.getAndIncrement + // Add an 'i' in the middle to distinguish from nameCounter in CodeGeneratorContext Review Comment: Why here will have conflicts? ########## flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/LongHashJoinGenerator.scala: ########## @@ -87,10 +90,11 @@ object LongHashJoinGenerator { def genProjection( tableConfig: ReadableConfig, classLoader: ClassLoader, - types: Array[LogicalType]): GeneratedProjection = { + types: Array[LogicalType], + parentCtx: CodeGeneratorContext): GeneratedProjection = { Review Comment: We can simplify the method signature to ``` def genProjection( types: Array[LogicalType], parentCtx: CodeGeneratorContext): GeneratedProjection ``` ########## flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/CastRule.java: ########## @@ -88,7 +91,16 @@ public ZoneId getSessionZoneId() { public ClassLoader getClassLoader() { return classLoader; } + + @Override + @Nullable + public CodeGeneratorContext getCodeGeneratorContext() { + return null; Review Comment: Can add some annotation to explain why here returns null? -- 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...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org