[GitHub] [flink] dawidwys commented on a change in pull request #13054: [FLINK-15803][table] Support DataViews in FLIP-65 aggregate functions

[GitBox <gi...@apache.org>]

dawidwys commented on a change in pull request #13054:
URL: https://github.com/apache/flink/pull/13054#discussion_r464985112



##########
File path: flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala
##########
@@ -671,28 +677,33 @@ class CodeGeneratorContext(val tableConfig: TableConfig) {
   /**
    * Adds a reusable [[DataStructureConverter]] to the member area of the generated class.
    *
-   * @param converter converter to be added
+   * @param dataType converter to be added
    * @param classLoaderTerm term to access the [[ClassLoader]] for user-defined classes
    */
   def addReusableConverter(
-      converter: DataStructureConverter[_, _],
+      dataType: DataType,
       classLoaderTerm: String = null)
     : String = {
+    reusableConverters.get(dataType) match {

Review comment:
       nit: should we ignore the nullability? If I am not mistaken we use the same converters for both nullable and not null types. In `DataTypeConverters` we use (`LogicalTypeRoot`, `conversionClass`) as a composite key.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org