Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/17088 )
Change subject: IMPALA-10520: Implement ds_theta_intersect() function ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Thanks for this patch! In overall this looks great, I just had some minor comments. http://gerrit.cloudera.org:8080/#/c/17088/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17088/3//COMMIT_MSG@14 PS3, Line 14: an nit: not needed http://gerrit.cloudera.org:8080/#/c/17088/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/17088/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test@271 PS3, Line 271: and checks if the intersection : # produces the same result as if these sketches were used separately to get the estimates Could you add tests that cover the second part of this sentence so that we can sew what ds_theta_intersect() gives when processing the sketches separately (and to see if they in fact match with the results of this test)? -- To view, visit http://gerrit.cloudera.org:8080/17088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I80e68c2151c4604f0386d3dfb004c82b10293f97 Gerrit-Change-Number: 17088 Gerrit-PatchSet: 3 Gerrit-Owner: Fucun Chu <chufu...@hotmail.com> Gerrit-Reviewer: Fucun Chu <chufu...@hotmail.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 02 Mar 2021 10:13:03 +0000 Gerrit-HasComments: Yes