Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16008 )

Change subject: IMPALA-9791: Support validWriteIdList in 
getPartialCatalogObject API
......................................................................


Patch Set 11: Code-Review+1

(4 comments)

LGTM, only spotted a few nits.

http://gerrit.cloudera.org:8080/#/c/16008/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/16008/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1619
PS11, Line 1619:             "Unable to fetch valid transaction ids while 
loading file metadata for table "
               :                 + getFullName());
You could also add the error message in 'ex' to get the root cause.


http://gerrit.cloudera.org:8080/#/c/16008/11/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java
File 
fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java:

http://gerrit.cloudera.org:8080/#/c/16008/11/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@86
PS11, Line 86:   @Before
             :   public void createTestTbls() throws Exception {
             :     try (HiveJdbcClient client = hiveClientPool_.getClient()) {
             :       client.executeSql("create database if not exists " + 
testDbName);
             :       client.executeSql("create table " + getTestTblName() + " 
like "
             :         + "functional.insert_only_transactional_table stored as 
parquet");
             :       client
             :         .executeSql("insert into " + getTestTblName() + " values 
(1)");
             :       client.executeSql("create table " + 
getPartitionedTblName() + " (c1 int) "
             :         + "partitioned by (part int) stored as parquet " + 
getTblProperties());
             :       client.executeSql("insert into " + getPartitionedTblName() 
+ " partition (part=1) "
             :         + " values (1)");
             :     }
             :     catalog_.reset();
             :   }
             :
             :   private static String getTblProperties() {
             :     return "tblproperties ('transactional'='true', 
'transactional_properties' = "
             :       + "'insert_only')";
             :   }
             :
             :   @After
             :   public void dropTestTbls() throws Exception {
             :     try (HiveJdbcClient client = hiveClientPool_.getClient()) {
             :       client.executeSql("drop database " + testDbName + " 
cascade");
             :     }
             :   }
Would it make sense to execute these via Impala? Probably it'd make the test a 
bit faster.


http://gerrit.cloudera.org:8080/#/c/16008/11/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@348
PS11, Line 348: 2
nit: missing comma


http://gerrit.cloudera.org:8080/#/c/16008/11/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@411
PS11, Line 411:         "alter table " + getTestTblName()
              :             + " compact 'minor' and wait");
nit: fits single line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied2c7c3cb2009c407e8fbc3af4722b0d34f57c4a
Gerrit-Change-Number: 16008
Gerrit-PatchSet: 11
Gerrit-Owner: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Anurag Mantripragada <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 10 Jun 2020 14:04:59 +0000
Gerrit-HasComments: Yes

Reply via email to