Aleksandr Efimov has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/24745


Change subject: IMPALA-15299: Cover grouping sets in the Substrait export tests
......................................................................

IMPALA-15299: Cover grouping sets in the Substrait export tests

The exporter is covered by shapes that came from the direct harness,
and none of them group by more than one set. Impala produces such
aggregates: CoerceNodes rebuilds the LogicalAggregate with
getGroupSets() untouched, and ImpalaAggRel builds one set of
expressions per set, so grouping sets, rollup and cube all reach the
seam as a real multi-set aggregate.

All three export today, without anything added to the exporter, and
nothing says so. Cover them, so that a change at the seam that flattens
or reorders the sets is caught here rather than downstream.

Cover the other half too. Neither grouping() nor grouping_id()
converts: Substrait has no grouping function and Isthmus does not
synthesise one, which is substrait-java#993. The shape of the query
makes no difference, sets or rollup or cube, one column or several, so
one case per spelling is enough. They belong here as expected
rejections rather than as candidates, since the fix is not ours.

The assertions are on the plan rather than on its bytes, and the sets
name their fields in ascending order of first appearance.
substrait-java#1161 moves the difference between Substrait's column
order for a multi-set aggregate and Calcite's into the aggregate's emit
mapping, and states that a plan already agreeing with Calcite
round-trips unchanged, so shapes written this way are the ones that
survive that upgrade.

Upstream issues:
https://github.com/substrait-io/substrait-java/issues/993
https://github.com/substrait-io/substrait-java/pull/1161

Testing, in a local cluster on Linux, the three classes in one run:
- SubstraitPlanExporterTest: 16 tests, 0 failures, 0 errors
- CalciteOptimizerTest and CalciteSubstraitCompatibilityTest, neither
  of which this patch touches: 11 and 4 tests, 0 failures
- The same shapes were measured against isthmus 0.101.0 as well as the
  pinned 0.100.0, with identical results
- git diff --check

Change-Id: I61b6c1ead2694950b1b6c3a56c50c5e5b41c06fd
Assisted-by: Claude Opus 5 (Anthropic)
---
M 
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitPlanExporterTest.java
1 file changed, 52 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/24745/1
--
To view, visit http://gerrit.cloudera.org:8080/24745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I61b6c1ead2694950b1b6c3a56c50c5e5b41c06fd
Gerrit-Change-Number: 24745
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <[email protected]>

Reply via email to