Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/16645 )
Change subject: IMPALA-10279: Import CPC functionality from DataSketches ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/16645/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16645/6//COMMIT_MSG@11 PS6, Line 11: : First, I updated our existing snapshot of DataSketches to the : following commit:2b84e213067b681b696ec883d245ddf911790ff2 > All change files have been updated I checked manually the commits between the git hash we used previously and the one that you bumped our snapshot to. Apparently there are some changes that went into the repo since the 2 snapshots but aren't visible here in your patch. (at least I don't see the files that were changed in the intermittent commits being changed here) Example commits where either kll, hll or common was changed: https://github.com/apache/incubator-datasketches-cpp/commit/7cafbef53f475b966c1984dce395a940c6cc93f5 https://github.com/apache/incubator-datasketches-cpp/commit/3135dc4a71af05150bd8864aaf0ff803532a554f https://github.com/apache/incubator-datasketches-cpp/commit/608a741a4d0db9078feddc165cb080f5bb43e351 https://github.com/apache/incubator-datasketches-cpp/commit/fa9516767ffece0c8aa3246302135d6614ae2deb https://github.com/apache/incubator-datasketches-cpp/commit/c5be98a81c5947e5100868b90fb0f50aba4a9334 Are you sure all the files here are in the Impala repo (common/ hll/ kll) are updated to the desired snapshot? http://gerrit.cloudera.org:8080/#/c/16645/6/be/src/exprs/datasketches-test.cc File be/src/exprs/datasketches-test.cc: http://gerrit.cloudera.org:8080/#/c/16645/6/be/src/exprs/datasketches-test.cc@114 PS6, Line 114: } > The test code comes from the official example: thx! I just wanted to make sure that this result is deterministic and is the very same regardless of how many times we run it. Note, KLL wasn't deterministic and I had to check if the result is in a given range. http://gerrit.cloudera.org:8080/#/c/16645/7/be/src/thirdparty/datasketches/README.md File be/src/thirdparty/datasketches/README.md: http://gerrit.cloudera.org:8080/#/c/16645/7/be/src/thirdparty/datasketches/README.md@16 PS7, Line 16: https://github.com/apache/incubator-datasketches-cpp/ Could you provide a link here that points to the particular snapshot that you are upgrading to? e.g. https://github.com/apache/incubator-datasketches-cpp/tree/2b84e213067b681b696ec883d245ddf911790ff2 -- To view, visit http://gerrit.cloudera.org:8080/16645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e8d0c2e80df95fa84af82c64d493df9bbb34a8c Gerrit-Change-Number: 16645 Gerrit-PatchSet: 7 Gerrit-Owner: Fucun Chu <[email protected]> Gerrit-Reviewer: Fucun Chu <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 03 Nov 2020 16:33:09 +0000 Gerrit-HasComments: Yes
