Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/15746 )
Change subject: IMPALA-9631: Import HLL functionality from DataSketches ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/15746/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15746/2//COMMIT_MSG@7 PS2, Line 7: IMPALA-9631: Import HLL functionality from DataSketches > Do you plan to merge this patch as it is? I would consider it more of a POC Well, I don't see a difference of merging it now of waiting for other development work and start the review then. Btw, this unlock other steps under the umbrella Jira: https://issues.apache.org/jira/browse/IMPALA-9593 Additionally, waiting for the rest of the functionality means that splitting up these tasks has no point and we could have kept a single Jira for everything HLL related. So I'd take this as a non-POC review now and we still can make changes later on if we want to modify the build structure or drop the entire library for some reason. http://gerrit.cloudera.org:8080/#/c/15746/2//COMMIT_MSG@9 PS2, Line 9: This patch imports the functionality needed for HLL approximate > Can you add this information in a README.md? To the datasketches folder? Done http://gerrit.cloudera.org:8080/#/c/15746/2//COMMIT_MSG@11 PS2, Line 11: files into be/src/thirdparty/datasketches. Note, that the original > nit: wrap at 72 Done http://gerrit.cloudera.org:8080/#/c/15746/2/be/src/exprs/datasketches-test.cc File be/src/exprs/datasketches-test.cc: http://gerrit.cloudera.org:8080/#/c/15746/2/be/src/exprs/datasketches-test.cc@27 PS2, Line 27: algorith > typo: algorithm Done http://gerrit.cloudera.org:8080/#/c/15746/2/be/src/exprs/datasketches-test.cc@38 PS2, Line 38: std::stringstream sketch_stream1; : std::stringstream sketch_stream2; > I would prefer to use stringstream instead of files to make this faster/avo Done -- To view, visit http://gerrit.cloudera.org:8080/15746 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ca8e77dcbb6b6c3b1e3bca7ab57cb7d3c018bbf Gerrit-Change-Number: 15746 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Mon, 20 Apr 2020 15:26:40 +0000 Gerrit-HasComments: Yes
