John Russell has posted comments on this change.

Change subject: [DOCS] Major update to Impala + Kudu page
......................................................................


Patch Set 13:

(8 comments)

New patch set coming any second.

http://gerrit.cloudera.org:8080/#/c/5649/13/docs/topics/impala_alter_table.xml
File docs/topics/impala_alter_table.xml:

PS13, Line 64: ADD [IF NOT EXISTS] | DROP [IF EXISTS] } PARTITION
Add another line here for ADD RANGE PARTITION.

(The ADD/DROP lines are already getting split apart as part of a different code 
review. That will happen here as part of resolving a merge conflict.)


PS13, Line 86: <ph rev="kudu"><varname>kudu_partition_spec</varname></ph>
Take this out of there because it doesn't apply to the basic ADD PARTITION and 
DROP PARTITION clauses.


http://gerrit.cloudera.org:8080/#/c/5649/13/docs/topics/impala_create_table.xml
File docs/topics/impala_create_table.xml:

PS13, Line 85: [PRIMARY KEY (<varname>col_name</varname>[, ...]]
> This doesn't belong here.
Done


PS13, Line 234: codeblock
Sprinkle some rev="kudu" eyecatchers on these Kudu-only codeblocks.


PS13, Line 244: <codeblock>CREATE TABLE [IF NOT EXISTS] 
<varname>db_name</varname>.]<varname>table_name</varname>
              :   [COMMENT '<varname>table_comment</varname>']
              :   STORED AS KUDU
              :   [TBLPROPERTIES 
('<varname>key1</varname>'='<varname>value1</varname>', 
'<varname>key2</varname>'='<varname>value2</varname>', ...)]
              : AS
              :   <varname>select_statement</varname></codeblock>
> This is not correct. You're missing the PRIMARY KEY and PARTITION BY clause
Done


PS13, Line 411: impala::username.kudu_t1
> nit: I don't think this is a good example. I would be really surprised if s
Done


http://gerrit.cloudera.org:8080/#/c/5649/13/docs/topics/impala_kudu.xml
File docs/topics/impala_kudu.xml:

PS13, Line 93: kudu.master_addresses
> That's the name of the table property. Can you verify that name of the impa
Done. Right, the config option is -kudu_master_hosts


PS13, Line 101: kudu.master_addresses
Same change to -kudu_master_hosts as in previous paragraph. Clean up the 
TBLPROPERTIES wording in this paragraph to reflect better the 
kudu.master_addresses param for that clause.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I76dcb948dab08532fe41326b22ef78d73282db2c
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <[email protected]>
Gerrit-Reviewer: Ambreen Kazi <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]>
Gerrit-Reviewer: John Russell <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to