fix(isthmus)!: keep the declared column order of an aggregate over grouping sets

[alexandrefimov]

substrait-java takes the grouping columns of an aggregate to be the distinct grouping expressions in the order they first appear across its grouping sets, while Calcite takes them from a bit set and emits them ordered by field index. (The spec orders them by the relation's shared grouping-expression list, which each set's expression references index into; the POJO models a per-set expression list and cannot hold that order, so first-appearance is this library's reconstruction — spec v0.101.0.) Neither direction accounted for that, so a plan whose sets first mention field 1 and then field 0 changed meaning on the way through: a reference to the aggregate's first column reached the column Calcite had put there instead, and its type changed with it.

Both directions now carry the difference in the emit mapping rather than in the shape of the plan. On the way in, the mapping a relation carries is translated into the order the converted aggregate emits, and a relation that emits directly gets the mapping that puts its columns back in the declared order. On the way out, the mapping presents the aggregate's output in Calcite's order, so a parent converted from the same Calcite plan finds its columns where it left them. Neither direction adds a relation the plan did not have, so a plan that already agrees with Calcite round-trips unchanged — which is what Substrait2SqlTest.simpleTestGroupingSets checks, and it is also how I found that fixing only one direction is not enough.

The pre-aggregate projection reuses one column for a field grouped on by several sets — two copies of it would each be missing from a grouping set, and Calcite would make both nullable — and only there, because that is where Aggregate.deriveRecordType dedups: a lone grouping set gives every mention a column of its own.

That leaves the order itself no reason to send an aggregate through the projection, since the emit mapping now carries it, and it no longer does. What a rewrite is still needed for is a grouping set naming an expression twice, which Calcite's bit set cannot hold. Measured against origin/main (934a60e):

aggregate over foo(a i64, b i64, c string)	declares	main	now
one set (2, 0, 2)	3 columns	3	3
one set (2, 0, 2), emit (0, 1, 2)	3 columns	3	3
one set (0, 2, 2)	3 columns	2	3
one set, a + 42 twice	2 columns	2	2
two sets (2, 0) and (2)	3 columns	4	3

The (0, 2, 2) row is wrong on main as well: its fields are ascending, so the aggregate went to Calcite directly and the bit set ate a column. The visible change from the order no longer triggering a rewrite is where the reordering sits — LogicalProject(c=[$1], a=[$0]) over LogicalAggregate(group=[{0, 2}]) over the scan, where the projection used to sit under the aggregate.

The per-mention count that keeps a repeated grouping expression a column of its own is a convention of this library rather than something the plan carries. RelProtoConverter.visit(Aggregate) dedups grouping_expressions unconditionally, where Aggregate.deriveRecordType dedups only across several sets, so the one-set (2, 0, 2) above writes two grouping expressions with references [0, 1, 0]: three columns here, two to a consumer reading the proto as the spec's list of grouping expressions in declaration order. A round trip through proto still compares equal, because both directions flatten per-Grouping. That split is #1210.

Worth a second look, since the change touches a branch that had no coverage: the GROUP_ID branch of SubstraitRelVisitor.visit(Aggregate) used a grouping-field count that included a field shared by several sets more than once, which disagrees with Aggregate.deriveRecordType. It now uses the distinct count. No query reaches that branch — Calcite folds GROUP_ID() into a literal wherever it can work out the answer, including over duplicated grouping sets — so the test that covers it builds the Calcite aggregate directly.

Two more defects in the grouping-set index column came out of the same math and are fixed here. The mapping that keeps that column replaced its index with aggregateCalls.size() - 1, which counts aggregate calls rather than output columns, so what came back was a copy of a grouping column. And the index the relation declares for it counted every mention of a grouping expression, so a field grouped on by several sets shifted it past the end: such an aggregate with a mapping that keeps the index threw ArrayIndexOutOfBoundsException, on main as well. Both counts are over the distinct grouping columns now. What the column holds is still Calcite's folded GROUP_ID() literal, which is #1182.

One difference is left alone: the grouping-set index comes back as an i64. The conversion builds Calcite's GROUP_ID call as a BIGINT while the spec gives the aggregate an i32 column, and Calcite folds the call to a literal of its own type. That is #1162.

Closes #1159

BREAKING CHANGE: an aggregate whose grouping sets do not first mention its fields in ascending order now converts with its grouping columns in the order the relation declares them, in both directions. For the sets {0, 3} and {1, 2}, converting the relation to Calcite emits fields 0, 3, 1, 2 where it emitted 0, 1, 2, 3, and converting the Calcite aggregate back gives the emit mapping [0, 2, 3, 1] where it gave [0, 1, 2, 3]. An aggregate whose grouping sets already agree with Calcite's field order converts as before.

[nielspardon]

The unconditional dedup in updateGrouping needs a decision before the rest is worth reading: a single grouping set that names a grouping expression twice now converts to fewer Calcite columns than the relation declares, where main converted the same plan correctly. Dropping the isOrdered trigger in isValidCalciteGrouping looks like the cleanest way out, since the emit mapping added here already handles a within-set reorder and the dedup map then has no job — but that reaches past the bug, so it is your call.

[nielspardon]

Add the BREAKING CHANGE: footer — the ! cuts the release, but the footer text is what fills the ⚠ section of the notes, and without it they ship empty for a change that rewrites the emit mapping in both directions. The rest is three comments the fix outran and one test gap; the type-blind grouping-column count is #1210's, not this PR's.

[nielspardon]

All three carried findings are fixed and the BREAKING CHANGE: footer is in place; one import nit below. The type-blind grouping-column count at SubstraitRelNodeConverter:368 stays with #1210 rather than growing this PR.

[nielspardon]

Drop the two (spec v0.101.0) markers: the rule they annotate has read identically since v0.57.0, so the version names what substrait-java pins rather than when the ordering changed.

[nielspardon]

LGTM