Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in aggs and joins ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11007/3/be/src/runtime/buffered-tuple-stream-test.cc File be/src/runtime/buffered-tuple-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/11007/3/be/src/runtime/buffered-tuple-stream-test.cc@801 PS3, Line 801: true Shouldn't we pass attach_on_read here? http://gerrit.cloudera.org:8080/#/c/11007/3/be/src/runtime/buffered-tuple-stream-test.cc@805 PS3, Line 805: ASSERT_OK(stream.GetNext(out_batch, &eos)); Can you add an assert to check that out_batch->num_buffers() == 0 before GetNext()? http://gerrit.cloudera.org:8080/#/c/11007/3/be/src/runtime/buffered-tuple-stream.h File be/src/runtime/buffered-tuple-stream.h: http://gerrit.cloudera.org:8080/#/c/11007/3/be/src/runtime/buffered-tuple-stream.h@184 PS3, Line 184: /// caller is responsible for managing the lifetime of those buffers. : /// 3. If the stream is unpinned and not in attach on read mode, then the batch returned : /// fro nit: is there a reason behind indenting these line with +1 space? -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Tue, 24 Jul 2018 13:57:15 +0000 Gerrit-HasComments: Yes
