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 <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
