You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sourabh Goyal (Code Review)" <ge...@cloudera.org> on 2021/10/13 19:02:53 UTC

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor 

Sourabh Goyal has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17919


Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................

[WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 61 insertions(+), 32 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 1
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

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

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

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................

[WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 63 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 3
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9613/ : 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/17919
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 4
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 12:12:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 6
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 17:43:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 5
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 11:15:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Refactor alter table ddl operation

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

Change subject: IMPALA-10975: Refactor alter table ddl operation
......................................................................


Patch Set 7:

(2 comments)

@Vihang: Thanks for the feedback. I have addressed the review comments.

http://gerrit.cloudera.org:8080/#/c/17919/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17919/6//COMMIT_MSG@7
PS6, Line 7: Refactor alter table ddl operation
           : 
> please keep this to one line as per convention. Suggest you to just title i
Sure


http://gerrit.cloudera.org:8080/#/c/17919/6//COMMIT_MSG@19
PS6, Line 19: 
> Can you also mention about the additional bug fix in the addToDeleteEventLo
Sure.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 7
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Oct 2021 09:19:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17919/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/17919/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1085
PS4, Line 1085:             //addSummary(response, "New location has been set for the specified partition.");
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 4
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 11:51:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17919/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/17919/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1084
PS1, Line 1084:             //addSummary(response, "New location has been set for the specified partition.");
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 1
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Oct 2021 19:03:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9598/ : 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/17919
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 1
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Oct 2021 19:25:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Refactor alter table ddl operation

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

Change subject: IMPALA-10975: Refactor alter table ddl operation
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 7
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Oct 2021 16:55:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 3
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 10:54:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9600/ : 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/17919
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 2
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Oct 2021 19:32:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9629/ : 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/17919
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 5
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 10:29:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Refactor alter table ddl operation

Posted by "Sourabh Goyal (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Yu-Wen Lai, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-10975: Refactor alter table ddl operation
......................................................................

IMPALA-10975: Refactor alter table ddl operation

For almost all alter table DDL operations in catalogOpExecutor,
a table is added to catalog update at a common place in the end
if reloadMetadata is true. However for few sub ddl operations like ADD
and DROP partitions, addTableToCatalogUpdate() is called locally.
This patch is to refactor addTableToCatalogUpdate() and call it at one
place for all the sub ddls.

After this patch, a table (after alter table ddl) is added to catalog
update if its old catalog version does not match the current catalog
version.

This patch also addresses a bug in which a HMS event was getting
added to delete event log even if event processing is not active.

Testing:
Relying on existing tests since it is a minor refactoring.

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 36 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/17919/7
-- 
To view, visit http://gerrit.cloudera.org:8080/17919
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 7
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 4
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 11:51:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 6:

(2 comments)

I can bump it up to +2 after the minor suggestions below are resolved. Since the suggested changes are only in commit msg, I don't believe we need to have a full precommit run again.

http://gerrit.cloudera.org:8080/#/c/17919/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17919/6//COMMIT_MSG@7
PS6, Line 7: Minor refactoring in alter table ddl op in
           : CatalogOpExecutor
please keep this to one line as per convention. Suggest you to just title is "Refactor alter table ddl operation" or something similar.


http://gerrit.cloudera.org:8080/#/c/17919/6//COMMIT_MSG@19
PS6, Line 19: s
Can you also mention about the additional bug fix in the addToDeleteEventLog method?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 6
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 22:55:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10975: Refactor alter table ddl operation

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

Change subject: IMPALA-10975: Refactor alter table ddl operation
......................................................................


Patch Set 7:

Giving a +1 verified since there were only commit msg changes since last verified patchset.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 7
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Oct 2021 16:56:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Refactor alter table ddl operation

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

Change subject: IMPALA-10975: Refactor alter table ddl operation
......................................................................

IMPALA-10975: Refactor alter table ddl operation

For almost all alter table DDL operations in catalogOpExecutor,
a table is added to catalog update at a common place in the end
if reloadMetadata is true. However for few sub ddl operations like ADD
and DROP partitions, addTableToCatalogUpdate() is called locally.
This patch is to refactor addTableToCatalogUpdate() and call it at one
place for all the sub ddls.

After this patch, a table (after alter table ddl) is added to catalog
update if its old catalog version does not match the current catalog
version.

This patch also addresses a bug in which a HMS event was getting
added to delete event log even if event processing is not active.

Testing:
Relying on existing tests since it is a minor refactoring.

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Reviewed-on: http://gerrit.cloudera.org:8080/17919
Reviewed-by: Vihang Karajgaonkar <vi...@cloudera.com>
Tested-by: Vihang Karajgaonkar <vi...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 36 insertions(+), 31 deletions(-)

Approvals:
  Vihang Karajgaonkar: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 8
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17919/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/17919/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1084
PS2, Line 1084:             //addSummary(response, "New location has been set for the specified partition.");
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 2
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Oct 2021 19:09:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

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

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

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................

[WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 65 insertions(+), 31 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 4
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7541/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 4
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 18:40:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

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

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

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................

IMPALA-10975: Minor refactoring in alter table ddl op in
CatalogOpExecutor

For almost all alter table DDL operations in catalogOpExecutor,
a table is added to catalog update at a common place in the end
if reloadMetadata is true. However for few sub ddl operations like ADD
and DROP partitions, the table to update catalog is performed locally.
This patch is to refactor addTableToCatalogUpdate() and call it at one
place for all the sub ddls.

After this patch, a table (after alter table ddl) is added to catalog
update if its old catalog version does not match the current catalog
version.

Testing:
Relying on existing tests since it is a minor refactoring.

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 36 insertions(+), 31 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 5
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 3
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 17:12:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9612/ : 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/17919
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 3
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 11:15:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17919/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/17919/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1085
PS3, Line 1085:             //addSummary(response, "New location has been set for the specified partition.");
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 3
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Oct 2021 10:55:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

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

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

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

Change subject: [WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................

[WIP]: Minor refactoring in alter table ddl op in CatalogOpExecutor

Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 62 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 2
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10975: Refactor alter table ddl operation

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

Change subject: IMPALA-10975: Refactor alter table ddl operation
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 7
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Oct 2021 16:56:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 5
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 17:33:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 6:

Please share your feedback on this patch.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 6
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 11:21:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor

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

Change subject: IMPALA-10975: Minor refactoring in alter table ddl op in CatalogOpExecutor
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbadcab68b4599ad18b681b1284052a47b74d802
Gerrit-Change-Number: 17919
Gerrit-PatchSet: 6
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Oct 2021 11:19:58 +0000
Gerrit-HasComments: No