impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3535: Ignore invalid per-pool default query options
Date Mon, 16 May 2016 00:20:17 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3535: Ignore invalid per-pool default query options
......................................................................


Patch Set 1:

(2 comments)

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

Line 432: ignore_invalid_options
> Not sure I follow. MergeStatus can deal with multiple errors and pass back 
Yeah you're right


Line 451:     if (ignore_invalid_options) {
> The test to catch this bug is to add an incorrect query option key=option t
In the case we expect this to error out, this would be false, so it will always hit l456 which
is what we want. In the case we want to ignore errors (only the pool defaults, right now),
this is true, so we never RETURN_IF_ERROR. So effectively it's the same behavior as just ignoring
errors, but the bug is that we log this regardless. The test cases cover both of these cases
wrt whether or not a failure was expected or not, we just don't have checks for the incorrect
logging.

However, I guess this is just too complicated... I'll just go with the MergeStatus approach.
Anyway, I think I misunderstood what you were saying at first but I see what you're saying
now, and I agree that is cleaner.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If04733b775963091b0314c65286df126fd812358
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message