Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24749
to look at the new patch set (#5).
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
IMPALA-15303: Cover a real workload and the reading side
The exporter is covered by query shapes written by hand, one construct
at a time. Two things that says nothing about: what a real workload
does, and what a consumer reads back.
Run the exporter over the TPC-DS queries that carry Calcite goldens.
Every gap the exporter turned out to have was found this way rather than
by hand, and the numbers those runs produced have been quoted without
anything in the tree reproducing them. The tables come from the schema
template rather than from the catalogue, so this runs wherever the
frontend tests run and needs no loaded warehouse; only column types
matter for an export, which also means the plans are not the ones a
loaded warehouse would produce.
The expectation is a named list rather than a count: which queries do
not export and what stops each. A count would only say the number moved.
Removing the abs mapping, for instance, fails the test with tpcds-q47,
q53, q57, q63 and q89 named against ABS. Today 95 of the 103 distinct
queries export; the directory holds 110 files, seven of which are plan
variants of a query already in it, so the run skips a query text it has
already seen. The eight that do not export are outside this exporter,
four on SINGLE_VALUE and four on GROUPING, which is substrait-java#993.
A query leaving that list matters as much as one joining it, so both
directions are reported, and a file whose query section cannot be read
is named rather than quietly skipped.
Read the exported plan back with Isthmus and check it still says the
same: the same relations in the same order, the same column types, the
same output names. A plan can serialize cleanly and still mean something
else on the way through, which is substrait-java#1159, and that was
found from the reading side rather than the writing one. Field names on
the relations are not compared, and should not be: Substrait carries
output names on the plan root, so Calcite invents its own coming back.
One difference is pinned rather than worked around. Impala's string is a
varchar of maximum width, and reading it back gives a varchar of 65536
because Calcite caps one there. Substrait has an unbounded string that
would carry it without loss, but reaching it through a UserTypeMapper
does not work: the mapped type is dropped when a literal is rebuilt from
the Calcite type name, so a plan mixing a mapped column with a string
literal fails VirtualTableScan's check that its rows agree with its
schema. Measured on tpcds-q08 and reported as substrait-java#1170.
Upstream issues:
https://github.com/substrait-io/substrait-java/issues/1159
https://github.com/substrait-io/substrait-java/issues/1170
Testing, in a local cluster on Linux:
- SubstraitTpcdsExportTest: 1 test over the 103 distinct queries in 110
files, about eight seconds including the schema fixture
- SubstraitRoundTripTest: 2 tests, ten shapes and the varchar
- SubstraitPlanExporterTest, CalciteOptimizerTest and
CalciteSubstraitCompatibilityTest, none of which this patch touches:
22, 11 and 4 tests, 0 failures
- git diff --check
Assisted-by: Claude Opus 5 (Anthropic)
Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
---
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java
A
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitTpcdsExportTest.java
2 files changed, 434 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/24749/5
--
To view, visit http://gerrit.cloudera.org:8080/24749
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 5
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
