Aleksandr Efimov has posted comments on this change. (
http://gerrit.cloudera.org:8080/24741 )
Change subject: IMPALA-15296: Export the seam plan to Substrait
......................................................................
Patch Set 9:
This is the second of four changes and the one to read first; the other three
are smaller and lean on it.
24667 438 lines what Isthmus can and cannot take from the seam, as a
test
24741 1441 lines the exporter and the operator mappings <- start here
24749 441 lines TPC-DS coverage and the reading side
24752 144 lines Calcite's estimates carried into the plan as hints
All of it lives under src/test, apart from a test-scope dependency block in
java/calcite-planner/pom.xml. Nothing runs in a query path, so the series can
only affect the build and the tests themselves.
Two things I would rather you challenged than took on trust.
The refusals are decisions, not gaps. SINGLE_VALUE could be exported as
any_value, and I think it should not be: Calcite uses it to guard a scalar
subquery, Impala turns it into a runtime cardinality check, and any_value picks
a row and asserts nothing, so a query that fails on bad data would start
returning an arbitrary one with nothing in the plan for a consumer to notice.
An Iceberg snapshot is refused on the same ground -- after RemoveSnapshotRule
the name left on the scan is synthetic, so a consumer falling back to the base
name would read the table as it stands now. Both sit in the rejection enum with
the reasoning beside them.
The mappings are keyed on operator identity, which is the part most likely to
rot. Isthmus resolves a scalar call by looking the operator up in a map, and
Calcite compares operators by class, name and kind, so an Impala operator
meaning exactly what Substrait's does still misses. Aggregates escape this
because Isthmus normalises them by SqlKind. That asymmetry is why the list
reads as arbitrary: it is a list of what identity comparison loses, not of what
Impala has.
Two gaps are upstream and tracked -- GROUPING has no Substrait function
(substrait-java#993), and the Iceberg read type is in the spec but not in
substrait-java (#1172, filed this week).
On CI: I could not get a dryrun today. The built-in node on jenkins.impala.io
has not been able to fork a process since early Wednesday, which takes out
gerrit-verify-dryrun-external and auto-critic together; there is a thread on
dev@ about it. Locally I ran the whole calcite-planner module against a
baseline of the same tree without the series, and the failure counts match
class for class.
No rush on my side -- take it whenever it suits.
--
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: comment
Gerrit-Change-Id: I06e7645e6d6af599af6ec3c53780d12935ab7efb
Gerrit-Change-Number: 24741
Gerrit-PatchSet: 9
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[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]>
Gerrit-Comment-Date: Wed, 26 Aug 2026 09:47:50 +0000
Gerrit-HasComments: No
