Adam Tamas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16370 )

Change subject: IMPALA-10108: Implement ds_kll_stringify function
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16370/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16370/3//COMMIT_MSG@10
PS3, Line 10: stringified format
> Could you mention what does this stringified format look like? What informa
Done


http://gerrit.cloudera.org:8080/#/c/16370/3/be/src/exprs/datasketches-functions-ir.cc
File be/src/exprs/datasketches-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/16370/3/be/src/exprs/datasketches-functions-ir.cc@144
PS3, Line 144: const StringVal& ser
> nit: this param would fit in the previous line, right?
Right, done.


http://gerrit.cloudera.org:8080/#/c/16370/3/be/src/exprs/datasketches-functions.h
File be/src/exprs/datasketches-functions.h:

http://gerrit.cloudera.org:8080/#/c/16370/3/be/src/exprs/datasketches-functions.h@109
PS3, Line 109: c
> nit: an
Done


http://gerrit.cloudera.org:8080/#/c/16370/3/be/src/exprs/datasketches-functions.h@110
PS3, Line 110:       PMFCDF mode);
> Is this meant to be in the private section?
I missed that it went here after the merge, fixed.


http://gerrit.cloudera.org:8080/#/c/16370/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-kll.test
File 
testdata/workloads/functional-query/queries/QueryTest/datasketches-kll.test:

http://gerrit.cloudera.org:8080/#/c/16370/3/testdata/workloads/functional-query/queries/QueryTest/datasketches-kll.test@604
PS3, Line 604: row_regex: .*### KLL sketch summary:.*Epsilon.*Epsilon PMF.*Em
> What is the content of the sketch summary? Can't you assert on anything tha
An alternative would look like for the whole summary:
'### KLL sketch summary:\n   K              : 200\n   min K          : 200\n   
M              : 8\n   N              : 8\n   Epsilon        : 1.33%%\n   
Epsilon PMF    : 1.65%%\n   Empty          : false\n   Estimation mode: false\n 
  Levels         : 1\n   Sorted         : false\n   Capacity items : 200\n   
Retained items : 8\n   Storage bytes  : 64\n   Min value      : 0\n   Max value 
     : 1.1\n### End sketch summary\n'
Modified it so it look for more parts from the sketch summary.



--
To view, visit http://gerrit.cloudera.org:8080/16370
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I97f654a4838bf91e3e0bed6a00d78b2c7aa96f75
Gerrit-Change-Number: 16370
Gerrit-PatchSet: 4
Gerrit-Owner: Adam Tamas <[email protected]>
Gerrit-Reviewer: Adam Tamas <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 26 Aug 2020 16:20:10 +0000
Gerrit-HasComments: Yes

Reply via email to