[GitHub] flink pull request #5555: [FLINK-8689][table]Add runtime support of distinct...

[fhueske <gi...@git.apache.org>]

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

    https://github.com/apache/flink/pull/5555#discussion_r182219690
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/runtime/stream/sql/OverWindowITCase.scala ---
    @@ -50,6 +51,96 @@ class OverWindowITCase extends StreamingWithStateTestBase {
         (8L, 8, "Hello World"),
         (20L, 20, "Hello World"))
     
    +  @Test
    +  def testProcTimeDistinctBoundedPartitionedRowsOver(): Unit = {
    --- End diff --
    
    I tested a few other queries with distinct aggregates. 
    * Queries with non-windowed `DISTINCT` aggregations work, but they are they are translated without distinct aggregations. So they changes in this PR are not used.
    * Queries with `DISTINCT` aggregates and `TUMBLE` or `HOP` windows fail with strange exceptions. Did not look related to these changes, but would be good to check.
    
    We don't have to fix these things in this PR (unless it is broken by these changes).
    
    In general, I think it would be good to add unit tests for the `AggregationCodeGenerator`. We could generate  `GeneratedAggregations` for different configurations, compile them, and check if the result is expected. Actually, we should have done that already. This should also work for state-backend backed views if we embed the test in a HarnessTest.


---