[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite
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 BehmTested-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 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-MarshallGerrit-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 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-MarshallGerrit-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 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-MarshallGerrit-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 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-MarshallGerrit-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
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-MarshallGerrit-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 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-MarshallGerrit-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 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-MarshallGerrit-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
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-MarshallGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite
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-MarshallGerrit-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 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-MarshallGerrit-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 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