Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21143 )

Change subject: IMPALA-12856: Event processor should ignore processing 
partition with empty partition values
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21143/5//COMMIT_MSG@13
PS5, Line 13: partitions with empty values, EP should ignore such partitions 
instead
> HMS will only return metadata info like schema, table location but not the
I think other fields could also be incomplete, like partition params, storage 
descriptor params, serde params, sort cols, bucket cols, skewed cols etc 
mentioned in HIVE-28145.


http://gerrit.cloudera.org:8080/#/c/21143/6/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/21143/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2625
PS6, Line 2625:     } catch (IllegalStateException ex) {
hmm, how can we make sure all the IllegalStateException are due to HMS errors 
here? I think we should just replace the check at L2614 with throwing an 
exception. E.g.

  if (partitionColumns.size() != values.size()) {
    throw new InvalidObjectException(String.format("Unmatched numbers of 
partition values: expected=%d, actual=%d", partitionColumns.size(), 
values.size()));
  }

Also add the table name in the error message.


http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2926
PS6, Line 2926:         continue;
If we use "continue" here, how do we throw an exception?


http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java:

http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@211
PS6, Line 211:           foundEmptyPartitionVals = true;
we can add "break" here


http://gerrit.cloudera.org:8080/#/c/21143/6/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@215
PS6, Line 215: partitions
nit: "values" ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09
Gerrit-Change-Number: 21143
Gerrit-PatchSet: 6
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Mon, 01 Apr 2024 01:15:16 +0000
Gerrit-HasComments: Yes

Reply via email to