impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2522: Add doc for sortby() and clustered hints
Date Thu, 12 Jan 2017 12:00:09 GMT
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 <jrussell@cloudera.com>
Gerrit-Reviewer: John Russell <jrussell@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message