Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
libenchao closed pull request #23984: [FLINK-33792] Generate the same code for the same logic URL: https://github.com/apache/flink/pull/23984 -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1888360571 @lsyldliu I have updated my code, please have a took when you have time. -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1888301428 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1886966518 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1886953624 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
lsyldliu commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1448202301 ## 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: I just mean that we can get the `tableConfig` and `classLoader` from `parentCtx`, but this isn't an important point, ignore it now. -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1884085496 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1882900226 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1882729028 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1882328998 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
lsyldliu commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1444597660 ## 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 you 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1440008761 ## 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 = null, name: String): String = { Review Comment: Get it, removed -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1874770437 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
libenchao commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1439438203 ## 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 = null, name: String): String = { Review Comment: My point is that do you really need to have a default value, IIUC, we want all places to pass `context`? -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1873932825 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1439369220 ## 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 = null, name: String): String = { Review Comment: Parameter section with *-parameter is not allowed to have default arguments ## 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 = null, 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) { Review Comment: The returned new names use the same index, I keep it the same as before. So, I did not call `newName` -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
libenchao commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1439188114 ## 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 = null, 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) { Review Comment: I'm wondering that if `newNames` can be optimized to something like `names.map(name => newName(context, name))`? ## flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/codegen/CodeGenUtilsTest.scala: ## @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.flink.table.planner.codegen + +import org.apache.flink.configuration.Configuration + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +import scala.collection.mutable.ArrayBuffer + +class CodeGenUtilsTest { + private val classLoader = Thread.currentThread().getContextClassLoader + + @Test + def testNewName(): Unit = { Review Comment: Could you also add a test for `parentCtx`? ## flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala: ## @@ -37,15 +37,28 @@ import org.apache.flink.util.InstantiationUtil import java.time.ZoneId import java.util.TimeZone +import java.util.concurrent.atomic.AtomicLong import java.util.function.{Supplier => JSupplier} import scala.collection.mutable /** * The context for code generator, maintaining various reusable statements that could be insert into * different code sections in the final generated class. + * + * Caution: As we use nameCounter in each CodeGeneratorContext, we must ensure that a unique + * CodeGeneratorContext(or contexts share the same ancestors) is used throughout the entire class to + * avoid naming conflicts. So when we create a context for a class, we can set parentCtx to null. + * However, when we create a context for a code block, we must ensure that all contexts for code + * blocks in a class share a common ancestor by setting parentCtx. */ -class CodeGeneratorContext(val tableConfig: ReadableConfig, val classLoader: ClassLoader) { +class CodeGeneratorContext( +val tableConfig: ReadableConfig, +val classLoader: ClassLoader, +parentCtx: CodeGeneratorContext) { Review Comment: Can this be `final` too? ## 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 = null, name: String): String = { Review Comment: This one has a default value for `context`, but `newNames` does not have, is there any rational behind this? -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1871831781 @libenchao CI is passed, please take a look when you have time. -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1871674246 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1870974216 @flinkbot run azure -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
libenchao commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1868890131 @zoudan The PR looks good to me, except that the CI is failing, could you fix that? -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1868679111 @libenchao Thanks for reviewing, I have remove the configuration, please take a look when you have time. -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
libenchao commented on code in PR #23984: URL: https://github.com/apache/flink/pull/23984#discussion_r1434845148 ## flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/TableConfigOptions.java: ## @@ -233,6 +233,16 @@ private TableConfigOptions() {} .withDescription( "Specifies a threshold where class members of generated code will be grouped into arrays by types."); +@Documentation.TableOption(execMode = Documentation.ExecMode.BATCH_STREAMING) +public static final ConfigOption INDEPENDENT_NAME_COUNTER_ENABLED = Review Comment: Why do we need this config, I think it's ok to change the behavior by default, it's no meaning to expose it to users. ## flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/HashCodeGenerator.scala: ## @@ -80,23 +80,25 @@ object HashCodeGenerator { } """.stripMargin +System.out.println(code) Review Comment: Remove it. -- 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
Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]
flinkbot commented on PR #23984: URL: https://github.com/apache/flink/pull/23984#issuecomment-1867349736 ## CI report: * d996c020cea9582b5df989f4eb277efb395931fb UNKNOWN Bot commands The @flinkbot bot supports the following commands: - `@flinkbot run azure` re-run the last Azure build -- 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
[PR] [FLINK-33792] Generate the same code for the same logic [flink]
zoudan opened a new pull request, #23984: URL: https://github.com/apache/flink/pull/23984 ## What is the purpose of the change This pull request ensure that we generate the same code for the same logic, it is a precondition for sharing generated classes between different jobs. ## Brief change log - add a name counter in each `CodeGeneratorContext` and use it when we generate names for variables. ## Verifying this change Please make sure both new and modified tests in this PR follows the conventions defined in our code quality guide: https://flink.apache.org/contributing/code-style-and-quality-common.html#testing This change added tests and can be verified as follows: - Add a new test: CodeGenUtilsTest#testNewName - Add tests in existing class: HashCodeGeneratorTest#testHashWithIndependentNameCounter and ProjectionCodeGeneratorTest#testHashWithIndependentNameCounter ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (yes) - If yes, how is the feature documented? (docs) -- 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