Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/17153 )
Change subject: IMPALA-10558: Implement ds_theta_exclude() function ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/17153/4/be/src/exprs/datasketches-common.cc File be/src/exprs/datasketches-common.cc: http://gerrit.cloudera.org:8080/#/c/17153/4/be/src/exprs/datasketches-common.cc@54 PS4, Line 54: bool DeserializeDsSketch( Could you please comment that this is a specialization of the template DeserializeDsSketch() for theta sketches? http://gerrit.cloudera.org:8080/#/c/17153/4/be/src/exprs/datasketches-functions-ir.cc File be/src/exprs/datasketches-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/17153/4/be/src/exprs/datasketches-functions-ir.cc@148 PS4, Line 148: // A is not null nit: I don't think this comment and the one below adds much. The comment above explains well the functionality here. http://gerrit.cloudera.org:8080/#/c/17153/3/be/src/exprs/datasketches-functions.h File be/src/exprs/datasketches-functions.h: http://gerrit.cloudera.org:8080/#/c/17153/3/be/src/exprs/datasketches-functions.h@74 PS3, Line 74: sketch. If it is not nit: "...sketches. If they are not..." http://gerrit.cloudera.org:8080/#/c/17153/4/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/17153/4/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test@398 PS4, Line 398: check that it is equal to the : # estimate of a. Does this mean that with this test A and B has no common items so the result of A-not-B equals to the cardinality of A? If yes, do you think it would make sense to add a test where there are some common items between the two input sets but not all the items are common? -- To view, visit http://gerrit.cloudera.org:8080/17153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I05119fd8c652c07ff248a99e44b0da3541e46ca3 Gerrit-Change-Number: 17153 Gerrit-PatchSet: 4 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: Wed, 17 Mar 2021 08:12:07 +0000 Gerrit-HasComments: Yes
