Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/9884 )
Change subject: IMPALA-6571: NullPointerException in SHOW CREATE TABLE for HBase tables ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/9884/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/9884/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@233 PS1, Line 233: RowFormat rowFormat = RowFormat.fromStorageDescriptor(msTable.getSd()); : HdfsFileFormat format = null; : HdfsCompression compression = null; : String inputFormat = msTable.getSd().getInputFormat(); : if (inputFormat != null) { : format = HdfsFileFormat.fromHdfsInputFormatClass(inputFormat); : compression = HdfsCompression.fromHdfsInputFormatClass(inputFormat); : } I think these things make sense only for an HdfsTable (kudu format is handled below separately). So instead of checking for inputFormat != null, should we do something like if (table instanceof HdfsTable) { // check for hdfs related stuff } http://gerrit.cloudera.org:8080/#/c/9884/1/testdata/workloads/functional-query/queries/QueryTest/hbase-show-create-table.test File testdata/workloads/functional-query/queries/QueryTest/hbase-show-create-table.test: http://gerrit.cloudera.org:8080/#/c/9884/1/testdata/workloads/functional-query/queries/QueryTest/hbase-show-create-table.test@2 PS1, Line 2: ---- QUERY : SHOW CREATE TABLE functional_hbase.alltypes : ---- RESULTS : CREATE EXTERNAL TABLE functional_hbase.alltypes ( : id INT COMMENT 'Add a comment', : bigint_col BIGINT, : bool_col BOOLEAN, : date_string_col STRING, : double_col DOUBLE, : float_col FLOAT, : int_col INT, : month INT, : smallint_col SMALLINT, : string_col STRING, : timestamp_col TIMESTAMP, : tinyint_col TINYINT, : year INT : ) : STORED BY 'org.apache.hadoop.hive.hbase.HBaseStorageHandler' : WITH SERDEPROPERTIES ('hbase.columns.mapping'=':key,d:bool_col,d:tinyint_col,d:smallint_col,d:int_col,d:bigint_col,d:float_col,d:double_col,d:date_string_col,d:string_col,d:timestamp_col,d:year,d:month', : 'serialization.format'='1') : TBLPROPERTIES ('hbase.table.name'='functional_hbase.alltypes', : 'storage_handler'='org.apache.hadoop.hive.hbase.HBaseStorageHandler') I think we could merge this with show-create-table.test. Don't think we need a separate file for it. -- To view, visit http://gerrit.cloudera.org:8080/9884 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe018313168fac5dcbd80be9a8f28b71a2c0389b Gerrit-Change-Number: 9884 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Comment-Date: Mon, 02 Apr 2018 17:09:42 +0000 Gerrit-HasComments: Yes
