Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17131 )

Change subject: IMPALA-10538: [DOCS] Document the newly added NDV parameter
......................................................................


Patch Set 3:

(5 comments)

Thanks for doing this, I have some suggestions/comments for making the docs 
clearer

http://gerrit.cloudera.org:8080/#/c/17131/3/docs/topics/impala_ndv.xml
File docs/topics/impala_ndv.xml:

http://gerrit.cloudera.org:8080/#/c/17131/3/docs/topics/impala_ndv.xml@49
PS3, Line 49: <codeblock>NDV([DISTINCT | ALL] <varname>expression</varname> 
[,scale])</codeblock>
Need to explain somewhere what scale is used if this parameter is  not 
specified.


http://gerrit.cloudera.org:8080/#/c/17131/3/docs/topics/impala_ndv.xml@51
PS3, Line 51:     <p> The argument <codeph>scale</codeph> must be an integer 
and can be in the range from 1 to 10
This seems a lot of detail for one optional argument. One way to deal with this 
might be to have some sort of heading like 'notes on the optional scale 
argument'.


http://gerrit.cloudera.org:8080/#/c/17131/3/docs/topics/impala_ndv.xml@52
PS3, Line 52:       and maps to a precision used by the HLL algorithm with the 
following mapping formula: </p>
Expand HLL or explain what it means


http://gerrit.cloudera.org:8080/#/c/17131/3/docs/topics/impala_ndv.xml@64
PS3, Line 64:       need of extra memory. For a given precision p, the amount 
of memory used by the HLL algorithm
Expand or explain HLL


http://gerrit.cloudera.org:8080/#/c/17131/3/docs/topics/impala_ndv.xml@69
PS3, Line 69:       When provided a scale of 10 against a total of 22 distinct 
data sets loaded into external
I don't understand this sentence, is there a part missing?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec8007b79afac59cdfb3984bb111806213c21c77
Gerrit-Change-Number: 17131
Gerrit-PatchSet: 3
Gerrit-Owner: Shajini Thayasingh <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tamas Mate <[email protected]>
Gerrit-Comment-Date: Wed, 31 Mar 2021 21:21:49 +0000
Gerrit-HasComments: Yes

Reply via email to