Key Range Query adjustments #414
Conversation
There was a problem hiding this comment.
This is too complicated.
I don't think you need to change anything except for _bisect_and_diff_tables(). And possibly change query_key_range() signature to query_key_range(self, get_min=True, get_max=True). (though always querying both min and max, and discarding the one you don't need, is simpler, and won't affect performance in a meaningful way)
(there would be more code comments, but first I think the approach needs to be simplified)
data_diff/diff_tables.py
Outdated
|
|
||
| # Query min/max values | ||
| key_ranges = self._threaded_call_as_completed("query_key_range", [table1, table2]) | ||
| if all([table1.min_key, table1.max_key, table2.min_key, table2.max_key]): |
There was a problem hiding this comment.
You should test for is None, because keys can be 0.
There was a problem hiding this comment.
Ah true, will fix that
|
Ok I'll simplify things :) |
| ): | ||
| ... | ||
|
|
||
| def _resolve_key_range(self, key_range_res, usr_key_range): |
There was a problem hiding this comment.
extracted to a function since this logic is needed twice.
RoderickJDunn
force-pushed
the
range-query-changes
branch
from
78d0a0a to
6b2d3c3
Compare
|
Lmk if this is closer to what you have in mind! |
|
Hey @RoderickJDunn, apologies that this PR has been sitting so long. Is this a change you're still interested in making? |
|
@dlawin I'm using a fork with some additional functionality (including this) and would like to contribute the changes upstream at some point. So I think this can be closed for now, since there doesn't seem to be interest from others in this change. Thanks for checking in! |
|
Sounds good @RoderickJDunn, let us know! |
4 participants
Changes
min_key+max_keyare both passed in as parameters, thequery_key_rangequery is bypassedmin_keyormax_keyare passed in,query_key_rangeis called but the min or max is overridden with the user-provided value.fixes #401