[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-08 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-node.cc@339
PS4, Line 339: if (state_ == PROBING_SPILLED_PARTITION && 
NeedToProcessUnmatchedBuildRows(join_op_)) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-node.cc@1154
PS4, Line 1154: hash_tbl_iterator_ = 
output_build_partitions_.front()->hash_tbl()->FirstUnmatched(ht_ctx_.get());
line too long (101 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 08 Nov 2019 14:31:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-08 Thread Tim Armstrong (Code Review) 
Tim Armstrong has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..

IMPALA-9126: part 1: hash join build partition cleanup

Clarify some invariants (e.g. which join modes actually
require build partition data to be attached to row batches).

Move some logic for maintenance of the hash partitions to
the builder.

Testing:
Ran exhaustive tests. We should already have adequate coverage for
spilling and non-spilling hash joins.

Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/blocking-join-node.h
A be/src/exec/join-op.h
M be/src/exec/nested-loop-join-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
8 files changed, 146 insertions(+), 92 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/14632/4
--
To view, visit http://gerrit.cloudera.org:8080/14632
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-08 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 4:

This is a very incremental bit of cleanup that will help with separating the 
build side for parallel plans, but I think is self contained and ready so I 
pushed it out for review.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Nov 2019 14:32:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-08 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 4:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Nov 2019 15:14:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Thomas Tauber-Marshall (Code Review) 
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h@119
PS4, Line 119:
nit: formatting


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h@124
PS4, Line 124:   // We don't need to pass in a batch because the anti-join 
only returns tuple data
Is it possible to DCHECK for this? I guess it would be 
null_aware_partition_->build_rows()->num_rows() == 0? Maybe unnecessarily 
defensive


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h@163
PS4, Line 163:   /// Needs to be log2(PARTITION_FANOUT).
Might make sense to move this and MAX_PARTITION_DEPTH up too, to keep the 
related static const stuff all together.


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.cc
File be/src/exec/partitioned-hash-join-builder.cc:

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.cc@483
PS4, Line 483: stream->Close(nullptr, 
RowBatch::FlushMode::NO_FLUSH_RESOURCES);
Maybe comment/DCHECK why we don't need to pass row_batch here



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 19 Nov 2019 20:08:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Tim Armstrong (Code Review) 
Hello Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..

IMPALA-9126: part 1: hash join build partition cleanup

Clarify some invariants (e.g. which join modes actually
require build partition data to be attached to row batches).

Move some logic for maintenance of the hash partitions to
the builder.

Testing:
Ran exhaustive tests. We should already have adequate coverage for
spilling and non-spilling hash joins.

Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/blocking-join-node.h
A be/src/exec/join-op.h
M be/src/exec/nested-loop-join-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
8 files changed, 164 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/14632/5
--
To view, visit http://gerrit.cloudera.org:8080/14632
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14632/5/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/14632/5/be/src/exec/partitioned-hash-join-node.cc@339
PS5, Line 339: if (state_ == PROBING_SPILLED_PARTITION && 
NeedToProcessUnmatchedBuildRows(join_op_)) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/14632/5/be/src/exec/partitioned-hash-join-node.cc@1154
PS5, Line 1154: hash_tbl_iterator_ = 
output_build_partitions_.front()->hash_tbl()->FirstUnmatched(ht_ctx_.get());
line too long (101 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 19 Nov 2019 21:54:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Tim Armstrong (Code Review) 
Hello Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..

IMPALA-9126: part 1: hash join build partition cleanup

Clarify some invariants (e.g. which join modes actually
require build partition data to be attached to row batches).

Move some logic for maintenance of the hash partitions to
the builder.

Testing:
Ran exhaustive tests. We should already have adequate coverage for
spilling and non-spilling hash joins.

Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/blocking-join-node.h
A be/src/exec/join-op.h
M be/src/exec/nested-loop-join-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
8 files changed, 166 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/14632/6
--
To view, visit http://gerrit.cloudera.org:8080/14632
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h@119
PS4, Line 119:
> nit: formatting
Done


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h@124
PS4, Line 124:   // We don't need to pass in a batch because the anti-join 
only returns tuple data
> Is it possible to DCHECK for this? I guess it would be null_aware_partition
I don't think there's a direct way to assert on it. It follows from the join 
node not including this tuple in its output row descriptor. I added that fact 
to the comment. Seems like an improvement since the reader can verify that fact 
(e.g. see the DCHECK in BlockingJoinNode::Prepare()).


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.h@163
PS4, Line 163:   /// Needs to be log2(PARTITION_FANOUT).
> Might make sense to move this and MAX_PARTITION_DEPTH up too, to keep the r
good point


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.cc
File be/src/exec/partitioned-hash-join-builder.cc:

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-builder.cc@483
PS4, Line 483: stream->Close(nullptr, 
RowBatch::FlushMode::NO_FLUSH_RESOURCES);
> Maybe comment/DCHECK why we don't need to pass row_batch here
Done


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-node.cc@339
PS4, Line 339: if (state_ == PROBING_SPILLED_PARTITION && 
NeedToProcessUnmatchedBuildRows(join_op_)) {
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/14632/4/be/src/exec/partitioned-hash-join-node.cc@1154
PS4, Line 1154: hash_tbl_iterator_ = 
output_build_partitions_.front()->hash_tbl()->FirstUnmatched(ht_ctx_.get());
> line too long (101 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 19 Nov 2019 21:55:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 5:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 19 Nov 2019 22:37:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-19 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 6:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 19 Nov 2019 22:38:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-20 Thread Thomas Tauber-Marshall (Code Review) 
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Nov 2019 18:17:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-20 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Nov 2019 18:20:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-20 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Nov 2019 18:20:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-20 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 8: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5258/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Nov 2019 04:20:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-21 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Nov 2019 08:31:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-21 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Nov 2019 08:31:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-21 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 9: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5265/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Nov 2019 13:07:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-21 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Nov 2019 18:09:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-21 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Nov 2019 22:54:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup

2019-11-21 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/14632 )

Change subject: IMPALA-9126: part 1: hash join build partition cleanup
..

IMPALA-9126: part 1: hash join build partition cleanup

Clarify some invariants (e.g. which join modes actually
require build partition data to be attached to row batches).

Move some logic for maintenance of the hash partitions to
the builder.

Testing:
Ran exhaustive tests. We should already have adequate coverage for
spilling and non-spilling hash joins.

Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Reviewed-on: http://gerrit.cloudera.org:8080/14632
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/blocking-join-node.h
A be/src/exec/join-op.h
M be/src/exec/nested-loop-join-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
8 files changed, 166 insertions(+), 106 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb
Gerrit-Change-Number: 14632
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong