impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joe McDonnell (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5039: Fix variability in parquet dictionary filtering test
Date Fri, 10 Mar 2017 01:16:04 GMT
Joe McDonnell has posted comments on this change.

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

Patch Set 2:

File tests/common/

Line 495:   print field_regex
> why print? does this go anywhere useful?
Removed. This was just leftover debugging.

Line 537:     expected_regexes.append(try_compile_regex(expected_line))
> What's the benefit of having one entry per expected_line in both these list
You can think of each of these as sparse lists of mutually exclusive things. An expected line
is either a regex, an aggregation or an exact match. None entries indicate the absence of
a regex or aggregation. Including the None entries means the lists are all the same size and
have the same indices. So, for a given index into the expected lines, the expected_regexes
tells you whether it is a regex or not. The expected_aggregations tells you whether it is
an aggregation or not. If it is neither, then it is an exact match.

This also allows you to walk through the expected lines in order and have a single "matched"
list across all expected lines.

I worked up a version that had separate lists, but it ended up not being much cleaner than
the current code.

I hope this makes sense.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b7b84d973b3ac678a24e82900f2637d569158bb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-HasComments: Yes

View raw message