[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Internal Jenkins has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. Fixes inserts into partitioned tables that have a shuffle hint and only constant partition exprs. The rows to be inserted are merged at the coordinator where the table sink is executed. There is no need to hash exchange rows. Now accepts insert hints when inserting into unpartitioned tables. The shuffle hint leads to a plan where all rows are merged at the coordinator where the table sink is executed. Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Reviewed-on: http://gerrit.cloudera.org:8080/4162 Reviewed-by: Marcel Kornacker Tested-by: Internal Jenkins --- M fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java M fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java M testdata/workloads/functional-planner/queries/PlannerTest/insert.test 4 files changed, 126 insertions(+), 85 deletions(-) Approvals: Marcel Kornacker: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Marcel Kornacker
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/4162/5/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: Line 241: partition = DataPartition.hashPartitioned(partitionExprs); > leave todo in hashPartitioned() to checkstate that the expr list isn't empt There's already a checkstate in the c'tor of DataPartition for that. -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 5: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/4162/5/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: Line 241: partition = DataPartition.hashPartitioned(partitionExprs); leave todo in hashPartitioned() to checkstate that the expr list isn't empty (don't add it, it might break some stuff :)) -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4162 to look at the new patch set (#5). Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. Fixes inserts into partitioned tables that have a shuffle hint and only constant partition exprs. The rows to be inserted are merged at the coordinator where the table sink is executed. There is no need to hash exchange rows. Now accepts insert hints when inserting into unpartitioned tables. The shuffle hint leads to a plan where all rows are merged at the coordinator where the table sink is executed. Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 --- M fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java M fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java M testdata/workloads/functional-planner/queries/PlannerTest/insert.test 4 files changed, 126 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/4162/5 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/4162/4/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: Line 197: if (insertStmt.hasNoShuffleHint()) return inputFragment; > i find this easier to read than the 3-valued "flag" from before Done Line 249: DataPartition partition = DataPartition.hashPartitioned(partitionExprs); > change this c'tor to return an unpartitioned DataPartition if the expr list Imo, changing the behavior of the hashPartitioned() function to return unpartitioned seems misleading to callers and not really expected. It seems like in most cases it would be unintentional, i.e., a bug. I made the change right here and got rid of the short-circuit above. -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/4162/4/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: Line 197: if (insertStmt.hasNoShuffleHint()) return inputFragment; i find this easier to read than the 3-valued "flag" from before Line 249: DataPartition partition = DataPartition.hashPartitioned(partitionExprs); change this c'tor to return an unpartitioned DataPartition if the expr list is empty? -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4162 to look at the new patch set (#4). Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. Fixes inserts into partitioned tables that have a shuffle hint and only constant partition exprs. The rows to be inserted are merged at the coordinator where the table sink is executed. There is no need to hash exchange rows. Now accepts insert hints when inserting into unpartitioned tables. The shuffle hint leads to a plan where all rows are merged at the coordinator where the table sink is executed. Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 --- M fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java M fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeStmtsTest.java M testdata/workloads/functional-planner/queries/PlannerTest/insert.test 4 files changed, 133 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/4162/4 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 3: Code-Review+1 Carry Dimitris' +1 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Hello Dimitris Tsirogiannis, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4162 to look at the new patch set (#3). Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. Fixes inserts into partitioned tables that have a shuffle hint and only constant partition exprs. The rows to be inserted are merged at the coordinator where the table sink is executed. There is no need to hash exchange rows. Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 --- M fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java M testdata/workloads/functional-planner/queries/PlannerTest/insert.test 2 files changed, 88 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/4162/3 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4162/2/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: PS2, Line 207: Preconditions.checkState(repartitionHint != null); > I think you can remove this. Ahh right, doesn't really make sense anymore. Done. -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4162/2/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: PS2, Line 207: Preconditions.checkState(repartitionHint != null); I think you can remove this. -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4162/1/testdata/workloads/functional-planner/queries/PlannerTest/insert.test File testdata/workloads/functional-planner/queries/PlannerTest/insert.test: Line 543: > Maybe I misunderstood, but I thought there were two different bugs here, th Yea, it covers both, but let me explain. In IMPALA-2570 we just ignored the shuffle hint if all partition exprs were constant. However, in recent Impala versions we cannot even run that query reported in IMPALA-3930 because we hit a Preconditions check instead. So the fix here is to not hit the Precondition and make sure we "shuffle" before the insert to give the user some control over how many files will be created. -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4162/1/testdata/workloads/functional-planner/queries/PlannerTest/insert.test File testdata/workloads/functional-planner/queries/PlannerTest/insert.test: Line 543: > That doesn't seem useful to me. Is there a specific aspect of the original Maybe I misunderstood, but I thought there were two different bugs here, the one ignores the hint and the other throws an exception. If the test cases cover both that's fine by me. -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has uploaded a new patch set (#2). Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. Fixes inserts into partitioned tables that have a shuffle hint and only constant partition exprs. The rows to be inserted are merged at the coordinator where the table sink is executed. There is no need to hash exchange rows. Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 --- M fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java M testdata/workloads/functional-planner/queries/PlannerTest/insert.test 2 files changed, 89 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/4162/2 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has posted comments on this change.
Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant
partition exprs.
..
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/4162/1/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java
File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java:
Line 199: // do nothing if the input fragment is already appropriately
partitioned
> expand comment: what happens if partitionExprs contains constants (why do y
changed the code back to the original order
PS1, Line 201: equivSets(inputPartition.getPartitionExprs(), partitionExprs
> So the fact that we may have constants doesn't affect this operation, corre
I moved the removal of constant exprs back up like it was before.
PS1, Line 217: don
> nit: "Don't" This files doesn't seem to be consistent on how comments are w
Done
Line 231: Preconditions.checkState(partitionHint == null || partitionHint);
> partitionHint doesn't describe the meaning. repartitionHint?
Done
PS1, Line 231: partitionHint == null || partitionHint
> oh this three-way logic...
Yup, seems necessary though since there are 3 values
Line 232: if (partitionExprs.isEmpty()) {
> can't you short-circuit this path and move it right below l206? are there o
Reworked to move this code up.
The other corner case is the noshuffle hint which is handled in L197.
http://gerrit.cloudera.org:8080/#/c/4162/1/testdata/workloads/functional-planner/queries/PlannerTest/insert.test
File testdata/workloads/functional-planner/queries/PlannerTest/insert.test:
PS1, Line 533: 2009, 1
> Can we also add a case with a constant expr? e.g. 2010-1
Done
Line 543:
> For completion, can you add the test case from IMPALA-3930?
That doesn't seem useful to me. Is there a specific aspect of the original test
that you find particularly useful? Only because it's "a" repro from the JIRA
doesn't mean it's the one which gives us most coverage.
--
To view, visit http://gerrit.cloudera.org:8080/4162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Marcel Kornacker
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Marcel Kornacker has posted comments on this change.
Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant
partition exprs.
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4162/1/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java
File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java:
Line 199: // do nothing if the input fragment is already appropriately
partitioned
expand comment: what happens if partitionExprs contains constants (why do you
need to take them into account?)
Line 231: Preconditions.checkState(partitionHint == null || partitionHint);
partitionHint doesn't describe the meaning. repartitionHint?
Line 232: if (partitionExprs.isEmpty()) {
can't you short-circuit this path and move it right below l206? are there other
hint-related corner cases that need to be excluded by the if stmt in l209?
--
To view, visit http://gerrit.cloudera.org:8080/4162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Marcel Kornacker
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4162/1/fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java File fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java: PS1, Line 201: equivSets(inputPartition.getPartitionExprs(), partitionExprs So the fact that we may have constants doesn't affect this operation, correct? Do we have a test case that triggers this path? PS1, Line 217: don nit: "Don't" This files doesn't seem to be consistent on how comments are written. PS1, Line 231: partitionHint == null || partitionHint oh this three-way logic... http://gerrit.cloudera.org:8080/#/c/4162/1/testdata/workloads/functional-planner/queries/PlannerTest/insert.test File testdata/workloads/functional-planner/queries/PlannerTest/insert.test: PS1, Line 533: 2009, 1 Can we also add a case with a constant expr? e.g. 2010-1 Line 543: For completion, can you add the test case from IMPALA-3930? -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs.
Alex Behm has uploaded a new change for review. http://gerrit.cloudera.org:8080/4162 Change subject: IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. .. IMPALA-3930,IMPALA-2570: Fix shuffle insert hint with constant partition exprs. Fixes inserts into partitioned tables that have a shuffle hint and only constant partition exprs. The rows to be inserted are merged at the coordinator where the table sink is executed. There is no need to hash exchange rows. Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 --- M fe/src/main/java/com/cloudera/impala/planner/DistributedPlanner.java M testdata/workloads/functional-planner/queries/PlannerTest/insert.test 2 files changed, 66 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/4162/1 -- To view, visit http://gerrit.cloudera.org:8080/4162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1084d49c95b7d867eeac3297fd2016daff0ab687 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm
