Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id CFC3F200BE2 for ; Thu, 1 Dec 2016 00:52:53 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id CE547160B19; Wed, 30 Nov 2016 23:52:53 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 244FE160B13 for ; Thu, 1 Dec 2016 00:52:52 +0100 (CET) Received: (qmail 14865 invoked by uid 500); 30 Nov 2016 23:52:52 -0000 Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list reviews@impala.incubator.apache.org Received: (qmail 14854 invoked by uid 99); 30 Nov 2016 23:52:52 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 30 Nov 2016 23:52:52 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id A71721A9AC9 for ; Wed, 30 Nov 2016 23:52:51 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.363 X-Spam-Level: X-Spam-Status: No, score=0.363 tagged_above=-999 required=6.31 tests=[RDNS_DYNAMIC=0.363, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id oZkTyD78mlH5 for ; Wed, 30 Nov 2016 23:52:49 +0000 (UTC) Received: from ip-10-146-233-104.ec2.internal (ec2-75-101-130-251.compute-1.amazonaws.com [75.101.130.251]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 92E275F58F for ; Wed, 30 Nov 2016 23:52:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by ip-10-146-233-104.ec2.internal (8.14.4/8.14.4) with ESMTP id uAUNqavO004640; Wed, 30 Nov 2016 23:52:36 GMT Message-Id: <201611302352.uAUNqavO004640@ip-10-146-233-104.ec2.internal> Date: Wed, 30 Nov 2016 23:52:36 +0000 From: "Dimitris Tsirogiannis (Code Review)" To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org CC: Alex Behm , Todd Lipcon Reply-To: dtsirogiannis@cloudera.com X-Gerrit-MessageType: comment Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4527=3A_Columns_in_Kudu_tables_created_from_Impala_default_to_=22NULL=22=0A?= X-Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 X-Gerrit-ChangeURL: X-Gerrit-Commit: b38e4761af5f80501bb378a261cd48d524e3e6e8 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/2.12.2 archived-at: Wed, 30 Nov 2016 23:52:54 -0000 Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL" ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5259/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java: Line 104: if (column.isSetIs_nullable()) { > This won't handle the ALTER TABLE ADD COLUMN case. Let's think about whethe There is no such place to handle both these cases. The fact that primary keys can be defined in both the column definitions and as a separate list of column names complicates things, i.e. we can't be setting the nullability while analyzing the ColumnDef. Changed the code to handle the addColumn as well. -- To view, visit http://gerrit.cloudera.org:8080/5259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I950d9a9c64e3851e11a641573617790b340ece94 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes