IMPALA-4335: Don't send 0-row batches to clients This patch restores some behaviour from pre-IMPALA-2905 where we would not send 0-row batches to the client. Although 0-row batches are legal, they're not very useful for clients to receive (and clients may not correctly process them).
No query was found which reliably produced 0-row batches, so no test is added. Change-Id: I7d339c1f9a55d9d75fb0e97d16b3176cc34f2171 Reviewed-on: http://gerrit.cloudera.org:8080/4787 Reviewed-by: Henry Robinson <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/48085274 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/48085274 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/48085274 Branch: refs/heads/master Commit: 48085274fa8ae57453477db21dae0e53eae6b766 Parents: e39f167 Author: Henry Robinson <[email protected]> Authored: Fri Oct 21 15:38:34 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Sat Oct 22 04:49:39 2016 +0000 ---------------------------------------------------------------------- be/src/exec/plan-root-sink.cc | 8 ++++++-- be/src/runtime/coordinator.h | 1 - 2 files changed, 6 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/48085274/be/src/exec/plan-root-sink.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/plan-root-sink.cc b/be/src/exec/plan-root-sink.cc index bd73953..c728f4a 100644 --- a/be/src/exec/plan-root-sink.cc +++ b/be/src/exec/plan-root-sink.cc @@ -84,7 +84,11 @@ void ValidateCollectionSlots(const RowDescriptor& row_desc, RowBatch* batch) { Status PlanRootSink::Send(RuntimeState* state, RowBatch* batch) { ValidateCollectionSlots(row_desc_, batch); int current_batch_row = 0; - do { + + // Don't enter the loop if batch->num_rows() == 0; no point triggering the consumer with + // 0 rows to return. Be wary of ever returning 0-row batches to the client; some poorly + // written clients may not cope correctly with them. See IMPALA-4335. + while (current_batch_row < batch->num_rows()) { unique_lock<mutex> l(lock_); while (results_ == nullptr && !consumer_done_) sender_cv_.wait(l); if (consumer_done_ || batch == nullptr) { @@ -114,7 +118,7 @@ Status PlanRootSink::Send(RuntimeState* state, RowBatch* batch) { results_ = nullptr; ExprContext::FreeLocalAllocations(output_expr_ctxs_); consumer_cv_.notify_all(); - } while (current_batch_row < batch->num_rows()); + } return Status::OK(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/48085274/be/src/runtime/coordinator.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/coordinator.h b/be/src/runtime/coordinator.h index f73cf42..9904def 100644 --- a/be/src/runtime/coordinator.h +++ b/be/src/runtime/coordinator.h @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. - #ifndef IMPALA_RUNTIME_COORDINATOR_H #define IMPALA_RUNTIME_COORDINATOR_H
