impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Casey Ching (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) Preview: Refactor CREATE TABLE grammar rules in prep for PRIMARY KEY
Date Thu, 28 Apr 2016 21:22:35 GMT
Casey Ching has uploaded a new patch set (#3).

Change subject: Preview: Refactor CREATE TABLE grammar rules in prep for PRIMARY KEY
......................................................................

Preview: Refactor CREATE TABLE grammar rules in prep for PRIMARY KEY

(The main question I have is if people are ok with the CreateTableStmt
changes.

A builder class could also be used but that would duplicate the
state, I'm thinking the builder class and CreateTableStmt would
basically have the same set of fields.

The exiting code had a TODO about using/creating CreateTableStmt params.
That seems fine too, I'll change to that if people prefer it.)

I tried adding PRIMARY KEY for Kudu tables but there was a reduce/reduce
error. Because the create table grammar rules are scattered and very
verbose it was difficult to figure out where the problem was. This
consolidates the various create table rules.

There is one change to the grammar, one or more versions of creating an
external table didn't allow the DISTRIBUTE BY clause. DISTRIBUTE BY only
make sense for managed Kudu tables. There was also a check for this in
analysis so not much needed to be done.

There is still some messiness around EXTERNAL but I think fixing that
will require very verbose grammar rules. It's probably not worth the
trouble.

Change-Id: I9a2b9e380a0b90c0e2e6f10f6905cab5164cb3c4
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/com/cloudera/impala/analysis/CreateTableDataSrcStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java
A fe/src/main/java/com/cloudera/impala/analysis/TableDataArrangementClause.java
A fe/src/main/java/com/cloudera/impala/analysis/TableDefClause.java
A fe/src/main/java/com/cloudera/impala/analysis/TableOptionsClause.java
M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/com/cloudera/impala/analysis/ParserTest.java
10 files changed, 391 insertions(+), 353 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/65/2865/3
-- 
To view, visit http://gerrit.cloudera.org:8080/2865
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a2b9e380a0b90c0e2e6f10f6905cab5164cb3c4
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <casey@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Casey Ching <casey@cloudera.com>

Mime
View raw message