Hello Joe McDonnell, Steve Carlin, Michael Smith, Stamatis Zampetakis, 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 (#14).
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. Reading the offending operator out of an
Isthmus message is prose-matching, so a caller counting what stopped a
corpus asks Rejected.offender() rather than matching on the text.
Relations are checked before conversion, because 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.
Expressions are left to Isthmus rather than to a list of what it
refuses, which would drift as the operator table changes and 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. What Isthmus cannot resolve by itself is taught to
it through additional signatures naming the Substrait function each
Impala operator stands for: scalar builtins such as abs and concat,
three window functions, and ndv, which earns approx_count_distinct on
the description rather than the name, both being HyperLogLog. Three need
more than a name, because Impala rewrites IF and cast into operators
carrying SqlKind.OTHER and a datetime plus arrives as a multiply of the
unit by the count, which Substrait has no form for. Substrait's
substring and round also take an i32 where Impala hands over a bigint,
so those
operands are narrowed first, integers only: Calcite's EXACT_TYPES also
holds DECIMAL, and casting one would drop everything after the point
(substrait-java#1163).
What stays refused says why, in the table and in a test. Substrait has
no truncate and no one-argument round; approx_count_distinct takes one
operand, so an ndv carrying a precision matches nothing and the
precision cannot be dropped quietly; GROUPING has no Substrait function
at all (substrait-java#993); and SINGLE_VALUE, which Calcite uses to
guard a scalar subquery and Impala turns into a runtime cardinality
check, would have to become any_value, which picks a row and asserts
nothing. One mapping is right only under a query option: length becomes
octet_length because Impala counts bytes, but with UTF8_MODE set
char_length is the one that matches, and the seam plan carries no
options to tell the two apart.
An Iceberg snapshot is refused on the same ground. RemoveSnapshotRule
takes the Snapshot node off the plan before the seam, leaving a scan
under a name Impala invented for this compilation, so a consumer
resolving it reads the table as it stands now and reports success.
Substrait can say what was meant, but substrait-java implements four of
the five read types and not ReadRel.IcebergTable (substrait-java#1172).
Timestamps leave at the precision Impala stores. ImpalaTypeSystemImpl
reports fifteen fractional digits and Substrait has room for nine, and
Isthmus copies the reported number through, so until now every plan
reading a table with a timestamp column was refused on the way back in
-- whether or not the query named that column, because the read carries
the table's whole schema. The narrowing is a UserTypeMapper rather than
an override of TypeConverter.toSubstrait: the converter asks the mapper
about every type it converts, one field at a time, so a relation's
schema and a type nested inside one are covered as well, which an
override is not -- toNamedStruct builds its fields through a private
overload. Two shapes the mapper covers cannot be reached from a query
today and carry no test: a timestamp with a local timezone, which the
planner refuses to cast before the seam, and a literal inside VALUES,
which needs IMPALA-15338.
A timestamp literal cannot be narrowed the same way and is refused,
carrying TIMESTAMP_LITERAL. LiteralConverter writes the value in units
of a precision it reads through a static call, so at fifteen digits
2020-01-01 is 1577836800 * 10^15, which wraps to -5454344746497474560
in a signed 64-bit integer; before this the plan exported and carried
that number.
Caffeine has to be declared for the Iceberg tests to run at all: the
only unshaded copy reaching this module rides inside the AWS SDK bundle
that the Surefire configuration drops, so an Iceberg table loaded as an
IncompleteTable whose cause was a NoClassDefFoundError.
Testing, in a local cluster on Linux:
- SubstraitPlanExporterTest: 27 tests, 0 failures, covering each mapping
by the Substrait function it produces, each refusal by the operator or
relation named, and the exporter's own contract
- The whole Substrait set in this module, which the two changes above
this one extend: 61 tests, 0 failures
- Withholding any one of the three converters, the narrowing or the
folding fails the test that covers it; withholding the snapshot check
fails its test with "expected ... to be rejected but it exported"
- Withholding the timestamp narrowing fails the test that reads the
precision back out of an exported plan and the corpus one, on its
first query, with expected:<9> but was:<15>. Refusing null literals
too, or dropping Expression.Literal.PrecisionTimestamp from the walk,
each fails one test and only that one
Assisted-by: Claude Opus 5 (Anthropic)
Change-Id: I06e7645e6d6af599af6ec3c53780d12935ab7efb
---
M java/calcite-planner/pom.xml
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitOperatorMappings.java
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
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitProtoTypes.java
5 files changed, 1,637 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/24741/14
--
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: 14
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: Stamatis Zampetakis <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
