You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Laurel Hale (Code Review)" <ge...@cloudera.org> on 2017/06/05 18:52:59 UTC
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Laurel Hale has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7080
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Corrected code example listed under "Column definitions inferred
from data file." Moved the table comment ([COMMENT
'table_comment']) to the end of the code block after the
SerDe properties. Also added a <ph> reference listing the
release number of Impala and the Jira number.
Change-Id: I543ff1dbfe1ab8a7e0a0a668130ab060e3af0a5f
---
M docs/topics/impala_create_table.xml
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/7080/1
--
To view, visit http://gerrit.cloudera.org:8080/7080
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I543ff1dbfe1ab8a7e0a0a668130ab060e3af0a5f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <la...@cloudera.com>
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
Patch Set 2:
> Lars, did you clarify this with Laurel? What to do with this
> change?
I think I missed this comment as I wasn't added as a reviewer on this change. Apologies for the delay.
SORT BY (a) requires it's ordering columns to be in parentheses, just like PARTITIONED BY(a).
Laurel, how can I help? Do you want to discuss the change in person?
--
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Laurel Hale (Code Review)" <ge...@cloudera.org>.
Laurel Hale has uploaded a new patch set (#2).
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Corrected code example listed under "Column definitions inferred
from data file."
Change-Id: I543ff1dbfe1ab8a7e0a0a668130ab060e3af0a5f
---
M docs/topics/impala_create_table.xml
1 file changed, 7 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/7080/2
--
To view, visit http://gerrit.cloudera.org:8080/7080
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I543ff1dbfe1ab8a7e0a0a668130ab060e3af0a5f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
Patch Set 1:
(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.
The correct order can be seen from our code in sql-parser.cup:
tbl_options ::=
opt_sort_cols:sort_cols opt_comment_val:comment row_format_val:row_format
serde_properties:serde_props file_format_create_table_val:file_format
location_val:location cache_op_val:cache_op
tbl_properties:tbl_props
{:
CreateTableStmt.unescapeProperties(serde_props);
CreateTableStmt.unescapeProperties(tbl_props);
RESULT = new TableDef.Options(sort_cols, comment, row_format, serde_props,
file_format, location, cache_op, tbl_props);
:}
;
1. Table comment comes immediately after PARTITIONED BY.
2. ROW FORMAT comes before WITH SERDEPROPERTIES
Might be good to play around with CREATE TABLE to confirm what exactly the syntax is.
--
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 <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has abandoned this change. ( http://gerrit.cloudera.org:8080/7080 )
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
Abandoned
This is an old review that was fixed elsewhere.
--
To view, visit http://gerrit.cloudera.org:8080/7080
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I543ff1dbfe1ab8a7e0a0a668130ab060e3af0a5f
Gerrit-Change-Number: 7080
Gerrit-PatchSet: 2
Gerrit-Owner: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Laurel Hale (Code Review)" <ge...@cloudera.org>.
Laurel Hale has posted comments on this change.
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
Patch Set 2:
(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="IMPALA-4850">
> There seem to be a few things amiss.
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
......................................................................
Patch Set 2:
Lars, did you clarify this with Laurel? What to do with this change?
--
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4850 [DOCS] Create table "comment comes after "partioned by"
Posted by "Laurel Hale (Code Review)" <ge...@cloudera.org>.
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 <la...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <la...@cloudera.com>
Gerrit-HasComments: Yes