Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS ...................................................................... Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc File be/src/runtime/buffered-tuple-stream.cc: http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc@763 PS8, Line 763: has_read_write_page Is this always safe? If next row will not fit into the page then the write page will be advanced on write. So calling AddRow() and GetNext() will decrease the pin count of the current page to 0. ReadWrite streams are only used by AnalyticEvalNode() - I looked at it a bit but I am not sure yet whether this can cause a problem or not. My idea for a safe solution is that if attach_on_read_ is false, then MarkFlushResources() has to be called for read_write pages, while if attach_on_read_ is true, then the page has to be attached to the RowBatch in the next GetNext() call before calling NextReadPage(). -- 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: 8 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-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 26 Jul 2018 13:52:31 +0000 Gerrit-HasComments: Yes
