Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/8196 )
Change subject: IMPALA-4236: Codegen CopyRows() for select nodes ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/8196/7/be/src/exec/select-node-ir.cc File be/src/exec/select-node-ir.cc: http://gerrit.cloudera.org:8080/#/c/8196/7/be/src/exec/select-node-ir.cc@28 PS7, Line 28: limit_ > This could return extra rows. E.g. if the limit for the node is 1025, we re Done. also added a test case that checks if limit is enforced by select node. planner tests already exist that check if limit is pushed to a select node, so I have only added a query test. http://gerrit.cloudera.org:8080/#/c/8196/7/be/src/exec/select-node.cc File be/src/exec/select-node.cc: http://gerrit.cloudera.org:8080/#/c/8196/7/be/src/exec/select-node.cc@91 PS7, Line 91: DCHECK(*eos == false); > This isn't safe - the GetNext() API doesn't require that the caller initial Done http://gerrit.cloudera.org:8080/#/c/8196/7/be/src/exec/select-node.cc@112 PS7, Line 112: > nit: remove extra vertical whitespace Done -- To view, visit http://gerrit.cloudera.org:8080/8196 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d496d004418468e16b6f564f90f45ebbf87c1e Gerrit-Change-Number: 8196 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 17 Oct 2017 03:02:53 +0000 Gerrit-HasComments: Yes
