Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9005 )

Change subject: IMPALA-6314: Add run time scalar subquery check for 
uncorrelated subqueries
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9005/6/be/src/exec/cardinality-check-node.h
File be/src/exec/cardinality-check-node.h:

http://gerrit.cloudera.org:8080/#/c/9005/6/be/src/exec/cardinality-check-node.h@44
PS6, Line 44: single row from child
> generalize the "single"
Done


http://gerrit.cloudera.org:8080/#/c/9005/6/be/src/exec/cardinality-check-node.h@45
PS6, Line 45: scalar
> perhaps "child" is a better name?
renamed it to simply 'row_batch_' because during copying the name 
'child_row_batch_' was confusing.


http://gerrit.cloudera.org:8080/#/c/9005/6/be/src/exec/cardinality-check-node.cc
File be/src/exec/cardinality-check-node.cc:

http://gerrit.cloudera.org:8080/#/c/9005/6/be/src/exec/cardinality-check-node.cc@51
PS6, Line 51: more than one row.
> should not be specific to "one"
woops, thanks


http://gerrit.cloudera.org:8080/#/c/9005/6/be/src/exec/cardinality-check-node.cc@89
PS6, Line 89: *eos = true;
> if max_cardinality_ > number of tuples in a batch, does this work?
No, in this case RowBatch::DeepCopyTo fails.

This problem is also present in Open, however in Open we create the RowBatch 
and we can set the capacity to max_cardinality. Maybe we can assume that 
max_cardinality is not too huge. If we cannot assume that, then I think we 
cannot implement it as a blocking node.

In GetNext, we get an already created output_row_batch and we can't change its 
capacity.
So I changed the behavior to something similar to SelectNode::GetNext().



--
To view, visit http://gerrit.cloudera.org:8080/9005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f52b93a60eeacedd242a2f17fa6b99c4fc38e06
Gerrit-Change-Number: 9005
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Attila Jeges <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 16 Jan 2018 18:09:29 +0000
Gerrit-HasComments: Yes

Reply via email to