You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/10/09 04:00:38 UTC

[GitHub] [flink] wuchong commented on a change in pull request #13487: [FLINK-19409][Documentation] remove unrelated comment for getValue in ListView

wuchong commented on a change in pull request #13487:
URL: https://github.com/apache/flink/pull/13487#discussion_r502173323



##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/api/dataview/ListView.java
##########
@@ -73,17 +73,11 @@
  *
  *   public void accumulate(MyAccumulator accumulator, String id) {
  *     accumulator.list.add(id);
- *     ... ...
- *     accumulator.list.get()
- *     ... ...
+ *     accumulator.count++;
  *   }
  *
  *   {@literal @}Override
  *   public Long getValue(MyAccumulator accumulator) {
- *     accumulator.list.add(id);
- *     ... ...
- *     accumulator.list.get()
- *     ... ...
  *     return accumulator.count;

Review comment:
       We need to add an example to access the list here, because this is an example for ListView. Currently, there are no access to the list in the example. 
   
   I think we can update the aggregate function to return a String value which concats the elements in list, e.g. 
   
   ```java
   public String getValue(MyAccumulator accumulator) {
       // return a concated elements and the count
       return count + ":" + String.join("|", acc.list.get());
   }
   ```




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