Aleksandr Efimov has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24752
Change subject: IMPALA-15306: Carry Calcite's estimates into the Substrait plan ...................................................................... IMPALA-15306: Carry Calcite's estimates into the Substrait plan A plan leaves the exporter saying nothing about how much data any relation is expected to produce. Isthmus derives no statistics of its own, so a consumer planning or costing the query has to guess. Substrait has a place for this on every relation, and Calcite has already worked the numbers out by the time the plan reaches the seam, so the two only need connecting. SubstraitRelVisitor.apply is public and every relation goes through it, so a subclass can attach RelMetadataQuery's row count and average row size to what it returns. Nothing is asserted that Substrait does not already treat as approximate: its own words for the field are "estimated statistics" and "the estimated row count". That matters here, because Impala's row count is a real one from COMPUTE STATS when the table has been analysed and an estimate derived from file sizes when it has not, and CalciteTable hands both over through the same getRowCount. Writing either into a field declared as an estimate says nothing untrue; writing them into a field that promised a fact would. The estimates are attached to what reaches apply rather than to the Calcite node they came from. Isthmus drops a projection that only renames, returning its input instead, and rewrites an aggregate before the visitor sees it, so binding to the returned relation keeps the numbers on the relation a consumer will read. This is what needed substrait-java 0.101.0: Rel.withHint arrived there, and 0.100.0 can read a hint but has no type-agnostic way to set one. Testing, in a local cluster on Linux, the five classes in one run, 41 tests, 0 failures, 0 errors: - SubstraitRoundTripTest: 3 tests. The added one checks the exported relation carries an estimate and that the numbers are the ones Calcite gives for the same node, so a wrong wiring shows up as a mismatch rather than as a plausible figure - SubstraitPlanExporterTest: 22 tests - SubstraitTpcdsExportTest: 1 test over 110 queries - CalciteOptimizerTest and CalciteSubstraitCompatibilityTest, neither of which this patch touches: 11 and 4 tests. The second asserts on the protobuf a consumer reads, so it is also the check that adding hints disturbs nothing already there - Dropping the visitor override fails the added test, so it is not vacuous - git diff --check Change-Id: Id581d390fe08b7034327c69899c6b1bf492c99c2 Assisted-by: Claude Opus 5 (Anthropic) --- M 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/SubstraitPlanStatistics.java M java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitRoundTripTest.java 3 files changed, 127 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/24752/1 -- To view, visit http://gerrit.cloudera.org:8080/24752 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id581d390fe08b7034327c69899c6b1bf492c99c2 Gerrit-Change-Number: 24752 Gerrit-PatchSet: 1 Gerrit-Owner: Aleksandr Efimov <[email protected]>
