Jiyoung Yoo has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24811
Change subject: IMPALA-15282: Export Substrait plan from Calcite planner (prototype) ...................................................................... IMPALA-15282: Export Substrait plan from Calcite planner (prototype) Squashed from 3 commits (test infra + prototype export + a follow-up pom.xml fix), prototype work not intended to ship as-is. Original commit messages preserved below in commit order. ====================================================================== Commit 1/3: dc8e1e682beb267adfbf80dc35cbe1c2528ee507 ====================================================================== IMPALA-15282: Test Substrait export Add a test-only Isthmus compatibility spike at the logical seam before Impala conversion. A failed export carries the kind of failure and nothing else: an earlier draft also carried a partial plan that was never anything but null at every construction site, so the assertion about it could not have failed. The execution behaviour comes from the converter provider, which builds exactly it, rather than being copied out by hand. Cover Values and Project, a named read with Filter and Project, scalar arithmetic, and typed failure for unsupported relations. Verify client-visible root names, protobuf serialization and parsing, schemas, field references, and relation and function inventories. 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 RelRoot.of() 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 exactly the same thing; CalciteOptimizerTest has its own copy of it too, which is worth folding in separately. SubstraitTestBase adds what only a caller of Isthmus needs, the check that SLF4J 2 and protobuf 4 are the versions the test classpath has. Depend on substrait-java 0.103.0, the current release. The floor is 0.100.0, the first stable release carrying the substrait-java#1065 fix for virtual table literal types (substrait-java#1064), and later in this series it rises to 0.101.0, where Rel.withHint arrived to carry the plan statistics. Taking the current release rather than either floor is not free: 0.103.0 carries substrait-java#1169, which gives the character types their declared length back, and that is what the round-trip test later in this series now expects. 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. Upstream issue: https://github.com/substrait-io/substrait-java/issues/1064 Upstream fix, released in 0.100.0: https://github.com/substrait-io/substrait-java/pull/1065 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 test-scope dependencies and the Surefire classpath exclusion do not disturb what is there - Against 0.101.0 the round-trip test later in this series fails twice, both times on character width, which is what the move to 0.103.0 buys - 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 the failure names the bundle as the jar it loaded org/slf4j/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 - git diff --check Change-Id: I359d6d6f88df63a7b0fdf7c6d541ec884602852f Assisted-by: gpt-5.6-sol (OpenAI Codex), Claude Opus 5 (Anthropic) ====================================================================== Commit 2/3: fcd62dfff7709f8804019cb4bbbe5ae1c215058f ====================================================================== remove test scope Change-Id: I29ef0310b85b4917ee53ab55348aa2bc3eee1870 ====================================================================== Commit 3/3: 9bb2f9b552015c8a03e5f62015e15848c09f8fa0 ====================================================================== IMPALA-15282: Export Substrait plan from Calcite planner (prototype) Best-effort emit a Substrait plan protobuf file to disk as a side effect of query planning, when a query runs through the Calcite planner (--planner=CALCITE). Intended for a downstream external execution engine to pick up for pipelining; query option gating, an EXPLAIN variant, an execution short-circuit, and the handoff mechanism to that engine are explicitly out of scope here. - New flag --substrait_plan_export_dir (empty by default = disabled), plumbed through the usual BE gflag -> thrift -> BackendConfig path (be/src/service/impala-server.cc, common/thrift/BackendGflags.thrift, be/src/util/backend-gflag-util.cc, BackendConfig.java). - SubstraitPlanExporter converts the plan at Impala's pre-Impala-conversion Calcite seam via Isthmus. An export either succeeds or is rejected with a reason; it never returns a half-converted plan. Every fractional-second `precision` field in the resulting proto is clamped to 9 (nanoseconds) before it is returned: Isthmus copies Calcite's TIMESTAMP precision (ImpalaTypeSystemImpl.MAX_TIMESTAMP_PRECISION = 15) straight through, and 15 is not one of the five values Substrait's own spec defines for that field, so every plan with a TIMESTAMP-typed expression failed to load in a spec-compliant reader (verified against DuckDB's Substrait reader) before this. A real fix belongs in ImpalaTypeSystemImpl itself; clamping at the export boundary is the narrowest fix for this prototype. - SubstraitOperatorMappings teaches Isthmus about Impala-specific operator shapes it does not otherwise resolve, including YEAR(timestamp): Impala routes YEAR through ImpalaOperatorTable.USE_IMPALA_OPERATOR rather than Calcite's own EXTRACT operator, so Isthmus's stock EXTRACT mapping never sees the call. Mapped to Substrait's extract:req_pts. - SubstraitExportUtil is the call site's integration point: no-ops if the flag is unset, catches Throwable so export can never affect a real query, and logs the outcome (DEBUG on rejection, WARN on unexpected failure, INFO on success). - CalciteOptimizer exposes the seam plan via getSeamPlan() so CalciteSingleNodePlanner can export it after optimize() without re-running the optimization pipeline. - calcite-planner/pom.xml: the protobuf-java 4.x pin used to be test-scoped; it now needs to hold on the production classpath too, since the exporter ships as part of the module. Testing: - tests/custom_cluster/test_substrait_export.py runs all 22 TPC-H queries through the Calcite planner and confirms each exports a Substrait plan (export happens at plan time, independent of whether the query then executes successfully), plus a test that the flag is off by default. - Verified out of band (not part of this change): exported plans for 19 of the 22 TPC-H queries checked out both semantically (same base tables, same output columns) against an independent reference plan generated by upstream substrait-java's own Isthmus CLI, and at the row level (DuckDB executing the exported plan against the raw TPC-H data returns the same rows as Impala executing the same query). The remaining 3 queries in that reference corpus use standard SQL EXTRACT(unit FROM x) syntax, which does not parse under Impala's Calcite grammar at all (a separate, pre-existing parser gap, not touched by this change); Impala silently falls back to the original planner for those, so the query still succeeds but nothing is exported. One further query (Q5) was not verified at the row level: DuckDB's Substrait reader executes a plan's join order as given rather than re-optimizing it, and this query's 6-way join was too expensive to execute that way at scale. Change-Id: I90a55a55e8416389847841a8495379759acd4ed7 --- M be/src/service/impala-server.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/service/BackendConfig.java M java/calcite-planner/pom.xml M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteSingleNodePlanner.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/SubstraitExportUtil.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/SubstraitOperatorMappings.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/SubstraitPlanExporter.java 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 A tests/custom_cluster/test_substrait_export.py 14 files changed, 1,829 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/24811/1 -- To view, visit http://gerrit.cloudera.org:8080/24811 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I90a55a55e8416389847841a8495379759acd4ed7 Gerrit-Change-Number: 24811 Gerrit-PatchSet: 1 Gerrit-Owner: Jiyoung Yoo <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]>
