Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201710061824.v96IOwaD028346@ip-10-146-233-104.ec2.internal>
Date: Fri, 6 Oct 2017 18:24:58 +0000
From: "Thomas Tauber-Marshall (Code Review)" <gerrit@cloudera.org>
To: Tianyi Wang <twang@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-2758=3A_Change_BufferedTupleStream=3A=3AGetRows_to_returning_multi_batches=0A?=
In-Reply-To: <gerrit.1507310620000.I3831c38994da2b69775a9809ff01de5d23584414@gerrit.cloudera.org>
References: <gerrit.1507310620000.I3831c38994da2b69775a9809ff01de5d23584414@gerrit.cloudera.org>
Reply-To: tmarshall@cloudera.com, impala-cr@cloudera.com, twang@cloudera.com,
        marcelk@gmail.com, reviews@impala.incubator.apache.org
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="XaYtyG98YBk="; charset=UTF-8
archived-at: Fri, 06 Oct 2017 18:25:05 -0000

--XaYtyG98YBk=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Thomas Tauber-Marshall has posted comments on this change=2E ( http://gerri=
t=2Ecloudera=2Eorg:8080/8226 )

Change subject: IMPALA-2758: Change Buffere=
dTupleStream::GetRows to returning multi batches
=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E


Patch Set 1:

(3 comments)

http://g=
errit=2Ecloudera=2Eorg:8080/#/c/8226/1//COMMIT_MSG
Commit Message:

http://=
gerrit=2Ecloudera=2Eorg:8080/#/c/8226/1//COMMIT_MSG@14
PS1, Line 14: 
Any t=
esting?


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8226/1/be/src/exec/partit=
ioned-hash-join-node=2Ecc
File be/src/exec/partitioned-hash-join-node=2Ecc:=


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8226/1/be/src/exec/partitioned-ha=
sh-join-node=2Ecc@950
PS1, Line 950: break;
I think this changes the behavi=
or here - this will only break out of the FOREACH_ROW loop, but what we wan=
t is to stop iterating over the entire set of build rows (that was previous=
ly one batch, but is now the same set of rows split into multiple batches)=
=2E


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8226/1/be/src/runtime/buffere=
d-tuple-stream=2Eh
File be/src/runtime/buffered-tuple-stream=2Eh:

http://g=
errit=2Ecloudera=2Eorg:8080/#/c/8226/1/be/src/runtime/buffered-tuple-stream=
=2Eh@339
PS1, Line 339: vector<std::unique_ptr<RowBatch>>* batch
Can you mo=
ve this to be after 'batch_size'? We generally keep out parameters as the t=
railing parameters for clarity=2E


-- 
To view, visit http://gerrit=2Eclo=
udera=2Eorg:8080/8226
To unsubscribe, visit http://gerrit=2Ecloudera=2Eorg:=
8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Mess=
ageType: comment
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d2358441=
4
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <=
twang@cloudera=2Ecom>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cl=
oudera=2Ecom>
Gerrit-Comment-Date: Fri, 06 Oct 2017 18:24:58 +0000
Gerrit-H=
asComments: Yes

--XaYtyG98YBk=--