Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12213 )
Change subject: IMPALA-7929. Impala query on HBASE table failing with InternalException. ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/12213/1/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: http://gerrit.cloudera.org:8080/#/c/12213/1/common/thrift/PlanNodes.thrift@287 PS1, Line 287: 2: optional string qualifier We write this in Java and read it in C++ (hbase-table-scanner.cc). In Java, optional allows us to leave this as null, which is equivalent to unset. What is happening on the C++ side? What value does it see? Does that value get conveyed as null when the C++ side uses JNI to call back into Java? By the way, you can examine the code that thrift generates by looking in be/generated-sources/gen-cpp for C++ and fe/generated-sources/gen-java for the Java. http://gerrit.cloudera.org:8080/#/c/12213/1/common/thrift/PlanNodes.thrift@291 PS1, Line 291: 3: optional i32 op_ordinal : 4: optional string filter_constant Do these need to be optional? Is filter_constant ever null? http://gerrit.cloudera.org:8080/#/c/12213/1/tests/query_test/test_hbase_queries.py File tests/query_test/test_hbase_queries.py: http://gerrit.cloudera.org:8080/#/c/12213/1/tests/query_test/test_hbase_queries.py@68 PS1, Line 68: @SkipIfIsilon.hive > line has trailing whitespace You are going to want to set up your editor to avoid trailing whitespace and flag it proactively. For emacs, I think I use this: (setq-default show-trailing-whitespace t) http://gerrit.cloudera.org:8080/#/c/12213/1/tests/query_test/test_hbase_queries.py@78 PS1, Line 78: cr_table = ("CREATE TABLE %s (k STRING, c STRING) STORED BY " : "'org.apache.hadoop.hive.hbase.HBaseStorageHandler' WITH " : "SERDEPROPERTIES ('hbase.columns.mapping'=':key,cf:c'," : " 'serialization.format'='1') TBLPROPERTIES " : "('hbase.table.name'=\'%s\', " : "'storage_handler'='org.apache.hadoop.hive.hbase.HBaseStorageHandler')") \ : % (table_name, table_name) Couple things here: 1. Use a """ multiline python string and make this more readable. See for example: https://github.com/apache/impala/blob/master/tests/query_test/test_insert_behaviour.py#L652 2. Use "{0}".format(blah) style replacements. -- To view, visit http://gerrit.cloudera.org:8080/12213 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I378c2249604481067b5b1c3a3bbb28c30ad4d751 Gerrit-Change-Number: 12213 Gerrit-PatchSet: 1 Gerrit-Owner: Yongjun Zhang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Yongjun Zhang <[email protected]> Gerrit-Comment-Date: Thu, 10 Jan 2019 21:44:22 +0000 Gerrit-HasComments: Yes
