Dan Hecht has posted comments on this change. Change subject: IMPALA-4674: Part 3: fix null-aware anti join ......................................................................
Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/7367/10/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: PS10, Line 330: int i = 0; i < PARTITION_FANOUT; ++i) doesn't look like you need 'i' anymore so could make this range based http://gerrit.cloudera.org:8080/#/c/7367/10/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: PS10, Line 205: frees all reservation does it free the reservation or the buffers? doesn't it hold on to the reservation but just free up the use of the reservation (i.e. the buffers)? http://gerrit.cloudera.org:8080/#/c/7367/10/be/src/exec/partitioned-hash-join-node-ir.cc File be/src/exec/partitioned-hash-join-node-ir.cc: PS10, Line 440: AppendSpilledProbeRow why add this rather than just always using the new AppendProbeRow() (maybe with a dcheck that we only end up on the slow path if not spilled)? http://gerrit.cloudera.org:8080/#/c/7367/10/tests/query_test/test_spilling.py File tests/query_test/test_spilling.py: Line 64: """Null-aware anti-join tests that will not pass with the debug action enabled""" why don't they work? -- To view, visit http://gerrit.cloudera.org:8080/7367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2e60eb4dd32bd287a31479a6232400df65964c1 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
