drill
drill copied to clipboard
DRILL-3715 Support all encoded vector types for Hash Aggregator.
@StevenMPhillips @jaltekruse
We should have a plan verification test to make sure that this change actually has the desired effect. This should be able to execute fine without your change as well, but the plan will have and unnecessary SelectionVectorRemover in it today. You can see examples of plan verification tests in /Users/jaltekruse/drill/exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java and other subclasses of PlanTestBase
There have been bugs in the handling of sv2 in the case where the first and second batch are filtered out. Could you add a test case for this? a similar test was added for streaming aggregate as part of DRILL-3069: https://github.com/apache/drill/commit/97a63168e93a70c7ed88d2e801dd2ea2e5f1dd74
@mehant @jaltekruse Could you take a look at recent patch? Thanks.
@amithadke, could you look at trying to write a test like this instead of checking in a large input file?
There is a session option to turn off hash aggregate, you should be able to just duplicate this test changing the session option. Be sure to change it back to the default value after your test as the session is currently shared across tests.
https://github.com/apache/drill/commit/97a63168e93a70c7ed88d2e801dd2ea2e5f1dd74#diff-91bfb8c9a26887fbe594262b6b872c98R295
I got that backwards, it looks like the existing test should be disabling hash aggregate and verifying that streaming agg is planned instead. To be very thorough you might want to include a plan check in your test, but hash based operations are currently the default.
@amithadke, did you have a chance to rework test in the way @jaltekruse proposed?