IMPALA-4527: Columns in Kudu tables created from Impala default to "NULL"

This commit reverts the behavior introduced by IMPALA-3719 which used
the Kudu default behavior for column nullability if none was specified
in the CREATE TABLE statement. With this commit, non-key columns of Kudu
tables that are created from Impala are by default nullable unless
specified otherwise.

Change-Id: I950d9a9c64e3851e11a641573617790b340ece94
Reviewed-on: http://gerrit.cloudera.org:8080/5259
Reviewed-by: Alex Behm <alex.b...@cloudera.com>
Tested-by: Internal Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/da34ce97
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/da34ce97
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/da34ce97

Branch: refs/heads/master
Commit: da34ce9780d03b9abcd7adf2cd75d2f57ab97b1d
Parents: b374061
Author: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Authored: Tue Nov 29 10:02:35 2016 -0800
Committer: Internal Jenkins <cloudera-hud...@gerrit.cloudera.org>
Committed: Fri Dec 2 02:06:22 2016 +0000

----------------------------------------------------------------------
 .../impala/service/KuduCatalogOpExecutor.java   | 15 ++++++++---
 .../queries/QueryTest/kudu_describe.test        | 26 ++++++++++----------
 .../queries/QueryTest/kudu_stats.test           |  6 ++---
 3 files changed, 27 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/da34ce97/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
----------------------------------------------------------------------
diff --git 
a/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java 
b/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
index e3f9a7f..aa553cf 100644
--- a/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
@@ -97,6 +97,7 @@ public class KuduCatalogOpExecutor {
   private static Schema createTableSchema(TCreateTableParams params)
       throws ImpalaRuntimeException {
     Set<String> keyColNames = new 
HashSet<>(params.getPrimary_key_column_names());
+    Preconditions.checkState(!keyColNames.isEmpty());
     List<ColumnSchema> colSchemas = new ArrayList<>(params.getColumnsSize());
     for (TColumn column: params.getColumns()) {
       Type type = Type.fromThrift(column.getColumnType());
@@ -105,9 +106,15 @@ public class KuduCatalogOpExecutor {
       // Create the actual column and check if the column is a key column
       ColumnSchemaBuilder csb =
           new ColumnSchemaBuilder(column.getColumnName(), kuduType);
-      Preconditions.checkState(column.isSetIs_key());
-      csb.key(keyColNames.contains(column.getColumnName()));
-      if (column.isSetIs_nullable()) csb.nullable(column.isIs_nullable());
+      boolean isKey = keyColNames.contains(column.getColumnName());
+      csb.key(isKey);
+      if (column.isSetIs_nullable()) {
+        csb.nullable(column.isIs_nullable());
+      } else if (!isKey) {
+        // Non-key columns are by default nullable unless the user explicitly 
sets their
+        // nullability.
+        csb.nullable(true);
+      }
       if (column.isSetDefault_value()) {
         
csb.defaultValue(KuduUtil.getKuduDefaultValue(column.getDefault_value(), 
kuduType,
             column.getColumnName()));
@@ -363,7 +370,7 @@ public class KuduCatalogOpExecutor {
       Type type = Type.fromThrift(column.getColumnType());
       Preconditions.checkState(type != null);
       org.apache.kudu.Type kuduType = KuduUtil.fromImpalaType(type);
-      boolean isNullable = column.isSetIs_nullable() && column.isIs_nullable();
+      boolean isNullable = !column.isSetIs_nullable() ? true : 
column.isIs_nullable();
       if (isNullable) {
         if (column.isSetDefault_value()) {
           // See KUDU-1747

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/da34ce97/testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test
----------------------------------------------------------------------
diff --git 
a/testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test 
b/testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test
index 280c87d..3d8ac2c 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_describe.test
@@ -4,19 +4,19 @@ describe functional_kudu.alltypes
 ---- LABELS
 
NAME,TYPE,COMMENT,PRIMARY_KEY,NULLABLE,DEFAULT_VALUE,ENCODING,COMPRESSION,BLOCK_SIZE
 ---- RESULTS
-'bigint_col','bigint','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'bool_col','boolean','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'date_string_col','string','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'double_col','double','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'float_col','float','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'bigint_col','bigint','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'bool_col','boolean','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'date_string_col','string','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'double_col','double','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'float_col','float','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
 'id','int','','true','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'int_col','int','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'month','int','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'smallint_col','smallint','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'string_col','string','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'timestamp_col','string','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'tinyint_col','tinyint','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'year','int','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'int_col','int','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'month','int','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'smallint_col','smallint','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'string_col','string','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'timestamp_col','string','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'tinyint_col','tinyint','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'year','int','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
 ---- TYPES
 STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING
 ====
@@ -27,7 +27,7 @@ create table describe_test
  pk2 int,
  pk3 string,
  c1 string null default 'abc' comment 'testing',
- c2 int default 100 encoding plain_encoding compression snappy,
+ c2 int not null default 100 encoding plain_encoding compression snappy,
  c3 int null block_size 8388608,
  primary key (pk1, pk2, pk3))
 distribute by hash (pk1) into 3 buckets

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/da34ce97/testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
----------------------------------------------------------------------
diff --git 
a/testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test 
b/testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
index 3ae4f69..dc8f052 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
@@ -23,9 +23,9 @@ compute stats simple;
 describe simple;
 ---- RESULTS
 'id','int','','true','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'name','string','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'valf','float','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
-'vali','bigint','','false','false','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'name','string','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'valf','float','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
+'vali','bigint','','false','true','','AUTO_ENCODING','DEFAULT_COMPRESSION','0'
 ---- TYPES
 STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING
 ====

Reply via email to