Skip to content

Commit

Permalink
db: respect changed filters in SetOptions
Browse files Browse the repository at this point in the history
Rebuild the iterator stack if filters (block or table) are modified.

Fix cockroachdb#1621.
  • Loading branch information
jbowens committed Apr 6, 2022
1 parent c8e7d41 commit 4ab231e
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 2 deletions.
24 changes: 23 additions & 1 deletion data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,22 @@ func runIterCmd(d *datadriven.TestData, iter *Iterator, closeIter bool) string {
iter.SetBounds(lower, upper)
valid = iter.Valid()
case "set-options":
const usageString = "set-options [lower=<lower>] [upper=<upper>] [key-types=point|range|both] [mask-suffix=<suffix>]\n"
const usageString = "set-options [lower=<lower>] [upper=<upper>] [key-types=point|range|both] [mask-suffix=<suffix>] [only-durable=<bool>] [table-filter=reuse|none] [point-filters=reuse|none]\n"
var opts IterOptions
for _, part := range parts[1:] {
arg := strings.SplitN(part, "=", 2)
if len(arg) != 2 {
return usageString
}
switch arg[0] {
case "point-filters":
switch arg[1] {
case "reuse":
opts.PointKeyFilters = iter.opts.PointKeyFilters
case "none":
default:
return fmt.Sprintf("set-options: unknown arg point-filter=%q:\n%s", arg[1], usageString)
}
case "lower":
opts.LowerBound = []byte(arg[1])
case "upper":
Expand All @@ -177,6 +185,20 @@ func runIterCmd(d *datadriven.TestData, iter *Iterator, closeIter bool) string {
}
case "mask-suffix":
opts.RangeKeyMasking.Suffix = []byte(arg[1])
case "table-filter":
switch arg[1] {
case "reuse":
opts.TableFilter = iter.opts.TableFilter
case "none":
default:
return fmt.Sprintf("set-options: unknown arg table-filter=%q:\n%s", arg[1], usageString)
}
case "only-durable":
var err error
opts.OnlyReadGuaranteedDurable, err = strconv.ParseBool(arg[1])
if err != nil {
return err.Error()
}
default:
return fmt.Sprintf("set-options: unknown arg %q:\n%s", arg[0], usageString)
}
Expand Down
24 changes: 23 additions & 1 deletion iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1739,11 +1739,33 @@ func (i *Iterator) SetOptions(o *IterOptions) {
}
i.iterValidityState = IterExhausted

var closePoint, closeRange bool
// If OnlyReadGuaranteedDurable changed, the iterator stacks are incorrect,
// improperly including or excluding memtables. Invalidate them so that
// finishInitializingIter will reconstruct them.
if i.opts.OnlyReadGuaranteedDurable != o.OnlyReadGuaranteedDurable {
closePoint = closePoint || i.opts.OnlyReadGuaranteedDurable != o.OnlyReadGuaranteedDurable

// If either the original options or the new options specify a table filter,
// we need to reconstruct the iterator stacks. If they both supply a table
// filter, we can't be certain that it's the same filter since we have no
// mechanism to compare the filter closures.
closePoint = closePoint || i.opts.TableFilter != nil || o.TableFilter != nil
closeRange = closeRange || i.opts.TableFilter != nil || o.TableFilter != nil

// If either options specify block property filters for an iterator stack,
// reconstruct it.
// TODO(jackson): Expose a InternalIterator.SetOptions function and
// propagate changed filters to the existing iterator stack. There will
// likely be complications with determinism.
closePoint = closePoint || len(i.opts.PointKeyFilters) > 0 || len(o.PointKeyFilters) > 0
closeRange = closeRange || len(i.opts.RangeKeyFilters) > 0 || len(o.RangeKeyFilters) > 0

if closePoint && i.pointIter != nil {
i.err = firstError(i.err, i.pointIter.Close())
i.pointIter = nil
}
if closeRange && i.rangeKey != nil {
i.err = firstError(i.err, i.rangeKey.iter.Close())
i.rangeKey = nil
}

Expand Down
31 changes: 31 additions & 0 deletions testdata/iterator_block_interval_filter
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,37 @@ d0704:d
d0704:d
.

# Repeat the above test, but calling set-options before iteration to set the
# same filter. The results should be identical.
iter id_lower_upper=(3,4,5) id_lower_upper=(5,7,9)
set-options point-filters=reuse
first
next
prev
prev
----
.
d0704:d
.
d0704:d
.

# Repeat the above test, but calling set-options before iteration to remove the
# filter.
iter id_lower_upper=(3,4,5) id_lower_upper=(5,7,9)
set-options point-filters=none
first
next
prev
prev
----
.
a1001:a
b0902:b
a1001:a
.


# Iterate with filter id=3 and id=5, where the two admitted sets are
# non-empty, but the intersection is empty.
iter id_lower_upper=(3,4,5) id_lower_upper=(5,8,9)
Expand Down
16 changes: 16 additions & 0 deletions testdata/iterator_table_filter
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,19 @@ iter filter=1
first
----
.

# Set-options that reuses the filter should still see the filter apply.
# Set-options that removes the filter should not.

iter filter=4
first
set-options table-filter=reuse
first
set-options table-filter=none
first
----
a:3
.
a:3
.
a:4

0 comments on commit 4ab231e

Please sign in to comment.