Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
Message-Id: <201604290228.u3T2SsE7024812@ip-10-146-233-104.ec2.internal>
Date: Fri, 29 Apr 2016 02:28:53 +0000
From: "Casey Ching (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, dev@impala.incubator.apache.org
CC: Matthew Jacobs <mj@cloudera.com>, Marcel Kornacker <marcel@cloudera.com>,
        Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Reply-To: casey@cloudera.com
Subject: 
 =?UTF-8?Q?[Impala-CR](cdh5-trunk)_Simplify_creating_external_Kudu_tables_and_add_DROP_DATABASE_CASCADE=0A?=
In-Reply-To: 
 <gerrit.1458773009000.Ic141102818b6dad3016181b179a14024d0ff709d@gerrit.cloudera.org>
References: 
 <gerrit.1458773009000.Ic141102818b6dad3016181b179a14024d0ff709d@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.10-rc0

Casey Ching has posted comments on this change.

Change subject: Simplify creating external Kudu tables and add DROP DATABASE CASCADE
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2617/10//COMMIT_MSG
Commit Message:

Line 23: Add KUDU as a file format.
> What do we gain by doing that other than being able to say "STORED AS KUDU"
Consistency. Users familiar with creating tables with various formats should expect "STORED AS KUDU". You think it'll be confusing to mix a file format with a storage engine?


http://gerrit.cloudera.org:8080/#/c/2617/10/fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java:

Line 350: TODO: Investigate what happens if more than one DistributeComponent
        :           //       without any columns is specified. Maybe reject that here?
> Are you going to address these TODOs in a follow up patch?
Yes, distribute params are for managed tables so I'll do this at the same time.


http://gerrit.cloudera.org:8080/#/c/2617/10/fe/src/main/java/com/cloudera/impala/catalog/delegates/KuduDdlDelegate.java
File fe/src/main/java/com/cloudera/impala/catalog/delegates/KuduDdlDelegate.java:

Line 88: of
> typo: if
Done


Line 177: schema.
> maybe "schema stored in msTable_."?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic141102818b6dad3016181b179a14024d0ff709d
Gerrit-PatchSet: 10
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <casey@cloudera.com>
Gerrit-Reviewer: Casey Ching <casey@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes