kgyrtkirk commented on code in PR #16738: URL: https://github.com/apache/druid/pull/16738#discussion_r1678871426
########## extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java: ########## @@ -724,4 +729,20 @@ public void testOverWindow() )) .run(); } + + @Test + public void testStddevNotSupportedOverWindow() + { + assumeFeatureAvailable(EngineFeature.WINDOW_FUNCTIONS); + + DruidException e = assertThrows( + DruidException.class, + () -> testBuilder() + .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS, true)) + .sql("SELECT stddev(m1) OVER () from numfoo") + .run() + ); + + assertTrue(e.getMessage().contains("Aggregation [STDDEV] is currently not supported for window functions")); Review Comment: `assertTrue` in this form doesn't give much to go on... either pass the full `e.getMessage()` as the error description; use `assertThat` like in the other tests ########## extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java: ########## @@ -724,4 +729,20 @@ public void testOverWindow() )) .run(); } + + @Test + public void testStddevNotSupportedOverWindow() Review Comment: I see why this test is here; I was looking for an agg in the `sql` module to move it there... I've bumped into a line where finalize is set to [false here](https://github.com/apache/druid/blob/6cf6838eb974634ce5ae664d5ca877d8c804036a/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java#L163) the comment is not entirely accurate; I think the window aggs do finalize now [here](https://github.com/apache/druid/blob/6cf6838eb974634ce5ae664d5ca877d8c804036a/processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultFramedOnHeapAggregatable.java#L315) so that comment should be removed or updated -- 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: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org