[Impala-ASF-CR] IMPALA-10538: [DOCS] Document the newly added argument

["Tamas Mate (Code Review)" <ge...@cloudera.org>]

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

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


Patch Set 1:

(4 comments)

Hi Shajini, thank you for this doc update. I had a few observations/questions on the doc change and some on commit.

Impala commit messages are in general complete sentences and describe what the commit contains. Unfortunately, this part does not have anchor, so can not link directly, but at the end of the 'Fix' there is a short paragraph about the styling.
https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala

http://gerrit.cloudera.org:8080/#/c/17131/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17131/1//COMMIT_MSG@7
PS1, Line 7: argument
Could you change this to "NDV parameter"?
'NDV' to be more specific and 'parameter' because the function's definition has parameters, these called arguments when referring to then when the function is called.


http://gerrit.cloudera.org:8080/#/c/17131/1//COMMIT_MSG@10
PS1, Line 10: hm
This line is a bit long.


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

http://gerrit.cloudera.org:8080/#/c/17131/1/docs/topics/impala_ndv.xml@51
PS1, Line 51:     <p> The argument <codeph>scale</codeph> must be an integer and can be in the range from 1 to 10
            :       and maps to a precision used by the HLL algorithm with the following mapping formula: </p>
            : 
            :     <p><codeblock>precision = scale + 8</codeblock></p>
I think an example at the end of the page could greatly help how the scale parameter affects the end result of NDV.


http://gerrit.cloudera.org:8080/#/c/17131/1/docs/topics/impala_ndv.xml@69
PS1, Line 69:       Without the secondary argument, all the syntax and semantics of the NDV function are
            :       preserved. The precision, which determines the total number of different estimators in the HLL
            :       algorithm, will be still 10.
This looks odd in a user documentation, we should probably just share a default value, if my understanding is right 10 is the default.



-- 
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: 1
Gerrit-Owner: Shajini Thayasingh <st...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Sun, 07 Mar 2021 15:36:31 +0000
Gerrit-HasComments: Yes