Aleksandr Efimov has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/24751


Change subject: IMPALA-15305: Export NDV as a Substrait approx_count_distinct
......................................................................

IMPALA-15305: Export NDV as a Substrait approx_count_distinct

NDV was left out of the mapping table added by IMPALA-15297 on the
grounds that approx_count_distinct is the obvious counterpart but the
rename would assert something about the estimator that a name alone does
not back.

Checking rather than assuming settles it. Substrait says of
approx_count_distinct that it calculates the approximate number of
distinct values "using HyperLogLog". Impala's ndv is HyperLogLog too,
sized by ComputeHllLengthFromScale. Both name the same algorithm, so the
mapping asserts nothing that is not already true of either side.

Map it, and wire an aggregate converter to carry it: the provider had
scalar and window converters until now.

The two-argument ndv, which sets the precision, needs nothing to stay
refused. Substrait has one variant of approx_count_distinct and it takes
one operand, so a call carrying a scale matches nothing and the export
says so. There is no way to drop the precision without noticing, and
nothing in the mapping arranges that, which is why it gets a test of its
own.

No TPC-DS query is affected: ndv is an Impala function rather than
standard SQL, so the corpus stays at 102 of 110. It is common in real
Impala queries, which is the reason to carry it.

Testing, in a local cluster on Linux, the five classes in one run,
40 tests, 0 failures, 0 errors:
- SubstraitPlanExporterTest: 22 tests, the two added ones covering the
  mapping and the refusal of the two-argument form
- SubstraitTpcdsExportTest: 1 test over 110 queries, unchanged at 102
- SubstraitRoundTripTest: 2 tests
- CalciteOptimizerTest and CalciteSubstraitCompatibilityTest, neither of
  which this patch touches: 11 and 4 tests
- Unwiring the aggregate converter fails the added mapping test, so it
  is not vacuous
- git diff --check

Change-Id: Ie54edf05b229a47662953721891df0f23adcd2ff
Assisted-by: Claude Opus 5 (Anthropic)
---
M 
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitOperatorMappings.java
M 
java/calcite-planner/src/test/java/org/apache/impala/calcite/service/SubstraitPlanExporterTest.java
2 files changed, 54 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/24751/1
--
To view, visit http://gerrit.cloudera.org:8080/24751
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie54edf05b229a47662953721891df0f23adcd2ff
Gerrit-Change-Number: 24751
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <[email protected]>

Reply via email to