zoudan commented on code in PR #23984:
URL: https://github.com/apache/flink/pull/23984#discussion_r1445558740
##########
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:
Sorry, I do not get your point, I think `tableConfig` and `classLoader` are
needed in this method. And I only add the parameter `parentCtx` without change
anything else.
##########
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:
There may be some scenarios where it is not convenient for us to obtain
class level CodeGeneratorContext, and we could use the nameCounter in
`CodeGenUtils` to generate new names. In these cases we may use nameCounter
from `CodeGenUtils` and `CodeGeneratorContext` in the same class.
##########
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:
Change it to non 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
