You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org> on 2026/08/26 00:14:49 UTC
[Impala-ASF-CR] IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
Aleksandr Efimov has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24749
Change subject: IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
......................................................................
IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
The exporter is covered by query shapes written by hand, one construct
at a time. Every gap it turned out to have was found by running a real
workload instead, and the numbers those runs produced have been quoted
in IMPALA-15297 and after without anything in the tree that reproduces
them.
Run the exporter over the TPC-DS queries that carry Calcite goldens, as
a test.
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, so no data or
statistics are involved, which also means the plans here 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 that the number
moved. Removing the abs mapping, for instance, fails the test with
tpcds-q47, q53, q57, q63 and q89 named against ABS.
Today 102 of 110 export. The eight that do not are outside this
exporter: four stop on SINGLE_VALUE, which Calcite uses to guard a
scalar subquery and Impala turns into a runtime cardinality check that
Substrait has no way to express, and four on GROUPING, which is
substrait-java#993. A query leaving that list matters as much as one
joining it, so the test reports both directions.
Upstream issue:
https://github.com/substrait-io/substrait-java/issues/993
Testing, in a local cluster on Linux, the four classes in one run:
- SubstraitTpcdsExportTest: 1 test over 110 queries, 0 failures. Takes
about nine seconds, the schema fixture included
- SubstraitPlanExporterTest: 20 tests, 0 failures, 0 errors
- CalciteOptimizerTest and CalciteSubstraitCompatibilityTest, neither
of which this patch touches: 11 and 4 tests, 0 failures
- Dropping a mapping fails the added test with the affected queries
named, so it is not vacuous
- git diff --check
Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Assisted-by: Claude Opus 5 (Anthropic)
---
A java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitTpcdsExportTest.java
1 file changed, 228 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/24749/1
--
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: newchange
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
[Impala-ASF-CR] IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
......................................................................
Patch Set 1:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24749/1/java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitTpcdsExportTest.java
File java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitTpcdsExportTest.java:
http://gerrit.cloudera.org:8080/#/c/24749/1/java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitTpcdsExportTest.java@31
PS1, Line 31: import java.util.TreeSet;
unused import: java.util.TreeSet
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 00:17:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23805/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 5
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 03:42:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
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 <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Steve Carlin, Michael Smith, 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 (#12).
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, which says nothing about what a real workload does or about
what a consumer reads back. This adds both.
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. 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 eight that do not being outside this exporter, four
on SINGLE_VALUE and four on GROUPING (substrait-java#993). A query
leaving that list matters as much as one joining it, so both directions
are reported.
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 -- substrait-java#1159, 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.
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 width that
substrait-java#1169 gives back, which is what pins 0.103.0
- SubstraitPlanExporterTest, CalciteOptimizerTest and
CalciteSubstraitCompatibilityTest, none of which this patch touches:
0 failures
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/12
--
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: 12
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23933/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 10
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Sep 2026 06:37:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 5:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24749/5/java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java
File java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java:
http://gerrit.cloudera.org:8080/#/c/24749/5/java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java@21
PS5, Line 21: import static org.junit.Assert.assertTrue;
unused import: org.junit.Assert.assertTrue
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 5
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 03:24:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23869/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 9
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 31 Aug 2026 13:57:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real Calcite workload and Substrait readback
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Steve Carlin, Michael Smith, 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 (#15).
Change subject: IMPALA-15303: Cover a real Calcite workload and Substrait readback
......................................................................
IMPALA-15303: Cover a real Calcite workload and Substrait readback
The exporter is covered by query shapes written by hand, one construct
at a time, which says nothing about what a real workload does or about
what a consumer reads back. This adds both.
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. 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 eight that do not being outside this exporter, four
on SINGLE_VALUE and four on GROUPING (substrait-java#993). A query
leaving that list matters as much as one joining it, so both directions
are reported.
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 -- substrait-java#1159, 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.
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 width that
substrait-java#1169 gives back, which is what pins 0.103.0
- SubstraitPlanExporterTest, CalciteOptimizerTest and
CalciteSubstraitCompatibilityTest, none of which this patch touches:
0 failures
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/15
--
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: 15
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23808/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 7
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 03:55:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 8: Verified+1
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 8
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Sun, 30 Aug 2026 12:07:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
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 (#7).
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, 441 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/24749/7
--
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: 7
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 7:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/24749/7/java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java
File java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java:
http://gerrit.cloudera.org:8080/#/c/24749/7/java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java@21
PS7, Line 21: import static org.junit.Assert.assertTrue;
unused import: org.junit.Assert.assertTrue
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 7
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 03:30:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover the TPC-DS corpus in the Substrait export tests
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/23794/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Aug 2026 00:30:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24749 )
Change subject: IMPALA-15303: Cover a real workload and the reading side
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/13959/ DRY_RUN=true
--
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: comment
Gerrit-Change-Id: If0f8921a4fd3bde41e227f7c0e986e82cf0d3ffa
Gerrit-Change-Number: 24749
Gerrit-PatchSet: 8
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Sun, 30 Aug 2026 07:33:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Steve Carlin, Michael Smith, 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 (#9).
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, 429 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/24749/9
--
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: 9
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-15303: Cover a real workload and the reading side
Posted by "Aleksandr Efimov (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Steve Carlin, Michael Smith, 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 (#10).
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 used to be pinned here. Impala's string is a varchar of
maximum width, and reading it back gave a varchar of 65536, because
Calcite caps one there and the Substrait type system left that default
alone. substrait-java#1169, released in 0.103.0, gives the character
types their declared length back, so the width now survives and the
corpus carries a string-valued call as well. Against 0.101.0 both of
those checks fail, which is what pins the version this depends on.
Upstream issues, closed since they were filed:
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/10
--
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: 10
Gerrit-Owner: Aleksandr Efimov <dz...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>