You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org> on 2026/09/30 17:18:08 UTC
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Mihaly Szjatinya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24982
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
COMPUTE STATS will write column NDV statistics as Iceberg Puffin
apache-datasketches-theta-v1 blobs (IMPALA-15004). For cross-engine
interop (Trino, Spark) each value must be hashed into the sketch using
Iceberg Appendix D single-value serialization (int=4B, float=4B,
date=4B, boolean=1B).
Impala's ds_theta_sketch() hashes values via the DataSketches typed
update() overloads, which widen int/date/boolean to 8 bytes and float to
double. This matches Hive's datasketches UDF but not the Iceberg Puffin
spec, so INT/FLOAT/DATE/BOOLEAN sketches would not interoperate with
Trino/Spark. Changing ds_theta_sketch() directly would break byte-level
interop with Hive-produced sketches (datasketches-theta.test).
This patch adds an internal-only aggregate flavor,
ds_theta_sketch_iceberg, that encodes values with Iceberg Appendix D
serialization. COMPUTE STATS (IMPALA-15004) uses it when writing Puffin
NDV blobs; the user-facing ds_theta_sketch() keeps its Hive-compatible
encoding.
- BE: added AggregateFunctions::DsThetaUpdateIceberg<T> with Appendix D
specialisations for BOOLEAN (1 byte), INT (4-byte LE), FLOAT (4-byte
LE) and DATE (4-byte LE). The other Iceberg column types (BIGINT,
DOUBLE, STRING, TIMESTAMP, DECIMAL) reuse DsThetaUpdate, which already
encodes identically.
- FE: registered the internal ds_theta_sketch_iceberg aggregate via a
new DS_THETA_ICEBERG_UPDATE_SYMBOL map in BuiltinsDb.java.
TINYINT/SMALLINT are intentionally omitted: Iceberg has no such column
types, and a stray narrow value would implicitly widen to INT and get
the correct encoding.
Testing:
- datasketches-theta.test: added a section that sketches the same values
as INT and BIGINT with both flavors and compares them with
ds_theta_exclude;
Regression:
- datasketches-theta.test
- aggregate-functions-test.cc DsThetaSketch.*:
Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Assisted-by: Claude Opus 4.8 (GitHub Copilot)
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
4 files changed, 133 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24982/1
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/24263/ : 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/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Sep 2026 17:38:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Hello Peter Rozsa, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24982
to look at the new patch set (#5).
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
COMPUTE STATS will write column NDV statistics as Iceberg Puffin
apache-datasketches-theta-v1 blobs (IMPALA-15004). For cross-engine
interop (Trino, Spark) each value must be hashed into the sketch using
Iceberg Appendix D single-value serialization.
Impala's ds_theta_sketch() hashes values via the DataSketches typed
update() overloads, which widen INT to an 8-byte integer and FLOAT to a
double. This matches Hive's datasketches UDF but not the Iceberg Puffin
spec. ds_theta_sketch() also does not support BOOLEAN, DATE, TIMESTAMP
and DECIMAL, so it cannot be used for those column types at all.
This patch adds an undocumented aggregate flavor,
ds_theta_sketch_iceberg(), that hashes every Iceberg column type
Impala computes NDV for with the Appendix D encoding. COMPUTE STATS
(IMPALA-15004) uses it when writing Puffin NDV blobs. The user-facing
ds_theta_sketch() is unchanged.
Type | ds_theta_sketch() | ds_theta_sketch_iceberg()
----------+-----------------------+---------------------------
BOOLEAN | not supported | 1 byte
INT | widened to 8 bytes | 4-byte LE
BIGINT | 8-byte LE | 8-byte LE (same symbol)
FLOAT | widened to double | 4-byte LE
DOUBLE | 8-byte LE | 8-byte LE (same symbol)
STRING | raw bytes | raw bytes (same symbol)
DATE | not supported | 4-byte LE day count
TIMESTAMP | not supported | 8-byte LE micros since epoch
DECIMAL | not supported | unscaled, BE two's-complement,
| | minimum bytes
TINYINT and SMALLINT are not registered for ds_theta_sketch_iceberg(),
as Iceberg has no such column types; such values are implicitly cast to
INT and get its encoding.
TIMESTAMP is the only lossy conversion: Impala timestamps have
nanosecond precision and are floored to microseconds, so two timestamps
that differ only below the microsecond are counted as one value. This
does not affect Iceberg, whose timestamp type has microsecond precision.
Changes:
- BE: added AggregateFunctions::DsThetaUpdateIceberg<T> with Appendix D
specialisations for BOOLEAN, INT, FLOAT, DATE, TIMESTAMP and DECIMAL,
sharing a DsThetaUpdateRaw() helper. DECIMAL handles all three backing
types (val4/val8/val16) based on the precision of the argument type.
- BE: extracted the minimal two's-complement trimming from
ColumnStats<DecimalValue>::SerializeIcebergSingleValue (Parquet
Iceberg stats) into BitUtil::TrimSignExtension() and reused it for
DECIMAL.
- FE: registered ds_theta_sketch_iceberg() in BuiltinsDb.java via a
new DS_THETA_ICEBERG_UPDATE_SYMBOL map; BIGINT, DOUBLE and STRING
reuse the ds_theta_sketch() update symbols.
Testing:
- datasketches-theta.test: distinct counts and exact-byte checks
against the expected Appendix D encoding for every supported type,
and a comparison with ds_theta_sketch().
- aggregate-functions-test.cc:
DsThetaSketch.IcebergDataToSketchTimestamp for the TIMESTAMP
conversion, including pre-epoch and sub-microsecond values.
Regression:
- test_datasketches.py::TestDatasketches::test_theta: existing
ds_theta_sketch() behaviour, including Hive-written sketches.
- aggregate-functions-test.cc DsThetaSketch.*: theta UDA lifecycle.
- serialize-single-value-test.cc: Parquet Iceberg single-value
serialization, refactored to use BitUtil::TrimSignExtension().
- bit-util-test.cc: BitUtil helpers.
Assisted-by: Claude Opus 4.8 (GitHub Copilot)
Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
---
M be/src/exec/parquet/parquet-column-stats.inline.h
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions-test.cc
M be/src/exprs/aggregate-functions.h
M be/src/util/bit-util.h
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
7 files changed, 405 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24982/5
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 5:
Swapped the order with IMPALA-15106 as suggested, so this change is now standalone on master and sufficient for IMPALA-15004. IMPALA-15106 is now rebased on this one and stays functional in case we need it later.
Reframed both issues description.
TIMESTAMP and DECIMAL moved here as DsThetaUpdateIceberg specialisations, so ds_theta_sketch_iceberg() covers every Iceberg type we compute NDV for, all with the Appendix D encoding. ds_theta_sketch() is unchanged.
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Oct 2026 13:35:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG@26
PS5, Line 26:
> optional: UUID could be added, it is simply bytewise hash as Impala already
Done.
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG@32
PS5, Line 32: FLOAT | widened to double | 4-byte LE
> Claude found a difference here:
Good catch, Trino also doesn't normalize. Done.
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG@46
PS5, Line 46: TIMESTAMP is the only lossy conversion: Impala timestamps have
> It could be noted that TIMESTAMP in Iceberg is always timestamp without tim
Done, added a note. TIMESTAMP is hashed without time zone conversion, and COMPUTE STATS (IMPALA-15004) will run the sketch query with TIMEZONE=UTC. That also applies to 'timestamptz' columns written by other engines, which Impala can read and converts to the session time zone.
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 6
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 31 Oct 2026 21:16:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/24982/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/24982/2//COMMIT_MSG@22
PS2, Line 22: internal-only
Is this actually hidden from the users? The test suggest to me that it isn't.
http://gerrit.cloudera.org:8080/#/c/24982/2/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/24982/2/be/src/exprs/aggregate-functions-ir.cc@2799
PS2, Line 2799: DCHECK(!dst->is_null);
: DCHECK_EQ(dst->len, sizeof(agg_state));
: auto agg_state_ptr = reinterpret_cast<agg_state*>(dst->ptr);
: DCHECK_EQ(agg_state_ptr->first, agg_phase::UPDATE);
: auto sketch_ptr =
: reinterpret_cast<datasketches::update_theta_sketch*>(agg_state_ptr->second);
: int32_t val = src.val;
: sketch_ptr->update(&val, sizeof(val));
nit: create a helper that does most of this? Besides the type (int32 here) and how to get the value it always seems to be the same
http://gerrit.cloudera.org:8080/#/c/24982/2/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
File fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java:
http://gerrit.cloudera.org:8080/#/c/24982/2/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java@459
PS2, Line 459: // Update symbols for the internal Iceberg (Appendix D) theta flavor used by
nit: comments are bit verbose
The details could be written down at one place, for example at AggregateFunctions::DsThetaUpdateIceberg, and others could refer to it
http://gerrit.cloudera.org:8080/#/c/24982/2/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
File testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test:
http://gerrit.cloudera.org:8080/#/c/24982/2/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test@622
PS2, Line 622: ds_theta_estimate(ds_theta_exclude(d_int, d_bigint)),
: ds_theta_estimate(ds_theta_exclude(i_int, i_bigint))
Add same check for other types?
Btw is it really needed to create a table for this? We could collect the sketch and check estimates in one query.
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Oct 2026 15:42:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/24497/ : 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/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Oct 2026 15:09:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
COMPUTE STATS will write column NDV statistics as Iceberg Puffin
apache-datasketches-theta-v1 blobs (IMPALA-15004). For cross-engine
interop (Trino, Spark) each value must be hashed into the sketch using
Iceberg Appendix D single-value serialization (int=4B, float=4B,
date=4B, boolean=1B).
Impala's ds_theta_sketch() hashes values via the DataSketches typed
update() overloads, which widen int/date/boolean to 8 bytes and float to
double. This matches Hive's datasketches UDF but not the Iceberg Puffin
spec, so INT/FLOAT/DATE/BOOLEAN sketches would not interoperate with
Trino/Spark. Changing ds_theta_sketch() directly would break byte-level
interop with Hive-produced sketches (datasketches-theta.test).
This patch adds an internal-only aggregate flavor,
ds_theta_sketch_iceberg, that encodes values with Iceberg Appendix D
serialization. COMPUTE STATS (IMPALA-15004) uses it when writing Puffin
NDV blobs; the user-facing ds_theta_sketch() keeps its Hive-compatible
encoding.
- BE: added AggregateFunctions::DsThetaUpdateIceberg<T> with Appendix D
specialisations for BOOLEAN (1 byte), INT (4-byte LE), FLOAT (4-byte
LE) and DATE (4-byte LE). The other Iceberg column types (BIGINT,
DOUBLE, STRING, TIMESTAMP, DECIMAL) reuse DsThetaUpdate, which already
encodes identically.
- FE: registered the internal ds_theta_sketch_iceberg aggregate via a
new DS_THETA_ICEBERG_UPDATE_SYMBOL map in BuiltinsDb.java.
TINYINT/SMALLINT are intentionally omitted: Iceberg has no such column
types, and a stray narrow value would implicitly widen to INT and get
the correct encoding.
Testing:
- datasketches-theta.test: added a section that sketches the same values
as INT and BIGINT with both flavors and compares them with
ds_theta_exclude;
Regression:
- datasketches-theta.test
- aggregate-functions-test.cc DsThetaSketch.*:
Assisted-by: Claude Opus 4.8 (GitHub Copilot)
Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
4 files changed, 133 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24982/2
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/24742/ : 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/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Oct 2026 13:44:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG@26
PS5, Line 26: Type
optional: UUID could be added, it is simply bytewise hash as Impala already stores them
there is no COMPUTE STATS for UUID yet, but it may come soon and having ds_theta_sketch_iceberg available would make it trivial to integrate with Puffin files
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG@32
PS5, Line 32: DOUBLE | 8-byte LE | 8-byte LE (same symbol)
Claude found a difference here:
datasketches normalizes doubles, which unifies different NaNs and -0/0 before hashing. Iceberg's Appandix D says to hash double's bytes as they are.
An example for the difference:
create table weird_doubles (d double);
insert into weird_doubles values (cast("nan" as double)), (cast("-nan" as double)), (cast("-0" as double)), (cast("0" as double));
select ndv(d) from weird_doubles; -- returns 4
select ds_theta_estimate(ds_theta_sketch(d)) from weird_doubles; -- returns 2
Honestly I think few people care about these values (and I also don't) - NaN is common but -0 and -NaN just cause trouble, but fixing this now would save us from AI finding this issue again. Hive also normalizes them, so an Iceberg specific function could be added for DOUBLE that hash as byte array, similarly to float.
http://gerrit.cloudera.org:8080/#/c/24982/5//COMMIT_MSG@46
PS5, Line 46: does not affect Iceberg, whose timestamp type has microsecond precision.
It could be noted that TIMESTAMP in Iceberg is always timestamp without timezone, so no timezone conversion needs to be considered.
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Oct 2026 16:56:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/24772/ : 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/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 6
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 31 Oct 2026 21:44:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/24274/ : 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/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Oct 2026 07:07:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
COMPUTE STATS will write column NDV statistics as Iceberg Puffin
apache-datasketches-theta-v1 blobs (IMPALA-15004). For cross-engine
interop (Trino, Spark) each value must be hashed into the sketch using
Iceberg Appendix D single-value serialization (int=4B, float=4B,
date=4B, boolean=1B).
Impala's ds_theta_sketch() hashes values via the DataSketches typed
update() overloads, which widen int/date/boolean to 8 bytes and float to
double. This matches Hive's datasketches UDF but not the Iceberg Puffin
spec, so INT/FLOAT/DATE/BOOLEAN sketches would not interoperate with
Trino/Spark. Changing ds_theta_sketch() directly would break byte-level
interop with Hive-produced sketches (datasketches-theta.test).
This patch adds an undocumented aggregate flavor,
ds_theta_sketch_iceberg, that encodes values with Iceberg Appendix D
serialization. COMPUTE STATS (IMPALA-15004) uses it when writing Puffin
NDV blobs; the user-facing ds_theta_sketch() keeps its Hive-compatible
encoding.
- BE: added AggregateFunctions::DsThetaUpdateIceberg<T> with Appendix D
specialisations for BOOLEAN (1 byte), INT (4-byte LE), FLOAT (4-byte
LE) and DATE (4-byte LE). The other Iceberg column types (BIGINT,
DOUBLE, STRING, TIMESTAMP, DECIMAL) reuse DsThetaUpdate, which already
encodes identically.
- FE: registered the internal ds_theta_sketch_iceberg aggregate via a
new DS_THETA_ICEBERG_UPDATE_SYMBOL map in BuiltinsDb.java.
TINYINT/SMALLINT are intentionally omitted: Iceberg has no such column
types, and a stray narrow value would implicitly widen to INT and get
the correct encoding.
Testing:
- datasketches-theta.test: added a section that sketches the same values
as INT and BIGINT with both flavors and compares them with
ds_theta_exclude;
Regression:
- datasketches-theta.test
- aggregate-functions-test.cc DsThetaSketch.*:
Assisted-by: Claude Opus 4.8 (GitHub Copilot)
Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
4 files changed, 150 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24982/3
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/24982/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/24982/2//COMMIT_MSG@22
PS2, Line 22: This patch adds an undocumented aggregate flavor,
> Is this actually hidden from the users? The test suggest to me that it isn'
Ack, maybe 'undocumented' would be better. It's the same status as ndv_no_finalize().
http://gerrit.cloudera.org:8080/#/c/24982/2/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/24982/2/be/src/exprs/aggregate-functions-ir.cc@2806
PS2, Line 2806: static void DsThetaUpdateRaw(const void* bytes, size_t len, StringVal* dst) {
> nit: create a helper that does most of this? Besides the type (int32 here)
Done - added the DsThetaUpdateRaw(bytes, len, dst) helper.
http://gerrit.cloudera.org:8080/#/c/24982/2/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
File fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java:
http://gerrit.cloudera.org:8080/#/c/24982/2/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java@459
PS2, Line 459: // Update symbols for ds_theta_sketch_iceberg(). BOOLEAN, INT, FLOAT and DATE need the
> nit: comments are bit verbose
Done - moved the details to the AggregateFunctions::DsThetaUpdateIceberg declaration in aggregate-functions.h and left a short pointer to it here. Trimmed the comment at the registration site as well.
http://gerrit.cloudera.org:8080/#/c/24982/2/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
File testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test:
http://gerrit.cloudera.org:8080/#/c/24982/2/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test@623
PS2, Line 623: ---- QUERY
> Add same check for other types?
Done on both counts. The table is gone - a CTE does the job.
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Oct 2026 14:42:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/24982 )
Change subject: IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
......................................................................
IMPALA-15459: Add Iceberg-encoded theta sketch flavor for Puffin NDV stats
COMPUTE STATS will write column NDV statistics as Iceberg Puffin
apache-datasketches-theta-v1 blobs (IMPALA-15004). For cross-engine
interop (Trino, Spark) each value must be hashed into the sketch using
Iceberg Appendix D single-value serialization.
Impala's ds_theta_sketch() hashes values via the DataSketches typed
update() overloads, which widen INT to an 8-byte integer and FLOAT to a
double, and canonicalize DOUBLE -0.0 and NaN values. This matches Hive's
datasketches UDF but not the Iceberg Puffin spec. ds_theta_sketch() also
does not support BOOLEAN, DATE, TIMESTAMP, DECIMAL and UUID, so it
cannot be used for those column types at all.
This patch adds an undocumented aggregate flavor,
ds_theta_sketch_iceberg(), that hashes every Iceberg column type
Impala computes NDV for with the Appendix D encoding. COMPUTE STATS
(IMPALA-15004) uses it when writing Puffin NDV blobs. The user-facing
ds_theta_sketch() is unchanged.
Type | ds_theta_sketch() | ds_theta_sketch_iceberg()
----------+-----------------------+---------------------------
BOOLEAN | not supported | 1 byte
INT | widened to 8 bytes | 4-byte LE
BIGINT | 8-byte LE | 8-byte LE (same symbol)
FLOAT | widened to double | 4-byte LE
DOUBLE | 8-byte LE, -0.0 and | 8-byte LE raw bits
| NaNs canonicalized |
STRING | raw bytes | raw bytes (same symbol)
DATE | not supported | 4-byte LE day count
TIMESTAMP | not supported | 8-byte LE micros since epoch
DECIMAL | not supported | unscaled, BE two's-complement,
| | minimum bytes
UUID | not supported | 16 bytes BE (STRING symbol)
TINYINT and SMALLINT are not registered for ds_theta_sketch_iceberg(),
as Iceberg has no such column types; such values are implicitly cast to
INT and get its encoding.
TIMESTAMP is the only lossy conversion: Impala timestamps have
nanosecond precision and are floored to microseconds, so two timestamps
that differ only below the microsecond are counted as one value. This
does not affect Iceberg, whose timestamp type has microsecond precision.
TIMESTAMP values are hashed without time zone conversion. COMPUTE
STATS (IMPALA-15004) runs the sketch query with TIMEZONE=UTC, so that
'timestamptz' columns, which the scanner converts to the session time
zone, are hashed in UTC as well.
UUID columns are not covered by COMPUTE STATS yet, but registering
ds_theta_sketch_iceberg() for them makes it trivial to add them later.
Changes:
- BE: added AggregateFunctions::DsThetaUpdateIceberg<T> with Appendix D
specialisations for BOOLEAN, INT, FLOAT, DOUBLE, DATE, TIMESTAMP and
DECIMAL, sharing a DsThetaUpdateRaw() helper. DECIMAL handles all
three backing types (val4/val8/val16) based on the precision of the
argument type.
- BE: extracted the minimal two's-complement trimming from
ColumnStats<DecimalValue>::SerializeIcebergSingleValue (Parquet
Iceberg stats) into BitUtil::TrimSignExtension() and reused it for
DECIMAL.
- FE: registered ds_theta_sketch_iceberg() in BuiltinsDb.java via a
new DS_THETA_ICEBERG_UPDATE_SYMBOL map; BIGINT, STRING and UUID reuse
the ds_theta_sketch() update symbols for BIGINT and STRING.
Testing:
- datasketches-theta.test: distinct counts and exact-byte checks
against the expected Appendix D encoding for every supported type,
DOUBLE -0.0/NaN handling, and a comparison with ds_theta_sketch().
- aggregate-functions-test.cc:
DsThetaSketch.IcebergDataToSketchTimestamp for the TIMESTAMP
conversion, including pre-epoch and sub-microsecond values.
Regression:
- test_datasketches.py::TestDatasketches::test_theta: existing
ds_theta_sketch() behaviour, including Hive-written sketches.
- aggregate-functions-test.cc DsThetaSketch.*: theta UDA lifecycle.
- serialize-single-value-test.cc: Parquet Iceberg single-value
serialization, refactored to use BitUtil::TrimSignExtension().
- bit-util-test.cc: BitUtil helpers.
Assisted-by: Claude Opus 4.8 (GitHub Copilot)
Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
---
M be/src/exec/parquet/parquet-column-stats.inline.h
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions-test.cc
M be/src/exprs/aggregate-functions.h
M be/src/util/bit-util.h
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test
7 files changed, 477 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/24982/6
--
To view, visit http://gerrit.cloudera.org:8080/24982
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I78c76c38dffa9807eb13ad465afab0b80f31984a
Gerrit-Change-Number: 24982
Gerrit-PatchSet: 6
Gerrit-Owner: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>