You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2018/07/26 07:52:36 UTC

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11056


Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................

IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

The bug was that HdfsTable#reloadPartition() was not setting the
partition's numRows_ after reloading the partition.

Fix: Move the code to set numRows_ to createPartition() call so that
the callers need not explicitly set it after calling createPartition().

Added a test.

Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M tests/metadata/test_refresh_partition.py
2 files changed, 32 insertions(+), 6 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11056/1/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/11056/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@855
PS1, Line 855:         if (!TAccessLevelUtil.impliesWriteAccess(partition.getAccessLevel())) {
             :           // TODO: READ_ONLY isn't exactly correct because the it's possible the
             :           // partition does not have READ permissions either. When we start checking
             :           // whether we can READ from a table, this should be updated to set the
             :           // table's access level to the "lowest" effective level across all
             :           // partitions. That is, if one partition has READ_ONLY and another has
             :           // WRITE_ONLY the table's access level should be NONE.
             :           accessLevel_ = TAccessLevel.READ_ONLY;
             :         }
while we are here, is it possible to consolidate this common code too?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 16:12:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 20:48:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Tianyi Wang, Todd Lipcon, Impala Public Jenkins, Vuk Ercegovac, 

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

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

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................

IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

The bug was that HdfsTable#reloadPartition() was not setting the
partition's numRows_ after reloading the partition.

Fix: Move the code to set numRows_ to createPartition() call so that
the callers need not explicitly set it after calling createPartition().
Additionally consolidates other common code in createPartition().

Added a test.

Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M tests/metadata/test_refresh_partition.py
2 files changed, 42 insertions(+), 25 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 07:54:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 08:25:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 2:

Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/75/ 

Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 17:16:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11056 )

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................

IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

The bug was that HdfsTable#reloadPartition() was not setting the
partition's numRows_ after reloading the partition.

Fix: Move the code to set numRows_ to createPartition() call so that
the callers need not explicitly set it after calling createPartition().
Additionally consolidates other common code in createPartition().

Added a test.

Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Reviewed-on: http://gerrit.cloudera.org:8080/11056
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M tests/metadata/test_refresh_partition.py
2 files changed, 42 insertions(+), 25 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 11:05:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 00:11:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11056/1/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/11056/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@855
PS1, Line 855:         if (!TAccessLevelUtil.impliesWriteAccess(partition.getAccessLevel())) {
             :           // TODO: READ_ONLY isn't exactly correct because the it's possible the
             :           // partition does not have READ permissions either. When we start checking
             :           // whether we can READ from a table, this should be updated to set the
             :           // table's access level to the "lowest" effective level across all
             :           // partitions. That is, if one partition has READ_ONLY and another has
             :           // WRITE_ONLY the table's access level should be NONE.
             :           accessLevel_ = TAccessLevel.READ_ONLY;
             :         }
> while we are here, is it possible to consolidate this common code too?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 17:16:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 1:

Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/73/ 

Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 1
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 07:52:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 17:50:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows

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

Change subject: IMPALA-7225: REFRESH..PARTITION shoud not reset partition's num rows
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5886684e26b453c76f331f6e807f813146c6bf3a
Gerrit-Change-Number: 11056
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Jul 2018 21:06:25 +0000
Gerrit-HasComments: No