Lars Volker has posted comments on this change.

Change subject: IMPALA-2522: Add doc for sortby() and clustered hints
......................................................................


Patch Set 4:

(10 comments)

Thanks for the change. I added comments inline.

http://gerrit.cloudera.org:8080/#/c/5655/4/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

PS4, Line 2639: minimizes
Technically this is incorrect now, since CLUSTERED reduces the number of 
concurrent files even more. Maybe rephrase to "reduces".


PS4, Line 2672: perform organize
This reads as if one word was superfluous


PS4, Line 2674: risking
to reduce the risk of an...


PS4, Line 2686: large insert operations
              :             that use dynamic partitioning
This surprises me. Maybe I'm missing something?


Line 3707:     <section id="kudu_common">
?


http://gerrit.cloudera.org:8080/#/c/5655/4/docs/topics/impala_hints.xml
File docs/topics/impala_hints.xml:

Line 87:       either the <codeph>/* */</codeph> or <codeph>--</codeph> 
notation.
We might want to state that the use of [] is discourage and will be deprecated 
in the next compatibility-breaking release of Impala.


PS4, Line 91: immediately before the hint name
It must only be present immediately before the hint list, i.e. the first hint 
name. For example /* +clustered,shuffle,sortby(a,b) */


Line 129: </codeblock>
we should have one example with multiple hints.


http://gerrit.cloudera.org:8080/#/c/5655/4/docs/topics/impala_insert.xml
File docs/topics/impala_insert.xml:

Line 67: hint_with_dashes ::= -- +SHUFFLE | -- +NOSHUFFLE <ph 
rev="IMPALA-4163">| -- +SORTBY(<varname>col</varname>, <varname>col</varname> 
...)]</ph>
Add CLUSTERED to the list?


Line 72:   (Note: with this hint format, the square brackets are part of the 
syntax.)
Maybe mention that this will deprecated in the next compatibility-breaking 
release here, too?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3c1da9a87ace361b096fa73d8504b2f54e75bed
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <[email protected]>
Gerrit-Reviewer: John Russell <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-HasComments: Yes

Reply via email to