Re: [PR] [FLINK-33792] Generate the same code for the same logic [flink]

2024-01-14 Thread via GitHub


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]

2024-01-11 Thread via GitHub


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]

2024-01-11 Thread via GitHub


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]

2024-01-11 Thread via GitHub


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]

2024-01-11 Thread via GitHub


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]

2024-01-10 Thread via GitHub


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]

2024-01-09 Thread via GitHub


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]

2024-01-09 Thread via GitHub


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]

2024-01-09 Thread via GitHub


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]

2024-01-08 Thread via GitHub


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]

2024-01-08 Thread via GitHub


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]

2024-01-08 Thread via GitHub


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]

2024-01-08 Thread via GitHub


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]

2024-01-02 Thread via GitHub


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]

2024-01-02 Thread via GitHub


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]

2024-01-02 Thread via GitHub


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]

2024-01-02 Thread via GitHub


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]

2024-01-02 Thread via GitHub


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]

2024-01-01 Thread via GitHub


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]

2023-12-29 Thread via GitHub


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]

2023-12-28 Thread via GitHub


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]

2023-12-28 Thread via GitHub


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]

2023-12-25 Thread via GitHub


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]

2023-12-24 Thread via GitHub


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]

2023-12-22 Thread via GitHub


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]

2023-12-21 Thread via GitHub


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]

2023-12-21 Thread via GitHub


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