Laurel Hale has posted comments on this change.

Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned 
by"
......................................................................


Patch Set 1:

(1 comment)

> (1 comment)

http://gerrit.cloudera.org:8080/#/c/7080/1/docs/topics/impala_create_table.xml
File docs/topics/impala_create_table.xml:

Line 172:   <ph rev="2.9.0 IMPALA-4850">[COMMENT 
'<varname>table_comment</varname>']</ph>
> There seem to be a few things amiss.
Thanks for your feedback. I don't see the reference to the 
PARTITIONED BY clause in the section of sql-parser.cup you pasted above. 
I looked at sql-parser.cup and don't see anything about the 
PARTITIONED BY clause other than the following section which follows 
the "tbl_options" section you pasted above:
----------------------------------
partitioned_data_layout ::=
  partition_param_list:partition_params
  {: RESULT = TableDataLayout.createKuduPartitionedLayout(partition_params); :}
  | /* empty */
  {: RESULT = TableDataLayout.createEmptyLayout(); :}
  ;
partition_column_defs ::=
  KW_PARTITIONED KW_BY LPAREN column_def_list:col_defs RPAREN
  {: RESULT = col_defs; :}
  ;
// The PARTITION BY clause contains any number of HASH() clauses followed by 
exactly zero
// or one RANGE clauses
partition_param_list ::=
  KW_PARTITION KW_BY hash_partition_param_list:list
  {: RESULT = list; :}
  | KW_PARTITION KW_BY range_partition_param:rng
  {: RESULT = Lists.newArrayList(rng); :}
  | KW_PARTITION KW_BY hash_partition_param_list:list COMMA 
range_partition_param:rng
  {:
    list.add(rng);
    RESULT = list;
  :}
  ;
-------------------------------------
Also, I've been testing and am getting many errors on many of the code 
examples in this file. For example, SORT BY throws an error in a simple 
CREATE TABLE statement (not a CTAS):
--------------------------------------
[vc0136.halxg.cloudera.com:21000] > create table laurel.komono (name string, 
number int, store_location string)
                                  > partitioned by (room string)
                                  > sort by store_location
                                  > comment 'impala-4850 test';
Query: create table laurel.komono (name string, number int, store_location 
string)
partitioned by (room string)
sort by store_location
comment 'impala-4850 test'
ERROR: AnalysisException: Syntax error in line 3:
sort by store_location
^
Encountered: IDENTIFIER
Expected: CACHED, COMMENT, LOCATION, ROW, STORED, TBLPROPERTIES, UNCACHED, WITH
CAUSED BY: Exception: Syntax error
--------------------------------------
This indicates that all of the code examples should be tested in this 
file, which exceeds the scope of this Jira. My testing did verify, 
however, that the table comment should be placed immediately after the 
PARTITIONED BY clause:
--------------------------------------
[vc0136.halxg.cloudera.com:21000] > create table laurel.favorite_places (rating 
int, name string, country string, why string)
                                  > partitioned by (color string)
                                  > comment 'test of impala-4850';
Query: create table laurel.favorite_places (rating int, name string, country 
string, why string)
partitioned by (color string)
comment 'test of impala-4850'
Fetched 0 row(s) in 0.13s
[vc0136.halxg.cloudera.com:21000] > show tables;
Query: show tables
+--------------------+
| name               |
+--------------------+
| favorite_places    |
| favorite_words     |
| french_departments |
+--------------------+
------------------------------------
I will make that fix and discuss the other code examples with John 
Russell to decide how/when that work can be done.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I543ff1dbfe1ab8a7e0a0a668130ab060e3af0a5f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: John Russell <[email protected]>
Gerrit-Reviewer: Laurel Hale <[email protected]>
Gerrit-HasComments: Yes

Reply via email to