Hello Aman Sinha, Steve Carlin, Joe McDonnell, Michael Smith, Impala Public
Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24667
to look at the new patch set (#15).
Change subject: IMPALA-15282: Test Calcite plan export to Substrait
......................................................................
IMPALA-15282: Test Calcite plan export to Substrait
Add a test-only Isthmus compatibility spike at the logical seam before
Impala conversion, covering Values and Project, a named read with Filter
and Project, scalar arithmetic, and typed failure for unsupported
relations. A failed export carries the kind of failure and nothing else.
Output names ride in on the RelRoot instead of being stamped onto a
rebuilt Plan.Root. SubstraitRelVisitor.convert() reads them off
validatedRowType, so the three-argument RelRoot.of() is where the client
labels belong. The plan's own row type is not a substitute: its names
are upper-cased by the time they reach the seam, so the two-argument
form exports INT_LITERAL where the client asked for int_literal. Handing
the labels to Isthmus rather than overwriting its result afterwards also
puts its name propagation under test, which overwriting had hidden.
Getting a query to the seam sits in CalciteSeamTestBase rather than in
the test: it is Calcite's business and not Substrait's, and the exporter
tests later in this series need the same thing. CalciteOptimizerTest has
its own copy of it too, which IMPALA-15319 folds in separately.
Depend on substrait-java 0.103.0. The floor is 0.100.0, the first stable
release carrying the fix for virtual table literal types
(substrait-java#1064), and 0.103.0 is what the round-trip test later in
this series needs: substrait-java#1169 gives the character types their
declared length back.
Keep the two Surefire classpath tweaks the tests need. Pin protobuf-java
4.35.1: the Substrait gencode calls a protobuf 4 class, while
calcite-core brings 3.25.8 in through avatica-core, one level closer
than the 4.x that io.substrait:protobuf declares. Exclude the AWS SDK
bundle: it carries an unshaded SLF4J 1 Logger that wins over the SLF4J 2
whose Logger.atDebug Isthmus calls. Isthmus stays test-scoped and out of
impala-package.
Testing, on Linux against the released 0.103.0:
- Every test class in the module: 43 tests, 0 failures. That includes
CalciteOptimizerTest, which this patch does not touch and which shares
the module, so it is the check that the added dependencies and the
Surefire exclusion disturb nothing already there
- Dropping the AWS SDK exclusion fails every test in the two classes
that build a converter, 27 of them, on SLF4J 2 Logger.atDebug being
unavailable, and names the bundle as the jar it loaded the Logger from
- Reverting to the two-argument RelRoot.of() fails three of the four
tests on the name casing, so the root-name assertions are not vacuous
Change-Id: I359d6d6f88df63a7b0fdf7c6d541ec884602852f
Assisted-by: gpt-5.6-sol (OpenAI Codex), Claude Opus 5 (Anthropic)
---
M java/calcite-planner/pom.xml
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/CalciteSeamTestBase.java
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/CalciteSubstraitCompatibilityTest.java
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitTestBase.java
4 files changed, 555 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/24667/15
--
To view, visit http://gerrit.cloudera.org:8080/24667
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I359d6d6f88df63a7b0fdf7c6d541ec884602852f
Gerrit-Change-Number: 24667
Gerrit-PatchSet: 15
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
