Dimitris Tsirogiannis has posted comments on this change. Change subject: [DOCS] Major update to Impala + Kudu page ......................................................................
Patch Set 11: (1 comment) John, sending a new patch without indicating which comments have been addressed doesn't really help the review process. Can you please respond to the comments? http://gerrit.cloudera.org:8080/#/c/5649/11/docs/topics/impala_create_table.xml File docs/topics/impala_create_table.xml: PS11, Line 81: CREATE [EXTERNAL] TABLE [IF NOT EXISTS] [<varname>db_name</varname>.]<varname>table_name</varname> : (<varname>col_name</varname> <varname>data_type</varname> : <ph rev="kudu IMPALA-3719">[<varname>kudu_column_attribute</varname> ...]</ph> : [COMMENT '<varname>col_comment</varname>'] : [, ...] : [PRIMARY KEY (<varname>col_name</varname>[, ...]] : ) : [PARTITIONED BY (<varname>col_name</varname> <varname>data_type</varname> [COMMENT '<varname>col_comment</varname>'], ...)] : [COMMENT '<varname>table_comment</varname>'] : [WITH SERDEPROPERTIES ('<varname>key1</varname>'='<varname>value1</varname>', '<varname>key2</varname>'='<varname>value2</varname>', ...)] : <ph rev="kudu">[PARTITION BY <varname>kudu_partition_clause</varname></ph> : [ : [ROW FORMAT <varname>row_format</varname>] [STORED AS <varname>file_format</varname>] : ] : [LOCATION '<varname>hdfs_path</varname>'] I think we can improve the syntax definition to avoid common mistakes and confusion from a user's point of view. For instance, instead of having [PARTITIONED BY...] and after 3 lines [PARTITION BY kudu...] we can replace them with something like: [partition_clause] partition_clause::= |hdfs_partition_clause |kudu_partition_clause hdfs_partition_clause ::= PARTITIONED BY .... kudu_partition_clause ::= PARTITION BY Also, now that I think of it, it doesn't make sense to merge the syntax for Kudu and non-kudu tables as they differ significantly. Todd, MJ what do you guys think? -- 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: 11 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
