Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4355#discussion_r130119984
  
    --- Diff: 
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/runtime/aggregate/GeneratedAggregations.scala
 ---
    @@ -19,13 +19,20 @@
     package org.apache.flink.table.runtime.aggregate
     
     import org.apache.flink.api.common.functions.Function
    +import org.apache.flink.table.dataview.{DataViewFactory, HeapViewFactory}
     import org.apache.flink.types.Row
     
     /**
       * Base class for code-generated aggregations.
       */
     abstract class GeneratedAggregations extends Function {
     
    +  var factory: DataViewFactory = new HeapViewFactory()
    +
    +  def getDataViewFactory: DataViewFactory = factory
    +
    +  def setDataViewFactory(factory: DataViewFactory): Unit = this.factory = 
factory
    --- End diff --
    
    I think we should rather add a method `initialize(ctx: RuntimeContext)` and 
generate code to register the state in this method. 
    
    IMO, the `DataViewFactory` is also not required, because 
    1. we can code-gen all of that functionality
    2. we can make heap the default for `MapView` and `ListView` such that we 
only need to replace it if it needs to be backed by state. So there would be 
only one implementation of `DataViewFactory`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to