impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3480: Add query options for min/max filter sizes
Date Wed, 11 May 2016 17:03:20 GMT
Henry Robinson has posted comments on this change.

Change subject: IMPALA-3480: Add query options for min/max filter sizes
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2966/2/be/src/service/query-options.cc
File be/src/service/query-options.cc:

Line 352:         query_options->__set_runtime_filter_min_size(size);
> you could simply check the set values after your 'if', and then return an e
I'm not quite following, sorry! Here's the case I'm worried about:

The caller is trying to set both min and max, and this method is being called in a loop to
set each query option one-at-a-time. If min is processed before max, there's no way of getting
at the as-yet unset max value in this method. So looking at query_options.runtime_filter_max_size
would give either the default, or the previously set value, neither of which are valid because
they'll get overwritten as soon as the max gets set in the next loop iteration from the caller
(see impala-hs2-server.cc for an example of how SetQueryOption() is called).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c13c200a0f1855f38a5da50ca34a737e741868b
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message