You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2016/10/25 22:14:24 UTC

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Thomas Tauber-Marshall has uploaded a new change for review.

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

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................

IMPALA-4260: Alter table add column drops all the column stats

Hive expects types for column stats to be specified as all lower
case. For some reason, it doesn't check this when the stats are
first written, but it does check when performing an 'alter table'.
This causes it to drop stats that Impala wrote because we specify
type names in upper case.

This patch converts the types that Impala sends to Hive for the
column stats to all lower case and adds a regression test.

I also filed HIVE-15061 to track the issue from the Hive end.

Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
2 files changed, 54 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................


Patch Set 1:

I think we recently broke something with respect to casing of column types. Jenny recently sent an email about DESCRIBE FORMATTED printing the types in uppercase. Can you take a quick look? These issues might be related (and the fix might be elsewhere)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................


IMPALA-4260: Alter table add column drops all the column stats

Hive expects types for column stats to be specified as all lower
case. For some reason, it doesn't check this when the stats are
first written, but it does check when performing an 'alter table'.
This causes it to drop stats that Impala wrote because we specify
type names in upper case.

This patch converts the types that Impala sends to Hive for the
column stats to all lower case and adds a regression test.

I also filed HIVE-15061 to track the issue from the Hive end.

Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Reviewed-on: http://gerrit.cloudera.org:8080/4845
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
2 files changed, 83 insertions(+), 1 deletion(-)

Approvals:
  Matthew Jacobs: Looks good to me, approved
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................


Patch Set 2: Code-Review+2

carrying dimitris' +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................

IMPALA-4260: Alter table add column drops all the column stats

Hive expects types for column stats to be specified as all lower
case. For some reason, it doesn't check this when the stats are
first written, but it does check when performing an 'alter table'.
This causes it to drop stats that Impala wrote because we specify
type names in upper case.

This patch converts the types that Impala sends to Hive for the
column stats to all lower case and adds a regression test.

I also filed HIVE-15061 to track the issue from the Hive end.

Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
2 files changed, 83 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4845/1/testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/compute-stats.test:

Line 121: ====
Can you also add a test case for CHANGE and REPLACE columns as well?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4260: Alter table add column drops all the column stats

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4260: Alter table add column drops all the column stats
......................................................................


Patch Set 2:

(1 comment)

The 'describe formatted' issue is unrelated.

I filed: https://issues.cloudera.org/browse/IMPALA-4372

http://gerrit.cloudera.org:8080/#/c/4845/1/testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/compute-stats.test:

Line 121: 'new_col2','INT',-1,-1,4,4
> Can you also add a test case for CHANGE and REPLACE columns as well?
Turns out this doesn't really work, due to a Hive issue.

I added a test for what does work, and I filed:
https://issues.apache.org/jira/browse/HIVE-15075
https://jira.cloudera.com/browse/CDH-46452


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia373ec917efa7ab9f2a59b8a870b7ebc30175dda
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes