Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24590 )
Change subject: IMPALA-9821: Change DataSketches functions to return BINARY ...................................................................... Patch Set 3: (9 comments) looks good, added a few comments about tests and commit message http://gerrit.cloudera.org:8080/#/c/24590/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24590/3//COMMIT_MSG@9 PS3, Line 9: When Hive writes DataSketches to ORC tables Is it just ORC? I assumed that this has nothing to do with file format, the sketches simply return BINARY type. http://gerrit.cloudera.org:8080/#/c/24590/3//COMMIT_MSG@61 PS3, Line 61: ORC As above, is it just ORC? http://gerrit.cloudera.org:8080/#/c/24590/3//COMMIT_MSG@74 PS3, Line 74: - All test_datasketches.py tests pass (5/5) nit: I think that this is redundant, we should assume that tests passed in a patch http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-cpc.test File testdata/workloads/functional-query/queries/QueryTest/datasketches-cpc.test: http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-cpc.test@a142 PS3, Line 142: see my comment in https://gerrit.cloudera.org/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-hll-hive-orc.test File testdata/workloads/functional-query/queries/QueryTest/datasketches-hll-hive-orc.test: http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-hll-hive-orc.test@5 PS3, Line 5: DATABASE I don't think that this is needed, the passed unique_database is used by default http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-hll.test File testdata/workloads/functional-query/queries/QueryTest/datasketches-hll.test: http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-hll.test@a141 PS3, Line 141: see my comment in https://gerrit.cloudera.org/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-kll.test File testdata/workloads/functional-query/queries/QueryTest/datasketches-kll.test: http://gerrit.cloudera.org:8080/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-kll.test@a6 PS3, Line 6: See my comment in https://gerrit.cloudera.org/#/c/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test http://gerrit.cloudera.org:8080/#/c/24590/3/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/24590/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test@a208 PS3, Line 208: : : : Why was this removed? Could still work after casting to BINARY. + same for other removed tests with "Unable to deserialize sketch" errr http://gerrit.cloudera.org:8080/#/c/24590/3/tests/query_test/test_datasketches.py File tests/query_test/test_datasketches.py: http://gerrit.cloudera.org:8080/#/c/24590/3/tests/query_test/test_datasketches.py@26 PS3, Line 26: _SKETCH_COLS_9 = ('ti binary, i binary, bi binary, f binary, d binary, ' Not in the scope of this batch, but it could be useful to have a query option that sets how create CREATE TABLE LIKE PARQUET works, and depending on its mode, create BINARY columns for Parquet columns not annotated as utf8. -- To view, visit http://gerrit.cloudera.org:8080/24590 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4a6b54089dd356e37257bc24adeb1eb98e82c25 Gerrit-Change-Number: 24590 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-Comment-Date: Mon, 27 Jul 2026 14:43:12 +0000 Gerrit-HasComments: Yes
