Fucun Chu has posted comments on this change. ( http://gerrit.cloudera.org:8080/17186 )
Change subject: IMPALA-10581: Implement ds_theta_intersect_f() function ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/17186/4/be/src/exprs/datasketches-functions-ir.cc File be/src/exprs/datasketches-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/17186/4/be/src/exprs/datasketches-functions-ir.cc@194 PS4, Line 194: datasketches::compact_theta_sketch sketch = intersection_sketch.get_result(); > Please add more comment about the use cases when this could return false. a Done http://gerrit.cloudera.org:8080/#/c/17186/4/be/src/exprs/datasketches-functions-ir.cc@195 PS4, Line 195: riali > typo: theta Done http://gerrit.cloudera.org:8080/#/c/17186/4/be/src/exprs/datasketches-functions-ir.cc@223 PS4, Line 223: if (serialized_sketch.is_null || serialized_sketch.len == 0) return BigIntVal::null(); > This comment is not needed Done http://gerrit.cloudera.org:8080/#/c/17186/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/17186/4/testdata/workloads/functional-query/queries/QueryTest/datasketches-theta.test@560 PS4, Line 560: 0 > I miss 2 tests here: Done -- To view, visit http://gerrit.cloudera.org:8080/17186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I335eada00730036d5433775cfe673e0e4babaa01 Gerrit-Change-Number: 17186 Gerrit-PatchSet: 5 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: Thu, 25 Mar 2021 15:19:17 +0000 Gerrit-HasComments: Yes
