impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5039: Fix variability in parquet dictionary filtering test
Date Thu, 09 Mar 2017 06:08:27 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-5039: Fix variability in parquet dictionary filtering test
......................................................................


Patch Set 2:

(2 comments)

Thanks for doing this! Very useful.

http://gerrit.cloudera.org:8080/#/c/6301/2/tests/common/test_result_verifier.py
File tests/common/test_result_verifier.py:

Line 495:   print field_regex
why print? does this go anywhere useful?


Line 537:     expected_regexes.append(try_compile_regex(expected_line))
What's the benefit of having one entry per expected_line in both these lists? Seems a little
confusing to me, esp. with the None checking below. It seems simpler to have two lists that
contain only non-None entries.

Does it also look like a bug to you to have None entries in expected_regexes? I think that
means we will just accept any garbage in the expected_lines (which in most cases is probably
non-intentional).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b7b84d973b3ac678a24e82900f2637d569158bb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message