Skip to content

Commit

Permalink
clustering_range_walker: fix false discontiguity detected after a sta…
Browse files Browse the repository at this point in the history
…tic row

clustering_range_walker detects when we jump from one row range to another. When
a static row is included in the query, the constructor sets up the first before/after
bounds to be exactly that static row. That creates an artificial range crossing if
the first clustering range is contiguous with the static row.

This can cause the index to be consulted needlessly if we happen to fall back
to sstable_mutation_reader after reading the static row.

A unit test is added.

Ref #7883.
  • Loading branch information
avikivity committed Feb 1, 2021
1 parent bb202db commit 7634a90
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
5 changes: 5 additions & 0 deletions clustering_ranges_walker.hh
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ private:
_current_start = position_in_partition_view::for_range_start(_current_range.front());
_current_end = position_in_partition_view::for_range_end(_current_range.front());
}
} else {
// If the first range is contiguous with the static row, then advance _current_end as much as we can
if (_current_range && !_current_range.front().start()) {
_current_end = position_in_partition_view::for_range_end(_current_range.front());
}
}
}

Expand Down
26 changes: 26 additions & 0 deletions test/boost/clustering_ranges_walker_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,3 +289,29 @@ SEASTAR_TEST_CASE(test_range_overlap) {

return make_ready_future<>();
}

SEASTAR_TEST_CASE(verify_static_row_can_be_contiguous_with_a_clustering_range_that_is_open_in_front) {
simple_schema s;
auto keys = s.make_ckeys(10);
auto range1 = query::clustering_range::make_ending_with({keys[2], false});
auto ranges = query::clustering_row_ranges({range1});
auto walker = clustering_ranges_walker(*s.schema(), ranges);
auto cc = walker.lower_bound_change_counter();
walker.advance_to(position_in_partition::for_key(keys[1]));
BOOST_REQUIRE_EQUAL(cc, walker.lower_bound_change_counter());
walker.advance_to(position_in_partition::for_key(keys[3]));
BOOST_REQUIRE_NE(cc, walker.lower_bound_change_counter());
return make_ready_future<>();
}

SEASTAR_TEST_CASE(verify_static_row_can_be_contiguous_with_a_clustering_range_negative_tests) {
simple_schema s;
auto keys = s.make_ckeys(10);
auto range1 = query::clustering_range::make({keys[2], false}, {keys[4], false});
auto ranges = query::clustering_row_ranges({range1});
auto walker = clustering_ranges_walker(*s.schema(), ranges);
auto cc = walker.lower_bound_change_counter();
walker.advance_to(position_in_partition::for_key(keys[3]));
BOOST_REQUIRE_NE(cc, walker.lower_bound_change_counter());
return make_ready_future<>();
}

0 comments on commit 7634a90

Please sign in to comment.