Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24741
to look at the new patch set (#3).
Change subject: IMPALA-15296: Export the seam plan to Substrait
......................................................................
IMPALA-15296: Export the seam plan to Substrait
Add an exporter next to the IMPALA-15282 compatibility spike. It takes
the plan at the pre-conversion seam and either produces a Substrait plan
or says why there is none. A rejection carries a reason, what Isthmus
said, and the relations or operators the plan contained, so a caller
does not have to re-walk the plan to find out what stopped it. It never
returns a half-converted plan.
What Isthmus accepts is decided by Isthmus, not by a list kept here.
A list would drift as the Impala operator table changes, and it would
also be wrong: Impala's aggregate classes convert because Isthmus
normalises aggregates by SqlKind, while its scalar functions resolve on
operator identity and do not. Naming an operator Impala-something says
nothing about whether it exports.
Relations are the exception and are checked up front. Impala's own
relations are unambiguously outside Substrait, and listing them is more
use than the exception Isthmus raises on the first one it meets. This is
the cte_threshold case: above the threshold the repeated subtree becomes
an ImpalaSequence over a producer and one consumer per reference, and
the seam stops being a plan of Calcite relations.
The inventory a rejection carries has to reach window calls too. A
window call is a RexOver, and RexShuttle routes those to visitOver
rather than visitCall, so collecting only calls leaves an analytic plan
with an empty inventory: the evidence goes missing exactly where a
caller needs it.
Client labels are passed in rather than read off the plan. The seam's
row type is upper-cased, so exporting its names would rename the
client's columns.
Testing, in a local cluster on Linux, the three classes in one run:
- SubstraitPlanExporterTest: 11 tests, 0 failures, 0 errors. Covers the
twelve shapes measured to convert, the five aggregates, label
propagation, a protobuf round trip, the lead and lag split that
USE_IMPALA_OPERATOR produces, and each rejection with the offending
relation or operator named
- Dropping the visitOver override fails the window inventory test with
an empty list, so that assertion is not vacuous
- CalciteOptimizerTest and CalciteSubstraitCompatibilityTest, neither of
which this patch touches: 11 and 4 tests, 0 failures
- CalcitePlannerTest and TestCalciteStats fail 10 tests in that cluster
both with and without this patch; its functional catalogue is only
partly loaded
- git diff --check
Change-Id: I06e7645e6d6af599af6ec3c53780d12935ab7efb
Assisted-by: Claude Opus 5 (Anthropic)
---
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitPlanExporter.java
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitPlanExporterTest.java
2 files changed, 496 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/24741/3
--
To view, visit http://gerrit.cloudera.org:8080/24741
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06e7645e6d6af599af6ec3c53780d12935ab7efb
Gerrit-Change-Number: 24741
Gerrit-PatchSet: 3
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
