Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/24889 )
Change subject: IMPALA-15106: Support missing types with theta sketches ...................................................................... Patch Set 3: (3 comments) Thanks for review! We already have a ds_data_sketches() Hive interop. Trino/Spark interop is in IMPALA-15004. See also the reply in thread. http://gerrit.cloudera.org:8080/#/c/24889/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24889/1//COMMIT_MSG@20 PS1, Line 20: TINYINT | yes | yes | already done : SMALLINT | yes | no | needs FE wiring only > It could be noted that these are not supported in Iceberg, as I assume that Good point. These will never be in Iceberg table. But ds_data_sketches() can still support those for consistency. Added a note. http://gerrit.cloudera.org:8080/#/c/24889/1//COMMIT_MSG@47 PS1, Line 47: NOTE: > There should be some test that checks the exact output compared to some ref This ticket is a prerequisite for 15004. It prepares these functions to be suitable for Puffin Spec and for Appendix D. A meaningful interop here that I can think of would be to compare against analogous functions in other engines. If my research is correct, out of Hive, Spark and Trino only Hive provides the external ds_theta_sketch() to compare against. And we already have such a test https://github.com/apache/Impala/blob/master/tests/query_test/test_datasketches.py#L70. On the other hand, Spark and Trino read/write puffin, follow Spec + Appendix, but don't expose the UDF's. For those the interop is in 15004. Note: For a number of types there is a difference between ds_data_sketches() in Hive and how Trino/Spark generate the sketches for puffin. For both interops to work, filed IMPALA-15459 http://gerrit.cloudera.org:8080/#/c/24889/1/be/src/exprs/aggregate-functions-test.cc File be/src/exprs/aggregate-functions-test.cc: http://gerrit.cloudera.org:8080/#/c/24889/1/be/src/exprs/aggregate-functions-test.cc@209 PS1, Line 209: // Three distinct timestamps (0 s, 1 s, 2 s since epoch). > It would be nice to also have a negative interval. Ack -- To view, visit http://gerrit.cloudera.org:8080/24889 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I549a51ba96d0b9be2052a9350efe2096c1755816 Gerrit-Change-Number: 24889 Gerrit-PatchSet: 3 Gerrit-Owner: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Wed, 30 Sep 2026 17:10:38 +0000 Gerrit-HasComments: Yes
