[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..

IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

This patch fixes two problems:
- Previously a CTAS into a Kudu table where an expr rewrite occurred
  would create an unpartitioned table, due to the partition info being
  reset in TableDataLayout and then never reconstructed. Since the
  Kudu partition info is set by the parser and never changes, the
  solution is to not reset it.
- Previously a CTAS into a Kudu table with a range partition where an
  expr rewrite occurred would fail with an analysis exception due to
  a Precondition check in RangePartition.analyze that checked that
  the RangePartition wasn't already analyzed, as the analysis can't
  be done twice. Since the state in RangePartition never changes, it
  doesn't need to be reanalyzed and we can just return instead of
  failing on the check.

Testing:
- Added an e2e test that creates a partitioned Kudu table with a CTAS
  with a rewrite, and checks that the expected partitions are created.

Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Reviewed-on: http://gerrit.cloudera.org:8080/10251
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 16 insertions(+), 8 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 02 May 2018 02:54:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2396/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 01 May 2018 22:59:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 01 May 2018 20:34:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@57
PS1, Line 57:
> Ideally we'd call clear() in the same statement that populates the partitio
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 01 May 2018 20:00:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Thomas Tauber-Marshall (Code Review)]

Hello Zoram Thanga, Alex Behm,

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

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

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

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..

IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

This patch fixes two problems:
- Previously a CTAS into a Kudu table where an expr rewrite occurred
  would create an unpartitioned table, due to the partition info being
  reset in TableDataLayout and then never reconstructed. Since the
  Kudu partition info is set by the parser and never changes, the
  solution is to not reset it.
- Previously a CTAS into a Kudu table with a range partition where an
  expr rewrite occurred would fail with an analysis exception due to
  a Precondition check in RangePartition.analyze that checked that
  the RangePartition wasn't already analyzed, as the analysis can't
  be done twice. Since the state in RangePartition never changes, it
  doesn't need to be reanalyzed and we can just return instead of
  failing on the check.

Testing:
- Added an e2e test that creates a partitioned Kudu table with a CTAS
  with a rewrite, and checks that the expected partitions are created.

Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
---
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
5 files changed, 16 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@57
PS1, Line 57: partitionColDefs_.clear();
> This was introduced in: https://gerrit.cloudera.org/#/c/8930/
Ideally we'd call clear() in the same statement that populates the partition 
column defs in its analyze() function.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 01 May 2018 18:58:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 2:

(2 comments)

Alex - can you think of any situations where an expr rewrite would happen for a 
CREATE TABLE that's not a CTAS (if so, then this patch doesn't solve all the 
problems, but I don't think its possible)?

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@57
PS1, Line 57: partitionColDefs_.clear();
> Why would this class even have a reset()? That function is used to wipe ana
This was introduced in: https://gerrit.cloudera.org/#/c/8930/

It's needed for partitionColDefs_ because that list is constructed during 
CreateTableAsSelectStmt::analyze().

Maybe it would be cleaner to just call clear() in 
CreateTableAsSelectStmt::reset()?


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

http://gerrit.cloudera.org:8080/#/c/10251/1/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test@340
PS1, Line 340: # IMPALA-6954: CTAS with an expr rewrite.
> IMPALA-6954: CTAS ...
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 01 May 2018 18:22:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Thomas Tauber-Marshall (Code Review)]

Hello Zoram Thanga, Alex Behm,

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

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

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

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..

IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

This patch fixes two problems:
- Previously a CTAS into a Kudu table where an expr rewrite occurred
  would create an unpartitioned table, due to the partition info being
  reset in TableDataLayout and then never reconstructed. Since the
  Kudu partition info is set by the parser and never changes, the
  solution is to not reset it.
- Previously a CTAS into a Kudu table with a range partition where an
  expr rewrite occurred would fail with an analysis exception due to
  a Precondition check in RangePartition.analyze that checked that
  the RangePartition wasn't already analyzed, as the analysis can't
  be done twice. Since the state in RangePartition never changes, it
  doesn't need to be reanalyzed and we can just return instead of
  failing on the check.

Testing:
- Added an e2e test that creates a partitioned Kudu table with a CTAS
  with a rewrite, and checks that the expected partitions are created.

Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
---
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
3 files changed, 14 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

http://gerrit.cloudera.org:8080/#/c/10251/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@57
PS1, Line 57: partitionColDefs_.clear();
Why would this class even have a reset()? That function is used to wipe 
analysis state, but this class has no analyze(). Can we remove reset() entirely?


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

http://gerrit.cloudera.org:8080/#/c/10251/1/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test@340
PS1, Line 340: # CTAS with an expr rewrite.
IMPALA-6954: CTAS ...



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 01 May 2018 16:05:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Zoram Thanga (Code Review)]

Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10251 )

Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 30 Apr 2018 23:10:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10251


Change subject: IMPALA-6954: Fix problems with CTAS into Kudu with an expr 
rewrite
..

IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

This patch fixes two problems:
- Previously a CTAS into a Kudu table where an expr rewrite occurred
  would create an unpartitioned table, due to the partition info being
  reset in TableDataLayout and then never reconstructed. Since the
  Kudu partition info is set by the parser and never changes, the
  solution is to not reset it.
- Previously a CTAS into a Kudu table with a range partition where an
  expr rewrite occurred would fail with an analysis exception due to
  a Precondition check in RangePartition.analyze that checked that
  the RangePartition wasn't already analyzed, as the analysis can't
  be done twice. Since the state in RangePartition never changes, it
  doesn't need to be reanalyzed and we can just return instead of
  failing on the check.

Testing:
- Added an e2e test that creates a partitioned Kudu table with a CTAS
  with a rewrite, and checks that the expected partitions are created.

Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
---
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
3 files changed, 14 insertions(+), 3 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I731743bd84cc695119e99342e1b155096147f0ed
Gerrit-Change-Number: 10251
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall