impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
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:


Thanks for the change. I added comments inline.
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">
File docs/topics/impala_hints.xml:

Line 87:       either the <codeph>/* */</codeph> or <codeph>--</codeph>
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.
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
To unsubscribe, visit

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

View raw message