You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2021/08/06 20:51:02 UTC

[Impala-ASF-CR] IMPALA-10845: Increase visibilty of methods and object.

Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17760


Change subject: IMPALA-10845: Increase visibilty of methods and object.
......................................................................

IMPALA-10845: Increase visibilty of methods and object.

Changed the visibility of some methods in SingleNodePlanner
and an object within HdfsPartition to allow extensibility by
a third party tool.

Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
---
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
2 files changed, 8 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17760/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7402/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 23:52:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9311/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 23:52:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................

IMPALA-10858: HMS request for acid fds fixed.

This commit addresses a problem for full acid tables. When the
file descriptors are requested on an HMS call, the insert and
delete file descriptors need to be gathered up and sent back
to the caller.

Also changed the visibility of some methods in SingleNodePlanner
and an object within HdfsPartition to allow extensibility by
a third party tool.

Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Reviewed-on: http://gerrit.cloudera.org:8080/17760
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Vihang Karajgaonkar <vi...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
4 files changed, 87 insertions(+), 8 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Vihang Karajgaonkar: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17760

to look at the new patch set (#2).

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................

IMPALA-10858: HMS request for acid fds fixed.

This commit addresses a problem for full acid tables. When the
file descriptors are requested on an HMS call, the insert and
delete file descriptors need to be gathered up and sent back
to the caller.

Also changed the visibility of some methods in SingleNodePlanner
and an object within HdfsPartition to allow extensibility by
a third party tool.

Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
---
M fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
4 files changed, 81 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17760/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
File fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java@108
PS4, Line 108: testFileMetadataForAcidPartitions
> nit, It would be great if we could add coverage for non-partitioned ACID ta
Looks like there are no unpartitioned ORC tables which have deleted files in the mini-cluster. You can keep it as a TODO for now and may be do it as a follow-up later.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 23:19:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17760

to look at the new patch set (#4).

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................

IMPALA-10858: HMS request for acid fds fixed.

This commit addresses a problem for full acid tables. When the
file descriptors are requested on an HMS call, the insert and
delete file descriptors need to be gathered up and sent back
to the caller.

Also changed the visibility of some methods in SingleNodePlanner
and an object within HdfsPartition to allow extensibility by
a third party tool.

Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
---
M fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
4 files changed, 85 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17760/4
-- 
To view, visit http://gerrit.cloudera.org:8080/17760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10845: Increase visibilty of methods and object.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10845: Increase visibilty of methods and object.
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9255/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Aug 2021 21:14:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17760

to look at the new patch set (#3).

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................

IMPALA-10858: HMS request for acid fds fixed.

This commit addresses a problem for full acid tables. When the
file descriptors are requested on an HMS call, the insert and
delete file descriptors need to be gathered up and sent back
to the caller.

Also changed the visibility of some methods in SingleNodePlanner
and an object within HdfsPartition to allow extensibility by
a third party tool.

Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
---
M fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
4 files changed, 83 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17760/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9293/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 03:39:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9294/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 03:41:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9292/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 03:39:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
File fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java@442
PS4, Line 442: isSetGetFileMetadata
> Why do we need this? I thought we had intentionally changed it earlier from
Reverted


http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1631
PS4, Line 1631: public static
> Can you please add a comment to these methods saying that they are made vis
Done


http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
File fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java@108
PS4, Line 108: testFileMetadataForAcidPartitions
> Looks like there are no unpartitioned ORC tables which have deleted files i
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 23:29:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17760

to look at the new patch set (#5).

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................

IMPALA-10858: HMS request for acid fds fixed.

This commit addresses a problem for full acid tables. When the
file descriptors are requested on an HMS call, the insert and
delete file descriptors need to be gathered up and sent back
to the caller.

Also changed the visibility of some methods in SingleNodePlanner
and an object within HdfsPartition to allow extensibility by
a third party tool.

Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
---
M fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
4 files changed, 87 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/17760/5
-- 
To view, visit http://gerrit.cloudera.org:8080/17760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/17760/2/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
File fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java:

http://gerrit.cloudera.org:8080/#/c/17760/2/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java@188
PS2, Line 188:         for (THdfsFileDesc fd : response.table_info.partitions.get(0).insert_file_descriptors) {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/17760/2/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java@191
PS2, Line 191:         for (THdfsFileDesc fd : response.table_info.partitions.get(0).delete_file_descriptors) {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/17760/2/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
File fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java:

http://gerrit.cloudera.org:8080/#/c/17760/2/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java@110
PS2, Line 110:     ValidWriteIdList writeIdList = getValidWriteIdList("function_orc_def", "alltypes_deleted_rows");
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/17760/2/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java@112
PS2, Line 112:         .getOrLoadTable("functional_orc_def", "alltypes_deleted_rows", "test", writeIdList);
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 03:16:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 4:

(3 comments)

The patch looks good to me. I have some minor suggestions/questions below. Once you address them I can +2 this change.

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
File fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java@442
PS4, Line 442: isSetGetFileMetadata
Why do we need this? I thought we had intentionally changed it earlier from isSetGetFileMetadata in https://gerrit.cloudera.org/#/c/17330 (which was later included in https://gerrit.cloudera.org/#/c/17429/


http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1631
PS4, Line 1631: public static
Can you please add a comment to these methods saying that they are made visible intentionally. This will hopefully avoid situations where someone unintentionally reduces the visibility of these methods.


http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java
File fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java:

http://gerrit.cloudera.org:8080/#/c/17760/4/fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsFileMetadataTest.java@108
PS4, Line 108: testFileMetadataForAcidPartitions
nit, It would be great if we could add coverage for non-partitioned ACID tables as well for getTableReq API.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Aug 2021 17:20:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Aug 2021 16:06:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Aug 2021 06:05:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10858: HMS request for acid fds fixed.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17760 )

Change subject: IMPALA-10858: HMS request for acid fds fixed.
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17760/3/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java
File fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java:

http://gerrit.cloudera.org:8080/#/c/17760/3/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java@188
PS3, Line 188:         for (THdfsFileDesc fd : response.table_info.partitions.get(0).insert_file_descriptors) {
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/17760/3/fe/src/main/java/org/apache/impala/catalog/CatalogHmsAPIHelper.java@191
PS3, Line 191:         for (THdfsFileDesc fd : response.table_info.partitions.get(0).delete_file_descriptors) {
line too long (96 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055960bf0471d62bc37dda3403f58f86f9b5f596
Gerrit-Change-Number: 17760
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 03:18:15 +0000
Gerrit-HasComments: Yes