Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21437 )

Change subject: IMPALA-12277: Fix NullPointerException for partitioned inserts 
when EP is turned off
......................................................................


Patch Set 14:

(4 comments)

Will take another look at this, but here is my preliminary comment.

http://gerrit.cloudera.org:8080/#/c/21437/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21437/12//COMMIT_MSG@17
PS12, Line 17: r lagged behind. This patch
             : address this issue by always loading the verifying the 
partitions from
             : HMS without loading the file metadata from HMS regardless
> Will change this.
Done


http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG@18
PS14, Line 18: always loading the verifying the partitions
nit: "always verify the partitions" ?


http://gerrit.cloudera.org:8080/#/c/21437/14/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/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1265
PS14, Line 1265: public void load(boolean reuseMetadata, IMetaStoreClient 
client,
               :       org.apache.hadoop.hive.metastore.api.Table msTbl, 
boolean loadPartitionFileMetadata,
               :       boolean loadTableSchema, boolean 
refreshUpdatedPartitions,
               :       @Nullable Set<String> partitionsToUpdate, @Nullable 
String debugAction,
               :       @Nullable Map<String, Long> partitionToEventId, String 
reason,
               :       EventSequence catalogTimeline, boolean 
isPreLoadForInsert)
This method has 12 parameter now. That is quite long.
Is it possible to wrap them into one Class? And maybe fluent style Builder for 
that class.

LoadParam param = LoadParamBuilder.reuseMetadata(true).metastoreClient(client). 
... .build();

See example at ResourceProfile.java and ResourceProfileBuilder.java.


http://gerrit.cloudera.org:8080/#/c/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1270
PS14, Line 1270: isPreLoadForInsert
Add documentation for isPreLoadForInsert parameter.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide8f1f6bf017e9a040b53bb5d5291ff2ea3e0d18
Gerrit-Change-Number: 21437
Gerrit-PatchSet: 14
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Thu, 13 Jun 2024 23:42:01 +0000
Gerrit-HasComments: Yes

Reply via email to