From a562cbd3242ab2312fdc41fdf867fc6daf3e8002 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Wed, 14 Oct 2026 12:59:11 -0700 Subject: [PATCH 01/10] Replace splinterdb_iterator_init variants with options struct + macro Co-Authored-By: Claude Sonnet 5.5 --- benchmarks/hash_range_benchmark.c | 11 +- docs/filtered-ranges-pr.md | 2 +- docs/hash-filtered-ranges.md | 11 +- include/splinterdb/splinterdb.h | 151 ++++++------ src/splinterdb.c | 97 ++------ tests/unit/splinterdb_quick_test.c | 360 +++++++++++++++-------------- 6 files changed, 301 insertions(+), 331 deletions(-) diff --git a/benchmarks/hash_range_benchmark.c b/benchmarks/hash_range_benchmark.c index 7be6887e5..3fcccac5d 100644 --- a/benchmarks/hash_range_benchmark.c +++ b/benchmarks/hash_range_benchmark.c @@ -105,15 +105,16 @@ run_queries(splinterdb *db, write_be(key + 8, 0); splinterdb_iterator *it = NULL; #ifdef PREDICATE_RANGE_FILTER_API - check(splinterdb_iterator_init_with_filter( + check(splinterdb_iterator_init( db, &it, greater_than_or_equal, slice_create(sizeof(key), key), - tenant_matches, - filter, - strcmp(mode, "pruned") == 0 ? slice_create(sizeof(hash_key), hash_key) - : NULL_SLICE)); + .predicate = tenant_matches, + .predicate_context = filter, + .routing_filter_key = strcmp(mode, "pruned") == 0 + ? slice_create(sizeof(hash_key), hash_key) + : NULL_SLICE)); #else check(splinterdb_iterator_init( db, &it, greater_than_or_equal, slice_create(sizeof(key), key))); diff --git a/docs/filtered-ranges-pr.md b/docs/filtered-ranges-pr.md index 95bb7e7fd..75f2f048b 100644 --- a/docs/filtered-ranges-pr.md +++ b/docs/filtered-ranges-pr.md @@ -1,4 +1,4 @@ -Allow range iterators to apply an exact key predicate, with optional routing-filter branch pruning. The new `splinterdb_iterator_init_with_filter()` and `splinterdb_iterator_init_with_bounds_and_filter()` take separate predicate, context and representative-key arguments. A predicate alone scans all relevant branches; a representative routing key requires a predicate and asserts hash compatibility under every possible seed. Pruning without a predicate returns `EINVAL`. +Allow range iterators to apply an exact key predicate, with optional routing-filter branch pruning. `splinterdb_iterator_init()` takes optional `.predicate`, `.predicate_context` and `.routing_filter_key` parameters. A predicate alone scans all relevant branches; a representative routing key requires a predicate and asserts hash compatibility under every possible seed. Pruning without a predicate returns `EINVAL`. Rejected keys are skipped before application merge/finalize callbacks, preserving complete update histories for accepted keys even when routing filters admit collisions. The iterator owns its routing-key copy across trunk spans, direction changes and thread handoff; predicate context remains caller-owned. Ordinary iterator APIs remain unchanged and do not allocate filter state. No callback is added to `data_config`. diff --git a/docs/hash-filtered-ranges.md b/docs/hash-filtered-ranges.md index 31ba69c8b..8fab8d30e 100644 --- a/docs/hash-filtered-ranges.md +++ b/docs/hash-filtered-ranges.md @@ -6,11 +6,12 @@ integration from `36183807` on top of main's persisted configuration. The main update leaves the previously tested implementation unchanged; see the [main-update proof](../benchmarks/results-20261013/filtered-main-1d6ea2a3/README.txt). -`splinterdb_iterator_init_with_filter()` and -`splinterdb_iterator_init_with_bounds_and_filter()` append three separate -arguments to the corresponding ordinary initializer: a -`splinterdb_key_predicate`, its `void *` context, and a `slice` -`routing_filter_key`. There is no new `data_config` callback. +`splinterdb_iterator_init()` accepts optional designated-initializer +parameters (see `splinterdb_iterator_init_options`). The filter parameters are +`.predicate` (a `splinterdb_key_predicate`), `.predicate_context` (a `void *`), +and `.routing_filter_key` (a `slice`); they combine freely with the `.min_key`, +`.min_key_comparison`, `.max_key` and `.max_key_comparison` bounds parameters. +There is no new `data_config` callback. | Predicate | Routing-filter key | Behavior | | --- | --- | --- | diff --git a/include/splinterdb/splinterdb.h b/include/splinterdb/splinterdb.h index a00f6ab86..5dee2306f 100644 --- a/include/splinterdb/splinterdb.h +++ b/include/splinterdb/splinterdb.h @@ -564,40 +564,6 @@ typedef enum comparison { } comparison; -// Initialize a new iterator, starting at the given key -// -// If start_key is NULL_SLICE, the iterator will start at: -// - greater_than_or_equal: the minimum key -// - less_than_or_equal: the maximum key -int -splinterdb_iterator_init(splinterdb *kvs, // IN - splinterdb_iterator **iter, // OUT - comparison start_type, // IN - slice start_key // IN -); - -// Initialize a new iterator with lower and upper bounds. -// -// If min_key is NULL_SLICE, the iterator has no lower bound. -// min_key_comparison must be greater_than or greater_than_or_equal. -// -// If max_key is NULL_SLICE, the iterator has no upper bound. -// max_key_comparison must be less_than or less_than_or_equal. -// -// If start_key is NULL_SLICE, the iterator will start at: -// - greater_than_or_equal / greater_than: the first key in bounds -// - less_than_or_equal / less_than: the last key in bounds -int -splinterdb_iterator_init_with_bounds(splinterdb *kvs, // IN - splinterdb_iterator **iter, // OUT - comparison min_key_comparison, // IN - slice min_key, // IN - comparison max_key_comparison, // IN - slice max_key, // IN - comparison start_type, // IN - slice start_key // IN -); - // A predicate over stored keys. The key slice is borrowed for this call only. // The predicate may run repeatedly, including for keys that were deleted, and // before values are merged. It must return the same result for keys that @@ -606,44 +572,89 @@ splinterdb_iterator_init_with_bounds(splinterdb *kvs, // IN // the iterator. typedef bool (*splinterdb_key_predicate)(void *context, slice key); -// Initialize an iterator that returns only keys accepted by predicate. -// NULL predicate with NULL_SLICE routing_filter_key is an ordinary scan. -// A predicate without a routing_filter_key scans all branches in the range. -// -// An optional routing_filter_key enables branch pruning. For EVERY seed, the -// application must guarantee that every key accepted by predicate has the same -// data_config.key_hash value as routing_filter_key. Violating this contract can -// omit results or parts of their update histories. The routing key uses the -// query-key representation accepted by key_hash and is copied by -// initialization; its original storage need not remain alive. NULL_SLICE -// disables pruning. A routing_filter_key without a predicate is invalid and -// returns EINVAL. -// -// Filtering runs before message merging; it does not change snapshot isolation. -// Bounds, starting position, direction and errors follow the ordinary iterator -// contract. The callback is not a value predicate and must not mutate the DB. +// Optional parameters for splinterdb_iterator_init. A zero-initialized options +// struct (apart from struct_size) requests an ordinary unbounded, unfiltered +// scan. Don't build one by hand; use the splinterdb_iterator_init macro below, +// which sets struct_size and lets you name just the fields you need. +typedef struct splinterdb_iterator_init_options { + // Set by the splinterdb_iterator_init macro. Lets future versions add + // fields without breaking callers built against this one. + size_t struct_size; + + // Lower bound. If min_key is NULL_SLICE (the default) there is no lower + // bound and min_key_comparison is ignored. Otherwise min_key_comparison + // must be set to greater_than or greater_than_or_equal. + slice min_key; + comparison min_key_comparison; + + // Upper bound. If max_key is NULL_SLICE (the default) there is no upper + // bound and max_key_comparison is ignored. Otherwise max_key_comparison + // must be set to less_than or less_than_or_equal. + slice max_key; + comparison max_key_comparison; + + // If non-NULL, the iterator returns only keys accepted by predicate. + // A predicate without a routing_filter_key scans all branches in the range. + splinterdb_key_predicate predicate; + void *predicate_context; + + // Optional branch pruning for filtered scans. For EVERY seed, the + // application must guarantee that every key accepted by predicate has the + // same data_config.key_hash value as routing_filter_key. Violating this + // contract can omit results or parts of their update histories. The routing + // key uses the query-key representation accepted by key_hash and is copied + // by initialization; its original storage need not remain alive. + // NULL_SLICE (the default) disables pruning. A routing_filter_key without a + // predicate is invalid and returns EINVAL. + slice routing_filter_key; +} splinterdb_iterator_init_options; + +// Initialize a new iterator, starting at the given key. +// +// If start_key is NULL_SLICE, the iterator will start at: +// - greater_than_or_equal / greater_than: the first key in bounds +// - less_than_or_equal / less_than: the last key in bounds +// +// Filtering runs before message merging; it does not change snapshot +// isolation. Bounds, starting position, direction and errors follow the +// ordinary iterator contract. The predicate is not a value predicate and must +// not mutate the DB. int -splinterdb_iterator_init_with_filter(splinterdb *kvs, - splinterdb_iterator **iter, - comparison start_type, - slice start_key, - splinterdb_key_predicate predicate, - void *predicate_context, - slice routing_filter_key); +splinterdb_iterator_init_full( + splinterdb *kvs, // IN + splinterdb_iterator **iter, // OUT + comparison start_type, // IN + slice start_key, // IN + const splinterdb_iterator_init_options *options // IN +); -int -splinterdb_iterator_init_with_bounds_and_filter( - splinterdb *kvs, - splinterdb_iterator **iter, - comparison min_key_comparison, - slice min_key, - comparison max_key_comparison, - slice max_key, - comparison start_type, - slice start_key, - splinterdb_key_predicate predicate, - void *predicate_context, - slice routing_filter_key); +// Convenience wrapper for splinterdb_iterator_init_full. After the required +// arguments, optionally list any of the fields of +// splinterdb_iterator_init_options as designated initializers: +// +// splinterdb_iterator_init(kvs, &it, greater_than_or_equal, NULL_SLICE); +// +// splinterdb_iterator_init(kvs, &it, greater_than_or_equal, start, +// .min_key = lo, +// .min_key_comparison = greater_than_or_equal, +// .max_key = hi, +// .max_key_comparison = less_than); +// +// splinterdb_iterator_init(kvs, &it, greater_than_or_equal, NULL_SLICE, +// .predicate = my_pred, +// .predicate_context = &ctx); +// +// Note that commas inside field values must be parenthesized as usual for +// macro arguments. +#define splinterdb_iterator_init(kvs, iter, start_type, start_key, ...) \ + splinterdb_iterator_init_full( \ + (kvs), \ + (iter), \ + (start_type), \ + (start_key), \ + &(const splinterdb_iterator_init_options){ \ + .struct_size = sizeof(splinterdb_iterator_init_options), \ + __VA_ARGS__}) // Deinitialize an iterator // diff --git a/src/splinterdb.c b/src/splinterdb.c index 823b1455b..0921577a2 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -1156,82 +1156,33 @@ splinterdb_iterator_max_comparison_is_valid(comparison cmp) } int -splinterdb_iterator_init(splinterdb *kvs, // IN - splinterdb_iterator **iter, // OUT - comparison start_key_comparison, // IN - slice user_start_key // IN +splinterdb_iterator_init_full( + splinterdb *kvs, // IN + splinterdb_iterator **iter, // OUT + comparison start_key_comparison, // IN + slice user_start_key, // IN + const splinterdb_iterator_init_options *options // IN ) { - return splinterdb_iterator_init_with_bounds(kvs, - iter, - greater_than_or_equal, - NULL_SLICE, - less_than, - NULL_SLICE, - start_key_comparison, - user_start_key); -} - -int -splinterdb_iterator_init_with_bounds(splinterdb *kvs, // IN - splinterdb_iterator **iter, // OUT - comparison min_key_comparison, // IN - slice user_min_key, // IN - comparison max_key_comparison, // IN - slice user_max_key, // IN - comparison start_key_comparison, // IN - slice user_start_key // IN -) -{ - return splinterdb_iterator_init_with_bounds_and_filter(kvs, - iter, - min_key_comparison, - user_min_key, - max_key_comparison, - user_max_key, - start_key_comparison, - user_start_key, - NULL, - NULL, - NULL_SLICE); -} - -int -splinterdb_iterator_init_with_filter(splinterdb *kvs, - splinterdb_iterator **iter, - comparison start_key_comparison, - slice user_start_key, - splinterdb_key_predicate predicate, - void *predicate_context, - slice routing_filter_key) -{ - return splinterdb_iterator_init_with_bounds_and_filter(kvs, - iter, - greater_than_or_equal, - NULL_SLICE, - less_than, - NULL_SLICE, - start_key_comparison, - user_start_key, - predicate, - predicate_context, - routing_filter_key); -} + if (options == NULL + || options->struct_size < sizeof(splinterdb_iterator_init_options)) + { + return platform_status_to_int(STATUS_BAD_PARAM); + } + slice user_min_key = options->min_key; + slice user_max_key = options->max_key; + splinterdb_key_predicate predicate = options->predicate; + void *predicate_context = options->predicate_context; + slice routing_filter_key = options->routing_filter_key; + comparison min_key_comparison = greater_than_or_equal; + comparison max_key_comparison = less_than; + if (!slice_is_null(user_min_key)) { + min_key_comparison = options->min_key_comparison; + } + if (!slice_is_null(user_max_key)) { + max_key_comparison = options->max_key_comparison; + } -int -splinterdb_iterator_init_with_bounds_and_filter( - splinterdb *kvs, - splinterdb_iterator **iter, - comparison min_key_comparison, - slice user_min_key, - comparison max_key_comparison, - slice user_max_key, - comparison start_key_comparison, - slice user_start_key, - splinterdb_key_predicate predicate, - void *predicate_context, - slice routing_filter_key) -{ if ((predicate == NULL && !slice_is_null(routing_filter_key)) || !splinterdb_iterator_min_comparison_is_valid(min_key_comparison) || !splinterdb_iterator_max_comparison_is_valid(max_key_comparison)) diff --git a/tests/unit/splinterdb_quick_test.c b/tests/unit/splinterdb_quick_test.c index 7dd5dd4ea..b31cde276 100644 --- a/tests/unit/splinterdb_quick_test.c +++ b/tests/unit/splinterdb_quick_test.c @@ -2408,14 +2408,14 @@ CTEST2(splinterdb_quick, test_bounded_iterator) ARRAY_SIZE(singleton)); ASSERT_EQUAL(0, rc); - rc = splinterdb_iterator_init_with_bounds(data->kvsb, - NULL, - less_than, - min, - less_than_or_equal, - max, - greater_than_or_equal, - NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, + NULL, + greater_than_or_equal, + NULL_SLICE, + .min_key_comparison = less_than, + .min_key = min, + .max_key_comparison = less_than_or_equal, + .max_key = max); ASSERT_EQUAL(EINVAL, rc); } @@ -4070,14 +4070,14 @@ check_bounded_iterator_sequence(splinterdb *kvsb, uint64 expected_count) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init_with_bounds(kvsb, - &it, - min_key_comparison, - min_key, - max_key_comparison, - max_key, - start_type, - NULL_SLICE); + int rc = splinterdb_iterator_init(kvsb, + &it, + start_type, + NULL_SLICE, + .min_key_comparison = min_key_comparison, + .min_key = min_key, + .max_key_comparison = max_key_comparison, + .max_key = max_key); ASSERT_EQUAL(0, rc); for (uint64 i = 0; i < expected_count; i++) { @@ -4109,14 +4109,14 @@ check_bounded_iterator_current_from_start(splinterdb *kvsb, int expected_i) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init_with_bounds(kvsb, - &it, - min_key_comparison, - min_key, - max_key_comparison, - max_key, - start_key_comparison, - start_key); + int rc = splinterdb_iterator_init(kvsb, + &it, + start_key_comparison, + start_key, + .min_key_comparison = min_key_comparison, + .min_key = min_key, + .max_key_comparison = max_key_comparison, + .max_key = max_key); ASSERT_EQUAL(0, rc); ASSERT_TRUE(splinterdb_iterator_valid(it)); @@ -4141,14 +4141,14 @@ check_bounded_iterator_boundary_from_start(splinterdb *kvsb, int expected_i) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init_with_bounds(kvsb, - &it, - min_key_comparison, - min_key, - max_key_comparison, - max_key, - start_key_comparison, - start_key); + int rc = splinterdb_iterator_init(kvsb, + &it, + start_key_comparison, + start_key, + .min_key_comparison = min_key_comparison, + .min_key = min_key, + .max_key_comparison = max_key_comparison, + .max_key = max_key); ASSERT_EQUAL(0, rc); ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -4334,14 +4334,14 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) for (int forwards = 0; forwards < 2; forwards++) { splinterdb_iterator *it = NULL; ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter( + splinterdb_iterator_init( data->kvsb, &it, forwards ? greater_than_or_equal : less_than_or_equal, NULL_SLICE, - range_tenant_matches, - &tenant, - slice_create(1, "a"))); + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = slice_create(1, "a"))); const char *expected[] = {"0a", "2a", "3a"}; for (int i = 0; i < 3; i++) { check_current_string_kv(it, expected[forwards ? i : 2 - i], "3"); @@ -4379,17 +4379,17 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) splinterdb_iterator *it = NULL; ASSERT_EQUAL( 0, - splinterdb_iterator_init_with_bounds_and_filter(data->kvsb, - &it, - greater_than, - slice_create(2, "0a"), - less_than, - slice_create(2, "3a"), - greater_than_or_equal, - NULL_SLICE, - range_tenant_matches, - &tenant, - slice_create(1, "a"))); + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .min_key_comparison = greater_than, + .min_key = slice_create(2, "0a"), + .max_key_comparison = less_than, + .max_key = slice_create(2, "3a"), + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = slice_create(1, "a"))); check_current_string_kv(it, "2a", "3"); splinterdb_iterator_next(it); ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -4398,25 +4398,26 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); tenant.tenant = 'z'; - ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - range_tenant_matches, - &tenant, - slice_create(1, "z"))); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = slice_create(1, "z"))); ASSERT_FALSE(splinterdb_iterator_valid(it)); ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - NULL, - NULL, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .predicate = NULL, + .predicate_context = NULL, + .routing_filter_key = NULL_SLICE)); check_current_string_kv(it, "0a", "3"); splinterdb_iterator_next(it); check_current_string_kv(it, "0b", "3"); @@ -4479,15 +4480,16 @@ CTEST2(splinterdb_quick, test_filtered_range_crosses_empty_trunk_leaves) bool no_match = false; uint64 hash_key = 0; splinterdb_iterator *it = NULL; - ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter( - data->kvsb, - &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, - range_sparse_matches, - &no_match, - slice_create(sizeof(hash_key), &hash_key))); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init( + data->kvsb, + &it, + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE, + .predicate = range_sparse_matches, + .predicate_context = &no_match, + .routing_filter_key = slice_create(sizeof(hash_key), &hash_key))); /* Later trunk spans must use the copied pruning key. Its original * storage may be reused immediately after initialization. */ hash_key = UINT64_MAX; @@ -4511,15 +4513,16 @@ CTEST2(splinterdb_quick, test_filtered_range_crosses_empty_trunk_leaves) no_match = true; hash_key = UINT64_MAX; - ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter( - data->kvsb, - &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, - range_sparse_matches, - &no_match, - slice_create(sizeof(hash_key), &hash_key))); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init( + data->kvsb, + &it, + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE, + .predicate = range_sparse_matches, + .predicate_context = &no_match, + .routing_filter_key = slice_create(sizeof(hash_key), &hash_key))); ASSERT_FALSE(splinterdb_iterator_valid(it)); ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); @@ -4642,20 +4645,23 @@ range_mixed_seed_check_scan(splinterdb *db, splinterdb_iterator *it = NULL; range_mixed_seed_seen = 0; range_mixed_seed_track = TRUE; - ASSERT_EQUAL( - 0, - splinterdb_iterator_init_with_bounds_and_filter( - db, - &it, - greater_than, - bounded ? range_mixed_seed_key(lower, low, 'a') : NULL_SLICE, - less_than, - bounded ? range_mixed_seed_key(upper, high, 'a') : NULL_SLICE, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, - range_tenant_matches, - &tenant, - pruned ? slice_create(sizeof(routing_key), routing_key) : NULL_SLICE)); + ASSERT_EQUAL(0, + splinterdb_iterator_init( + db, + &it, + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE, + .min_key_comparison = greater_than, + .min_key = bounded ? range_mixed_seed_key(lower, low, 'a') + : NULL_SLICE, + .max_key_comparison = less_than, + .max_key = bounded ? range_mixed_seed_key(upper, high, 'a') + : NULL_SLICE, + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = + pruned ? slice_create(sizeof(routing_key), routing_key) + : NULL_SLICE)); const uint32 initial_seed_mask = range_mixed_seed_seen; /* The iterator must retain its own routing key for later saved seeds. * a and b collide under seed 11, but differ under seed 22. */ @@ -4831,13 +4837,13 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) splinterdb_iterator *it = NULL; /* No callback is needed on an empty database. */ ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - range_digit_matches, - &digits, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); ASSERT_FALSE(splinterdb_iterator_valid(it)); ASSERT_EQUAL(0, splinterdb_iterator_status(it)); ASSERT_EQUAL_U(0, digits.calls); @@ -4856,14 +4862,14 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) * predicate must visit every branch, including the current memtable. */ for (int forwards = 0; forwards < 2; forwards++) { ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter( - data->kvsb, - &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, - range_digit_matches, - &digits, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + forwards ? greater_than_or_equal + : less_than_or_equal, + NULL_SLICE, + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); for (int i = 0; i < 5; i++) { char expected[] = {(forwards ? 1 + 2 * i : 9 - 2 * i) + '0', 0}; check_current_string_kv(it, expected, "v"); @@ -4878,18 +4884,18 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) splinterdb_iterator_deinit(it); ASSERT_EQUAL(0, - splinterdb_iterator_init_with_bounds_and_filter( - data->kvsb, - &it, - greater_than, - slice_create(1, "1"), - less_than, - slice_create(1, "9"), - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, - range_digit_matches, - &digits, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + forwards ? greater_than_or_equal + : less_than_or_equal, + NULL_SLICE, + .min_key_comparison = greater_than, + .min_key = slice_create(1, "1"), + .max_key_comparison = less_than, + .max_key = slice_create(1, "9"), + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); for (int i = 0; i < 3; i++) { char expected[] = {(forwards ? 3 + 2 * i : 7 - 2 * i) + '0', 0}; check_current_string_kv(it, expected, "v"); @@ -4914,14 +4920,13 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) splinterdb_iterator_deinit(it); ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - forwards ? greater_than - : less_than, - slice_create(1, "5"), - range_digit_matches, - &digits, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + forwards ? greater_than : less_than, + slice_create(1, "5"), + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); check_current_string_kv(it, forwards ? "7" : "3", "v"); ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); @@ -4931,40 +4936,41 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) /* Context remains caller-owned; each iterator borrows its current value. */ digits.remainder = 2; ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - range_digit_matches, - &digits, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); ASSERT_FALSE(splinterdb_iterator_valid(it)); ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); it = NULL; - ASSERT_EQUAL(EINVAL, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - NULL, - NULL, - slice_create(1, "1"))); + ASSERT_EQUAL( + EINVAL, + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .predicate = NULL, + .predicate_context = NULL, + .routing_filter_key = slice_create(1, "1"))); ASSERT_NULL(it); ASSERT_EQUAL( EINVAL, - splinterdb_iterator_init_with_bounds_and_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - less_than_or_equal, - NULL_SLICE, - greater_than_or_equal, - NULL_SLICE, - NULL, - NULL, - slice_create(1, "1"))); + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .min_key_comparison = greater_than_or_equal, + .min_key = NULL_SLICE, + .max_key_comparison = less_than_or_equal, + .max_key = NULL_SLICE, + .predicate = NULL, + .predicate_context = NULL, + .routing_filter_key = slice_create(1, "1"))); ASSERT_NULL(it); } @@ -4990,13 +4996,13 @@ CTEST2(splinterdb_quick, test_filtered_range_propagates_matching_merge_errors) data->kvsb, slice_create(2, "2a"), slice_create(1, "v"), NULL)); splinterdb_iterator *it = NULL; ASSERT_EQUAL(0, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - range_tenant_matches, - &tenant, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + greater_than_or_equal, + NULL_SLICE, + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = NULL_SLICE)); check_current_string_kv(it, "1a", "v"); splinterdb_iterator_next(it); ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -5005,13 +5011,13 @@ CTEST2(splinterdb_quick, test_filtered_range_propagates_matching_merge_errors) it = NULL; ASSERT_EQUAL(ENOMEM, - splinterdb_iterator_init_with_filter(data->kvsb, - &it, - less_than_or_equal, - NULL_SLICE, - range_tenant_matches, - &tenant, - NULL_SLICE)); + splinterdb_iterator_init(data->kvsb, + &it, + less_than_or_equal, + NULL_SLICE, + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = NULL_SLICE)); ASSERT_NULL(it); /* Restore a successful finalizer before the fixture closes the database. */ data->default_data_cfg.super.merge_tuples_final = hash_range_finalize; @@ -5029,14 +5035,14 @@ filtered_iterator_handoff_create(void *context) { filtered_iterator_handoff *handoff = context; char pruning_key[] = "a"; - handoff->rc = - splinterdb_iterator_init_with_filter(handoff->db, - &handoff->itor, - greater_than_or_equal, - NULL_SLICE, - range_tenant_matches, - &handoff->tenant, - slice_create(1, pruning_key)); + handoff->rc = splinterdb_iterator_init(handoff->db, + &handoff->itor, + greater_than_or_equal, + NULL_SLICE, + .predicate = range_tenant_matches, + .predicate_context = &handoff->tenant, + .routing_filter_key = + slice_create(1, pruning_key)); return NULL; } From 6ace245842cd9598d1d4ad8651162176a5d951da Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Wed, 14 Oct 2026 13:21:53 -0700 Subject: [PATCH 02/10] Iterator init: wrap bounds/start in typed limit and start-position helpers Start position becomes an optional parameter; zero value is a full forward scan. Co-Authored-By: Claude Sonnet 5.5 --- benchmarks/barrier_bench.c | 3 +- benchmarks/hash_range_benchmark.c | 9 +- benchmarks/kvbench.c | 12 +- benchmarks/optimize_notification_bench.c | 3 +- docs/hash-filtered-ranges.md | 4 +- ...interdb_custom_ipv4_addr_sortcmp_example.c | 4 +- examples/splinterdb_intro_example.c | 3 +- examples/splinterdb_iterators_example.c | 4 +- include/splinterdb/splinterdb.h | 159 +++++++-- src/splinterdb.c | 25 +- tests/functional/scan_benchmark.c | 9 +- tests/unit/splinterdb_optimize_test.c | 3 +- tests/unit/splinterdb_quick_test.c | 319 ++++++++---------- tests/unit/splinterdb_stress_test.c | 8 +- 14 files changed, 325 insertions(+), 240 deletions(-) diff --git a/benchmarks/barrier_bench.c b/benchmarks/barrier_bench.c index 2ea260f44..ebe54300d 100644 --- a/benchmarks/barrier_bench.c +++ b/benchmarks/barrier_bench.c @@ -241,8 +241,7 @@ main(int argc, char **argv) splinterdb *db; checked(splinterdb_open(&cfg, &db), "open"); splinterdb_iterator *it; - checked(splinterdb_iterator_init(db, &it, greater_than_or_equal, NULL_SLICE), - "iterator init"); + checked(splinterdb_iterator_init(db, &it), "iterator init"); for (uint64_t id = 0; id < batch * count; id++) { assert(splinterdb_iterator_valid(it)); slice k, v; diff --git a/benchmarks/hash_range_benchmark.c b/benchmarks/hash_range_benchmark.c index 3fcccac5d..209991287 100644 --- a/benchmarks/hash_range_benchmark.c +++ b/benchmarks/hash_range_benchmark.c @@ -108,8 +108,8 @@ run_queries(splinterdb *db, check(splinterdb_iterator_init( db, &it, - greater_than_or_equal, - slice_create(sizeof(key), key), + .start_position = + splinterdb_start_at_or_after(slice_create(sizeof(key), key)), .predicate = tenant_matches, .predicate_context = filter, .routing_filter_key = strcmp(mode, "pruned") == 0 @@ -117,7 +117,10 @@ run_queries(splinterdb *db, : NULL_SLICE)); #else check(splinterdb_iterator_init( - db, &it, greater_than_or_equal, slice_create(sizeof(key), key))); + db, + &it, + .start_position = + splinterdb_start_at_or_after(slice_create(sizeof(key), key)))); #endif int count = 0; while (splinterdb_iterator_valid(it)) { diff --git a/benchmarks/kvbench.c b/benchmarks/kvbench.c index 9b91f4ef9..d314f641b 100644 --- a/benchmarks/kvbench.c +++ b/benchmarks/kvbench.c @@ -758,8 +758,11 @@ warm_worker(void *arg) make_key(2 * lo, key); wait_start(); splinterdb_iterator *it; - int rc = splinterdb_iterator_init( - kvs, &it, greater_than_or_equal, slice_create(KEY_SIZE, key)); + int rc = + splinterdb_iterator_init(kvs, + &it, + .start_position = splinterdb_start_at_or_after( + slice_create(KEY_SIZE, key))); if (rc) { w->errors++; fprintf(stderr, "ERROR: warm iterator_init rc=%d\n", rc); @@ -906,7 +909,10 @@ range_worker(void *arg) uint64_t t0 = now_ns(), t1 = 0, tl = 0; splinterdb_iterator *it; int rc = splinterdb_iterator_init( - kvs, &it, greater_than_or_equal, slice_create(KEY_SIZE, key)); + kvs, + &it, + .start_position = + splinterdb_start_at_or_after(slice_create(KEY_SIZE, key))); if (rc) { w->errors++; if (w->errors <= 3) { diff --git a/benchmarks/optimize_notification_bench.c b/benchmarks/optimize_notification_bench.c index ff39c3764..4946248a8 100644 --- a/benchmarks/optimize_notification_bench.c +++ b/benchmarks/optimize_notification_bench.c @@ -129,8 +129,7 @@ main(int argc, char **argv) checked(splinterdb_close(&db, false), "close"); checked(splinterdb_open(&cfg, &db), "reopen"); splinterdb_iterator *it; - checked(splinterdb_iterator_init(db, &it, greater_than_or_equal, NULL_SLICE), - "iterator init"); + checked(splinterdb_iterator_init(db, &it), "iterator init"); for (uint64_t id = 0; id < keys; id++) { assert(splinterdb_iterator_valid(it)); slice k, v; diff --git a/docs/hash-filtered-ranges.md b/docs/hash-filtered-ranges.md index 8fab8d30e..f491233ee 100644 --- a/docs/hash-filtered-ranges.md +++ b/docs/hash-filtered-ranges.md @@ -9,8 +9,8 @@ update leaves the previously tested implementation unchanged; see the `splinterdb_iterator_init()` accepts optional designated-initializer parameters (see `splinterdb_iterator_init_options`). The filter parameters are `.predicate` (a `splinterdb_key_predicate`), `.predicate_context` (a `void *`), -and `.routing_filter_key` (a `slice`); they combine freely with the `.min_key`, -`.min_key_comparison`, `.max_key` and `.max_key_comparison` bounds parameters. +and `.routing_filter_key` (a `slice`); they combine freely with the `.lower_limit`, +`.upper_limit` and `.start_position` parameters. There is no new `data_config` callback. | Predicate | Routing-filter key | Behavior | diff --git a/examples/splinterdb_custom_ipv4_addr_sortcmp_example.c b/examples/splinterdb_custom_ipv4_addr_sortcmp_example.c index 8855fc0ce..d54a9407a 100644 --- a/examples/splinterdb_custom_ipv4_addr_sortcmp_example.c +++ b/examples/splinterdb_custom_ipv4_addr_sortcmp_example.c @@ -347,7 +347,9 @@ do_iterate_from(splinterdb *spl_handle, const char *from_key) slice start_key = (from_key ? slice_create(strlen(from_key), from_key) : NULL_SLICE); int rc = splinterdb_iterator_init( - spl_handle, &it, greater_than_or_equal, start_key); + spl_handle, + &it, + .start_position = splinterdb_start_at_or_after(start_key)); int i = 0; diff --git a/examples/splinterdb_intro_example.c b/examples/splinterdb_intro_example.c index 7c47575e1..0a7701e96 100644 --- a/examples/splinterdb_intro_example.c +++ b/examples/splinterdb_intro_example.c @@ -115,8 +115,7 @@ main() " returning keys in lexicographic sort order:\n"); splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init( - spl_handle, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(spl_handle, &it); int i = 0; for (; splinterdb_iterator_valid(it); splinterdb_iterator_next(it)) { diff --git a/examples/splinterdb_iterators_example.c b/examples/splinterdb_iterators_example.c index 8adb2c257..f89b54c4f 100644 --- a/examples/splinterdb_iterators_example.c +++ b/examples/splinterdb_iterators_example.c @@ -143,7 +143,9 @@ do_iterate_from(splinterdb *spl_handle, const char *from_key) slice start_key = (from_key ? slice_create(strlen(from_key), from_key) : NULL_SLICE); int rc = splinterdb_iterator_init( - spl_handle, &it, greater_than_or_equal, start_key); + spl_handle, + &it, + .start_position = splinterdb_start_at_or_after(start_key)); int i = 0; diff --git a/include/splinterdb/splinterdb.h b/include/splinterdb/splinterdb.h index 5dee2306f..a2c27c3f8 100644 --- a/include/splinterdb/splinterdb.h +++ b/include/splinterdb/splinterdb.h @@ -532,8 +532,7 @@ Known issue: a live iterator may block inserts and deletes from the same thread. Sample application code: splinterdb_iterator* it; - int rc = splinterdb_iterator_init(kvs, &it, greater_than_or_equal, - NULL_SLICE); + int rc = splinterdb_iterator_init(kvs, &it); if (rc != 0) { ... handle error ... } slice key, value; @@ -572,26 +571,119 @@ typedef enum comparison { // the iterator. typedef bool (*splinterdb_key_predicate)(void *context, slice key); +// A limit on the range an iterator returns. Build these with the helpers +// below (splinterdb_greater_than, splinterdb_less_than_or_equal, ...) rather +// than by hand. The zero value means "no limit". A limit whose key is +// NULL_SLICE also means "no limit". +typedef struct splinterdb_lower_limit { + bool present; + comparison cmp; // greater_than or greater_than_or_equal + slice key; +} splinterdb_lower_limit; + +typedef struct splinterdb_upper_limit { + bool present; + comparison cmp; // less_than or less_than_or_equal + slice key; +} splinterdb_upper_limit; + +// Where an iterator starts, and therefore which way it initially travels. The +// zero value means "at or after the smallest key", i.e. a forward scan of the +// whole range. +typedef struct splinterdb_start_position { + bool present; + comparison cmp; + slice key; +} splinterdb_start_position; + +// Lower limits: only return keys > key, or >= key. +static inline splinterdb_lower_limit +splinterdb_greater_than(slice key) +{ + return (splinterdb_lower_limit){true, greater_than, key}; +} +static inline splinterdb_lower_limit +splinterdb_greater_than_or_equal(slice key) +{ + return (splinterdb_lower_limit){true, greater_than_or_equal, key}; +} +// Generic form, for when the comparison is only known at run time. Passing a +// comparison other than greater_than/greater_than_or_equal makes +// splinterdb_iterator_init fail with EINVAL. +static inline splinterdb_lower_limit +splinterdb_make_lower_limit(comparison cmp, slice key) +{ + return (splinterdb_lower_limit){true, cmp, key}; +} + +// Upper limits: only return keys < key, or <= key. +static inline splinterdb_upper_limit +splinterdb_less_than(slice key) +{ + return (splinterdb_upper_limit){true, less_than, key}; +} +static inline splinterdb_upper_limit +splinterdb_less_than_or_equal(slice key) +{ + return (splinterdb_upper_limit){true, less_than_or_equal, key}; +} +// Generic form. Passing a comparison other than less_than/less_than_or_equal +// makes splinterdb_iterator_init fail with EINVAL. +static inline splinterdb_upper_limit +splinterdb_make_upper_limit(comparison cmp, slice key) +{ + return (splinterdb_upper_limit){true, cmp, key}; +} + +// Start positions. The iterator starts at the closest key satisfying the +// relation, and the relation also sets the initial direction (the first two +// scan forward, the last two backward). If key is NULL_SLICE, the iterator +// starts at the first key in the limits for the forward forms, and the last +// key in the limits for the backward forms. +static inline splinterdb_start_position +splinterdb_start_at_or_after(slice key) +{ + return (splinterdb_start_position){true, greater_than_or_equal, key}; +} +static inline splinterdb_start_position +splinterdb_start_after(slice key) +{ + return (splinterdb_start_position){true, greater_than, key}; +} +static inline splinterdb_start_position +splinterdb_start_at_or_before(slice key) +{ + return (splinterdb_start_position){true, less_than_or_equal, key}; +} +static inline splinterdb_start_position +splinterdb_start_before(slice key) +{ + return (splinterdb_start_position){true, less_than, key}; +} +// Generic form, for when the comparison is only known at run time. +static inline splinterdb_start_position +splinterdb_make_start_position(comparison cmp, slice key) +{ + return (splinterdb_start_position){true, cmp, key}; +} + // Optional parameters for splinterdb_iterator_init. A zero-initialized options // struct (apart from struct_size) requests an ordinary unbounded, unfiltered -// scan. Don't build one by hand; use the splinterdb_iterator_init macro below, -// which sets struct_size and lets you name just the fields you need. +// forward scan. Don't build one by hand; use the splinterdb_iterator_init +// macro below, which sets struct_size and lets you name just the fields you +// need. typedef struct splinterdb_iterator_init_options { // Set by the splinterdb_iterator_init macro. Lets future versions add // fields without breaking callers built against this one. size_t struct_size; - // Lower bound. If min_key is NULL_SLICE (the default) there is no lower - // bound and min_key_comparison is ignored. Otherwise min_key_comparison - // must be set to greater_than or greater_than_or_equal. - slice min_key; - comparison min_key_comparison; + // Range limits. Default: unbounded. + splinterdb_lower_limit lower_limit; + splinterdb_upper_limit upper_limit; - // Upper bound. If max_key is NULL_SLICE (the default) there is no upper - // bound and max_key_comparison is ignored. Otherwise max_key_comparison - // must be set to less_than or less_than_or_equal. - slice max_key; - comparison max_key_comparison; + // Where to start, and in which direction. Default: at or after the smallest + // key in the limits. + splinterdb_start_position start_position; // If non-NULL, the iterator returns only keys accepted by predicate. // A predicate without a routing_filter_key scans all branches in the range. @@ -609,11 +701,7 @@ typedef struct splinterdb_iterator_init_options { slice routing_filter_key; } splinterdb_iterator_init_options; -// Initialize a new iterator, starting at the given key. -// -// If start_key is NULL_SLICE, the iterator will start at: -// - greater_than_or_equal / greater_than: the first key in bounds -// - less_than_or_equal / less_than: the last key in bounds +// Initialize a new iterator. // // Filtering runs before message merging; it does not change snapshot // isolation. Bounds, starting position, direction and errors follow the @@ -621,37 +709,40 @@ typedef struct splinterdb_iterator_init_options { // not mutate the DB. int splinterdb_iterator_init_full( - splinterdb *kvs, // IN - splinterdb_iterator **iter, // OUT - comparison start_type, // IN - slice start_key, // IN - const splinterdb_iterator_init_options *options // IN + splinterdb *kvs, // IN + splinterdb_iterator **iter, // OUT + const splinterdb_iterator_init_options *options // IN ); // Convenience wrapper for splinterdb_iterator_init_full. After the required // arguments, optionally list any of the fields of // splinterdb_iterator_init_options as designated initializers: // -// splinterdb_iterator_init(kvs, &it, greater_than_or_equal, NULL_SLICE); +// // Forward scan of everything +// splinterdb_iterator_init(kvs, &it); +// +// // Forward scan of [lo, hi), starting at the first key >= start +// splinterdb_iterator_init(kvs, &it, +// .lower_limit = +// splinterdb_greater_than_or_equal(lo), .upper_limit +// = splinterdb_less_than(hi), .start_position = +// splinterdb_start_at_or_after(start)); // -// splinterdb_iterator_init(kvs, &it, greater_than_or_equal, start, -// .min_key = lo, -// .min_key_comparison = greater_than_or_equal, -// .max_key = hi, -// .max_key_comparison = less_than); +// // Backward scan from the end +// splinterdb_iterator_init(kvs, &it, +// .start_position = +// splinterdb_start_at_or_before(NULL_SLICE)); // -// splinterdb_iterator_init(kvs, &it, greater_than_or_equal, NULL_SLICE, +// splinterdb_iterator_init(kvs, &it, // .predicate = my_pred, // .predicate_context = &ctx); // // Note that commas inside field values must be parenthesized as usual for // macro arguments. -#define splinterdb_iterator_init(kvs, iter, start_type, start_key, ...) \ +#define splinterdb_iterator_init(kvs, iter, ...) \ splinterdb_iterator_init_full( \ (kvs), \ (iter), \ - (start_type), \ - (start_key), \ &(const splinterdb_iterator_init_options){ \ .struct_size = sizeof(splinterdb_iterator_init_options), \ __VA_ARGS__}) diff --git a/src/splinterdb.c b/src/splinterdb.c index 0921577a2..535ef6fb8 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -1157,11 +1157,9 @@ splinterdb_iterator_max_comparison_is_valid(comparison cmp) int splinterdb_iterator_init_full( - splinterdb *kvs, // IN - splinterdb_iterator **iter, // OUT - comparison start_key_comparison, // IN - slice user_start_key, // IN - const splinterdb_iterator_init_options *options // IN + splinterdb *kvs, // IN + splinterdb_iterator **iter, // OUT + const splinterdb_iterator_init_options *options // IN ) { if (options == NULL @@ -1169,18 +1167,23 @@ splinterdb_iterator_init_full( { return platform_status_to_int(STATUS_BAD_PARAM); } - slice user_min_key = options->min_key; - slice user_max_key = options->max_key; + slice user_min_key = options->lower_limit.key; + slice user_max_key = options->upper_limit.key; + slice user_start_key = options->start_position.key; splinterdb_key_predicate predicate = options->predicate; void *predicate_context = options->predicate_context; slice routing_filter_key = options->routing_filter_key; comparison min_key_comparison = greater_than_or_equal; comparison max_key_comparison = less_than; - if (!slice_is_null(user_min_key)) { - min_key_comparison = options->min_key_comparison; + comparison start_key_comparison = greater_than_or_equal; + if (options->lower_limit.present) { + min_key_comparison = options->lower_limit.cmp; } - if (!slice_is_null(user_max_key)) { - max_key_comparison = options->max_key_comparison; + if (options->upper_limit.present) { + max_key_comparison = options->upper_limit.cmp; + } + if (options->start_position.present) { + start_key_comparison = options->start_position.cmp; } if ((predicate == NULL && !slice_is_null(routing_filter_key)) diff --git a/tests/functional/scan_benchmark.c b/tests/functional/scan_benchmark.c index cceaa69d3..57f3a35e0 100644 --- a/tests/functional/scan_benchmark.c +++ b/tests/functional/scan_benchmark.c @@ -545,7 +545,10 @@ scan_benchmark_run_scan(const splinterdb_config *cfg, splinterdb_iterator *iter = NULL; timestamp start_time = platform_get_timestamp(); rc = splinterdb_iterator_init( - kvs, &iter, scan_benchmark_start_comparison(backwards_scan), NULL_SLICE); + kvs, + &iter, + .start_position = splinterdb_make_start_position( + scan_benchmark_start_comparison(backwards_scan), NULL_SLICE)); if (rc != 0) { splinterdb_close(&kvs, FALSE); return rc; @@ -686,8 +689,8 @@ scan_benchmark_run_repeated_scans(const splinterdb_config *cfg, rc = splinterdb_iterator_init( kvs, &iter, - scan_benchmark_start_comparison(backwards_scan), - start_key); + .start_position = splinterdb_make_start_position( + scan_benchmark_start_comparison(backwards_scan), start_key)); if (rc != 0) { splinterdb_close(&kvs, FALSE); return rc; diff --git a/tests/unit/splinterdb_optimize_test.c b/tests/unit/splinterdb_optimize_test.c index a1ec64dcb..fdae0413f 100644 --- a/tests/unit/splinterdb_optimize_test.c +++ b/tests/unit/splinterdb_optimize_test.c @@ -490,8 +490,7 @@ static void verify_full_scan(splinterdb *kvsb, uint32 num_keys) { splinterdb_iterator *itor = NULL; - int rc = - splinterdb_iterator_init(kvsb, &itor, greater_than_or_equal, NULL_SLICE); + int rc = splinterdb_iterator_init(kvsb, &itor); ASSERT_EQUAL(0, rc); for (uint32 key_no = 0; key_no < num_keys; key_no++) { diff --git a/tests/unit/splinterdb_quick_test.c b/tests/unit/splinterdb_quick_test.c index b31cde276..7ce23f348 100644 --- a/tests/unit/splinterdb_quick_test.c +++ b/tests/unit/splinterdb_quick_test.c @@ -368,9 +368,7 @@ CTEST2(splinterdb_quick, test_async_durable_barrier_notifications) splinterdb_notification_deinit(&closing); ASSERT_EQUAL(0, splinterdb_open(&data->cfg, &data->kvsb)); splinterdb_iterator *it; - ASSERT_EQUAL(0, - splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE)); + ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &it)); for (uint64 i = 0; i < 512; i++) { ASSERT_TRUE(splinterdb_iterator_valid(it)); ASSERT_EQUAL(0, check_current_tuple(it, i)); @@ -1112,8 +1110,7 @@ CTEST2(splinterdb_quick, test_iterator_survives_memtable_recycle) ASSERT_EQUAL(0, rc); splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); ASSERT_TRUE(splinterdb_iterator_valid(it)); @@ -2262,8 +2259,7 @@ CTEST2(splinterdb_quick, test_basic_iterator) splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); for (; splinterdb_iterator_valid(it); splinterdb_iterator_next(it)) { @@ -2286,8 +2282,7 @@ CTEST2(splinterdb_quick, test_iterator_delete_current) ASSERT_EQUAL(0, rc); splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); int num_deleted = 0; @@ -2331,8 +2326,7 @@ CTEST2(splinterdb_quick, test_iterator_delete_current) CTEST2(splinterdb_quick, test_empty_iterator) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + int rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -2408,14 +2402,11 @@ CTEST2(splinterdb_quick, test_bounded_iterator) ARRAY_SIZE(singleton)); ASSERT_EQUAL(0, rc); - rc = splinterdb_iterator_init(data->kvsb, - NULL, - greater_than_or_equal, - NULL_SLICE, - .min_key_comparison = less_than, - .min_key = min, - .max_key_comparison = less_than_or_equal, - .max_key = max); + rc = splinterdb_iterator_init( + data->kvsb, + NULL, + .lower_limit = splinterdb_make_lower_limit(less_than, min), + .upper_limit = splinterdb_less_than_or_equal(max)); ASSERT_EQUAL(EINVAL, rc); } @@ -2628,8 +2619,10 @@ CTEST2(splinterdb_quick, test_splinterdb_iterator_with_startkey) // Initialize the i'th key snprintf(key, sizeof(key), key_fmt, ictr); slice start_key = slice_create(strlen(key), key); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); bool32 is_valid = splinterdb_iterator_valid(it); @@ -2663,8 +2656,10 @@ CTEST2(splinterdb_quick, test_splinterdb_iterator_with_non_existent_startkey) char *keystring = "unknownKey"; slice start_key = slice_create(strlen(keystring), keystring); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); // Iterator should be invalid, as lookup key is non-existent. bool32 is_valid = splinterdb_iterator_valid(it); @@ -2677,8 +2672,10 @@ CTEST2(splinterdb_quick, test_splinterdb_iterator_with_non_existent_startkey) // before 'key...', which is what key's format is.) keystring = "UnknownKey"; start_key = slice_create(strlen(keystring), keystring); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); int ictr = 0; @@ -2731,8 +2728,10 @@ CTEST2(splinterdb_quick, splinterdb_iterator *it = NULL; slice start_key = slice_create(strlen(key), key); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); bool32 is_valid = splinterdb_iterator_valid(it); @@ -2751,8 +2750,10 @@ CTEST2(splinterdb_quick, snprintf(key, sizeof(key), key_fmt, kctr); start_key = slice_create(strlen(key), key); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); is_valid = splinterdb_iterator_valid(it); @@ -2770,8 +2771,10 @@ CTEST2(splinterdb_quick, kctr = 5; snprintf(key, sizeof(key), key_fmt, kctr); start_key = slice_create(strlen(key), key); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); is_valid = splinterdb_iterator_valid(it); @@ -2789,8 +2792,10 @@ CTEST2(splinterdb_quick, kctr = minkey + 3 * num_inserts; snprintf(key, sizeof(key), key_fmt, kctr); start_key = slice_create(strlen(key), key); - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init(data->kvsb, + &it, + .start_position = + splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); is_valid = splinterdb_iterator_valid(it); @@ -2848,8 +2853,7 @@ CTEST2(splinterdb_quick, test_iterator_reversal_on_key_in_two_branches) insert_string_kv(data->kvsb, "c", "c1"); splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + int rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); check_current_string_kv(it, "a", "a0"); @@ -2890,8 +2894,7 @@ CTEST2(splinterdb_quick, test_iterator_reversal_with_three_branches) insert_string_kv(data->kvsb, "d", "d2"); splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + int rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); check_current_string_kv(it, "a", "a2"); @@ -3685,8 +3688,7 @@ CTEST2(splinterdb_quick, test_iterator_custom_comparator) ASSERT_EQUAL(0, rc); splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); int i = 0; @@ -3728,8 +3730,7 @@ CTEST2(splinterdb_quick, test_iterator_init_bug) // Iterator init should find nothing when no keys were inserted, yet. splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); bool32 iter_valid = splinterdb_iterator_valid(it); @@ -3743,8 +3744,7 @@ CTEST2(splinterdb_quick, test_iterator_init_bug) ASSERT_EQUAL(0, rc); it = NULL; - rc = splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE); + rc = splinterdb_iterator_init(data->kvsb, &it); ASSERT_EQUAL(0, rc); iter_valid = splinterdb_iterator_valid(it); @@ -4070,14 +4070,12 @@ check_bounded_iterator_sequence(splinterdb *kvsb, uint64 expected_count) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init(kvsb, - &it, - start_type, - NULL_SLICE, - .min_key_comparison = min_key_comparison, - .min_key = min_key, - .max_key_comparison = max_key_comparison, - .max_key = max_key); + int rc = splinterdb_iterator_init( + kvsb, + &it, + .lower_limit = splinterdb_make_lower_limit(min_key_comparison, min_key), + .upper_limit = splinterdb_make_upper_limit(max_key_comparison, max_key), + .start_position = splinterdb_make_start_position(start_type, NULL_SLICE)); ASSERT_EQUAL(0, rc); for (uint64 i = 0; i < expected_count; i++) { @@ -4109,14 +4107,13 @@ check_bounded_iterator_current_from_start(splinterdb *kvsb, int expected_i) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init(kvsb, - &it, - start_key_comparison, - start_key, - .min_key_comparison = min_key_comparison, - .min_key = min_key, - .max_key_comparison = max_key_comparison, - .max_key = max_key); + int rc = splinterdb_iterator_init( + kvsb, + &it, + .lower_limit = splinterdb_make_lower_limit(min_key_comparison, min_key), + .upper_limit = splinterdb_make_upper_limit(max_key_comparison, max_key), + .start_position = + splinterdb_make_start_position(start_key_comparison, start_key)); ASSERT_EQUAL(0, rc); ASSERT_TRUE(splinterdb_iterator_valid(it)); @@ -4141,14 +4138,13 @@ check_bounded_iterator_boundary_from_start(splinterdb *kvsb, int expected_i) { splinterdb_iterator *it = NULL; - int rc = splinterdb_iterator_init(kvsb, - &it, - start_key_comparison, - start_key, - .min_key_comparison = min_key_comparison, - .min_key = min_key, - .max_key_comparison = max_key_comparison, - .max_key = max_key); + int rc = splinterdb_iterator_init( + kvsb, + &it, + .lower_limit = splinterdb_make_lower_limit(min_key_comparison, min_key), + .upper_limit = splinterdb_make_upper_limit(max_key_comparison, max_key), + .start_position = + splinterdb_make_start_position(start_key_comparison, start_key)); ASSERT_EQUAL(0, rc); ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -4186,7 +4182,8 @@ test_two_step_iterator(splinterdb *kvsb, { int rc; splinterdb_iterator *it = NULL; - rc = splinterdb_iterator_init(kvsb, &it, greater_than_or_equal, start_key); + rc = splinterdb_iterator_init( + kvsb, &it, .start_position = splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); for (int i = start_i; i < num_keys; i++) { @@ -4337,8 +4334,9 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) splinterdb_iterator_init( data->kvsb, &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, + .start_position = splinterdb_make_start_position( + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE), .predicate = range_tenant_matches, .predicate_context = &tenant, .routing_filter_key = slice_create(1, "a"))); @@ -4379,17 +4377,14 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) splinterdb_iterator *it = NULL; ASSERT_EQUAL( 0, - splinterdb_iterator_init(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - .min_key_comparison = greater_than, - .min_key = slice_create(2, "0a"), - .max_key_comparison = less_than, - .max_key = slice_create(2, "3a"), - .predicate = range_tenant_matches, - .predicate_context = &tenant, - .routing_filter_key = slice_create(1, "a"))); + splinterdb_iterator_init( + data->kvsb, + &it, + .lower_limit = splinterdb_greater_than(slice_create(2, "0a")), + .upper_limit = splinterdb_less_than(slice_create(2, "3a")), + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = slice_create(1, "a"))); check_current_string_kv(it, "2a", "3"); splinterdb_iterator_next(it); ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -4402,8 +4397,6 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) 0, splinterdb_iterator_init(data->kvsb, &it, - greater_than_or_equal, - NULL_SLICE, .predicate = range_tenant_matches, .predicate_context = &tenant, .routing_filter_key = slice_create(1, "z"))); @@ -4413,8 +4406,6 @@ CTEST2(splinterdb_quick, test_filtered_range_merges_only_matching_keys) ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &it, - greater_than_or_equal, - NULL_SLICE, .predicate = NULL, .predicate_context = NULL, .routing_filter_key = NULL_SLICE)); @@ -4485,8 +4476,9 @@ CTEST2(splinterdb_quick, test_filtered_range_crosses_empty_trunk_leaves) splinterdb_iterator_init( data->kvsb, &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, + .start_position = splinterdb_make_start_position( + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE), .predicate = range_sparse_matches, .predicate_context = &no_match, .routing_filter_key = slice_create(sizeof(hash_key), &hash_key))); @@ -4518,8 +4510,9 @@ CTEST2(splinterdb_quick, test_filtered_range_crosses_empty_trunk_leaves) splinterdb_iterator_init( data->kvsb, &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, + .start_position = splinterdb_make_start_position( + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE), .predicate = range_sparse_matches, .predicate_context = &no_match, .routing_filter_key = slice_create(sizeof(hash_key), &hash_key))); @@ -4645,23 +4638,22 @@ range_mixed_seed_check_scan(splinterdb *db, splinterdb_iterator *it = NULL; range_mixed_seed_seen = 0; range_mixed_seed_track = TRUE; - ASSERT_EQUAL(0, - splinterdb_iterator_init( - db, - &it, - forwards ? greater_than_or_equal : less_than_or_equal, - NULL_SLICE, - .min_key_comparison = greater_than, - .min_key = bounded ? range_mixed_seed_key(lower, low, 'a') - : NULL_SLICE, - .max_key_comparison = less_than, - .max_key = bounded ? range_mixed_seed_key(upper, high, 'a') - : NULL_SLICE, - .predicate = range_tenant_matches, - .predicate_context = &tenant, - .routing_filter_key = - pruned ? slice_create(sizeof(routing_key), routing_key) - : NULL_SLICE)); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init( + db, + &it, + .lower_limit = splinterdb_greater_than( + bounded ? range_mixed_seed_key(lower, low, 'a') : NULL_SLICE), + .upper_limit = splinterdb_less_than( + bounded ? range_mixed_seed_key(upper, high, 'a') : NULL_SLICE), + .start_position = splinterdb_make_start_position( + forwards ? greater_than_or_equal : less_than_or_equal, NULL_SLICE), + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = + pruned ? slice_create(sizeof(routing_key), routing_key) + : NULL_SLICE)); const uint32 initial_seed_mask = range_mixed_seed_seen; /* The iterator must retain its own routing key for later saved seeds. * a and b collide under seed 11, but differ under seed 22. */ @@ -4839,8 +4831,6 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &it, - greater_than_or_equal, - NULL_SLICE, .predicate = range_digit_matches, .predicate_context = &digits, .routing_filter_key = NULL_SLICE)); @@ -4862,14 +4852,15 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) * predicate must visit every branch, including the current memtable. */ for (int forwards = 0; forwards < 2; forwards++) { ASSERT_EQUAL(0, - splinterdb_iterator_init(data->kvsb, - &it, - forwards ? greater_than_or_equal - : less_than_or_equal, - NULL_SLICE, - .predicate = range_digit_matches, - .predicate_context = &digits, - .routing_filter_key = NULL_SLICE)); + splinterdb_iterator_init( + data->kvsb, + &it, + .start_position = splinterdb_make_start_position( + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE), + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); for (int i = 0; i < 5; i++) { char expected[] = {(forwards ? 1 + 2 * i : 9 - 2 * i) + '0', 0}; check_current_string_kv(it, expected, "v"); @@ -4883,19 +4874,19 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); - ASSERT_EQUAL(0, - splinterdb_iterator_init(data->kvsb, - &it, - forwards ? greater_than_or_equal - : less_than_or_equal, - NULL_SLICE, - .min_key_comparison = greater_than, - .min_key = slice_create(1, "1"), - .max_key_comparison = less_than, - .max_key = slice_create(1, "9"), - .predicate = range_digit_matches, - .predicate_context = &digits, - .routing_filter_key = NULL_SLICE)); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init( + data->kvsb, + &it, + .lower_limit = splinterdb_greater_than(slice_create(1, "1")), + .upper_limit = splinterdb_less_than(slice_create(1, "9")), + .start_position = splinterdb_make_start_position( + forwards ? greater_than_or_equal : less_than_or_equal, + NULL_SLICE), + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); for (int i = 0; i < 3; i++) { char expected[] = {(forwards ? 3 + 2 * i : 7 - 2 * i) + '0', 0}; check_current_string_kv(it, expected, "v"); @@ -4919,14 +4910,16 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); - ASSERT_EQUAL(0, - splinterdb_iterator_init(data->kvsb, - &it, - forwards ? greater_than : less_than, - slice_create(1, "5"), - .predicate = range_digit_matches, - .predicate_context = &digits, - .routing_filter_key = NULL_SLICE)); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init( + data->kvsb, + &it, + .start_position = splinterdb_make_start_position( + forwards ? greater_than : less_than, slice_create(1, "5")), + .predicate = range_digit_matches, + .predicate_context = &digits, + .routing_filter_key = NULL_SLICE)); check_current_string_kv(it, forwards ? "7" : "3", "v"); ASSERT_EQUAL(0, splinterdb_iterator_status(it)); splinterdb_iterator_deinit(it); @@ -4938,8 +4931,6 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &it, - greater_than_or_equal, - NULL_SLICE, .predicate = range_digit_matches, .predicate_context = &digits, .routing_filter_key = NULL_SLICE)); @@ -4952,25 +4943,19 @@ CTEST2(splinterdb_quick, test_filtered_range_predicate_only_and_bounds) EINVAL, splinterdb_iterator_init(data->kvsb, &it, - greater_than_or_equal, - NULL_SLICE, .predicate = NULL, .predicate_context = NULL, .routing_filter_key = slice_create(1, "1"))); ASSERT_NULL(it); - ASSERT_EQUAL( - EINVAL, - splinterdb_iterator_init(data->kvsb, - &it, - greater_than_or_equal, - NULL_SLICE, - .min_key_comparison = greater_than_or_equal, - .min_key = NULL_SLICE, - .max_key_comparison = less_than_or_equal, - .max_key = NULL_SLICE, - .predicate = NULL, - .predicate_context = NULL, - .routing_filter_key = slice_create(1, "1"))); + ASSERT_EQUAL(EINVAL, + splinterdb_iterator_init( + data->kvsb, + &it, + .lower_limit = splinterdb_greater_than_or_equal(NULL_SLICE), + .upper_limit = splinterdb_less_than_or_equal(NULL_SLICE), + .predicate = NULL, + .predicate_context = NULL, + .routing_filter_key = slice_create(1, "1"))); ASSERT_NULL(it); } @@ -4998,8 +4983,6 @@ CTEST2(splinterdb_quick, test_filtered_range_propagates_matching_merge_errors) ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &it, - greater_than_or_equal, - NULL_SLICE, .predicate = range_tenant_matches, .predicate_context = &tenant, .routing_filter_key = NULL_SLICE)); @@ -5011,13 +4994,13 @@ CTEST2(splinterdb_quick, test_filtered_range_propagates_matching_merge_errors) it = NULL; ASSERT_EQUAL(ENOMEM, - splinterdb_iterator_init(data->kvsb, - &it, - less_than_or_equal, - NULL_SLICE, - .predicate = range_tenant_matches, - .predicate_context = &tenant, - .routing_filter_key = NULL_SLICE)); + splinterdb_iterator_init( + data->kvsb, + &it, + .start_position = splinterdb_start_at_or_before(NULL_SLICE), + .predicate = range_tenant_matches, + .predicate_context = &tenant, + .routing_filter_key = NULL_SLICE)); ASSERT_NULL(it); /* Restore a successful finalizer before the fixture closes the database. */ data->default_data_cfg.super.merge_tuples_final = hash_range_finalize; @@ -5037,8 +5020,6 @@ filtered_iterator_handoff_create(void *context) char pruning_key[] = "a"; handoff->rc = splinterdb_iterator_init(handoff->db, &handoff->itor, - greater_than_or_equal, - NULL_SLICE, .predicate = range_tenant_matches, .predicate_context = &handoff->tenant, .routing_filter_key = @@ -5123,8 +5104,7 @@ static void * iterator_handoff_create_and_exit(void *arg) { iterator_handoff *handoff = arg; - handoff->rc = splinterdb_iterator_init( - handoff->db, &handoff->itor, greater_than_or_equal, NULL_SLICE); + handoff->rc = splinterdb_iterator_init(handoff->db, &handoff->itor); handoff->tid = platform_get_tid(); return NULL; } @@ -5169,10 +5149,7 @@ CTEST2(splinterdb_quick, test_iterator_thread_handoff) ASSERT_EQUAL(0, splinterdb_open(&data->cfg, &data->kvsb)); } iterator_handoff handoff = {.db = data->kvsb}; - ASSERT_EQUAL( - 0, - splinterdb_iterator_init( - data->kvsb, &handoff.itor, greater_than_or_equal, NULL_SLICE)); + ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &handoff.itor)); pthread_t receiver; ASSERT_EQUAL( 0, diff --git a/tests/unit/splinterdb_stress_test.c b/tests/unit/splinterdb_stress_test.c index 68e26e520..f45a4ad29 100644 --- a/tests/unit/splinterdb_stress_test.c +++ b/tests/unit/splinterdb_stress_test.c @@ -165,7 +165,9 @@ CTEST2(splinterdb_stress, test_iterator_over_many_kvs) slice start_key = slice_create(sizeof(key_str), key_str); ASSERT_EQUAL(0, splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, start_key)); + data->kvsb, + &it, + .start_position = splinterdb_start_at_or_after(start_key))); // assert that the iterator is in the state we expect ASSERT_FALSE(splinterdb_iterator_valid(it)); @@ -319,8 +321,8 @@ naive_range_delete(splinterdb *kvsb, slice start_key, uint32 count) splinterdb_iterator *it; - int rc = - splinterdb_iterator_init(kvsb, &it, greater_than_or_equal, start_key); + int rc = splinterdb_iterator_init( + kvsb, &it, .start_position = splinterdb_start_at_or_after(start_key)); ASSERT_EQUAL(0, rc); slice key, value; From bcff189aa062e77ccd9a4db6a0f060c6d92182b9 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Wed, 14 Oct 2026 14:19:05 -0700 Subject: [PATCH 03/10] Options-struct macros for splinterdb_lookup_result_init and splinterdb_optimize Co-Authored-By: Claude Sonnet 5.5 --- benchmarks/insert_benchmark.c | 2 +- benchmarks/kvbench.c | 4 +- benchmarks/optimize_notification_bench.c | 12 ++- examples/splinterdb_intro_example.c | 3 +- examples/splinterdb_optimize_example.c | 10 +-- examples/splinterdb_wide_values_example.c | 2 +- include/splinterdb/splinterdb.h | 69 +++++++++++--- src/splinterdb.c | 36 +++++--- tests/functional/scan_benchmark.c | 3 +- tests/unit/splinterdb_optimize_test.c | 78 ++++++++-------- tests/unit/splinterdb_quick_test.c | 104 ++++++++-------------- tests/unit/trunk_split_test.c | 3 +- 12 files changed, 174 insertions(+), 152 deletions(-) diff --git a/benchmarks/insert_benchmark.c b/benchmarks/insert_benchmark.c index a51b0a71c..3fec74304 100644 --- a/benchmarks/insert_benchmark.c +++ b/benchmarks/insert_benchmark.c @@ -245,7 +245,7 @@ run_worker(void *arg) gettid()); if (w->query) { splinterdb_lookup_result_init( - b->db, &result, SPLINTERDB_LOOKUP_VALUE, sizeof(value), value); + b->db, &result, .buffer = slice_create(sizeof(value), value)); } while (!atomic_load_explicit(&b->start, memory_order_acquire)) { struct timespec delay = {.tv_nsec = 1000000}; diff --git a/benchmarks/kvbench.c b/benchmarks/kvbench.c index d314f641b..7214f2689 100644 --- a/benchmarks/kvbench.c +++ b/benchmarks/kvbench.c @@ -818,7 +818,7 @@ point_worker(void *arg) char buf[256]; splinterdb_lookup_result res; splinterdb_lookup_result_init( - kvs, &res, SPLINTERDB_LOOKUP_VALUE, sizeof(buf), buf); + kvs, &res, .buffer = slice_create(sizeof(buf), buf)); uint64_t T = (uint64_t)nworkers; uint64_t lo = opt.n * (uint64_t)w->idx / T, hi = opt.n * (uint64_t)(w->idx + 1) / T; @@ -1043,7 +1043,7 @@ verify_overwrite_sample(const phase_t *ph, int threads) char key[KEY_SIZE], buffer[VALUE_SIZE]; splinterdb_lookup_result result; splinterdb_lookup_result_init( - kvs, &result, SPLINTERDB_LOOKUP_VALUE, sizeof(buffer), buffer); + kvs, &result, .buffer = slice_create(sizeof(buffer), buffer)); for (int i = 0; i < threads; i++) { worker_t *original = &workers[i]; diff --git a/benchmarks/optimize_notification_bench.c b/benchmarks/optimize_notification_bench.c index 4946248a8..17ce4c4fb 100644 --- a/benchmarks/optimize_notification_bench.c +++ b/benchmarks/optimize_notification_bench.c @@ -54,11 +54,19 @@ optimize(splinterdb *db, bool empty_range) { slice bound = empty_range ? slice_create(1, "k") : NULL_SLICE; #ifdef HAVE_SENTINEL_NOTIFICATION - checked(splinterdb_optimize(db, bound, bound, true, NULL), "optimize"); + checked( + splinterdb_optimize( + db, .min_key = bound, .max_key = bound, .full_leaf_compactions = true), + "optimize"); #else splinterdb_notification note; splinterdb_notification_init_blocking(¬e); - checked(splinterdb_optimize(db, bound, bound, true, ¬e), "optimize"); + checked(splinterdb_optimize(db, + .min_key = bound, + .max_key = bound, + .full_leaf_compactions = true, + .notification = ¬e), + "optimize"); splinterdb_notification_deinit(¬e); #endif } diff --git a/examples/splinterdb_intro_example.c b/examples/splinterdb_intro_example.c index 0a7701e96..c59a159cb 100644 --- a/examples/splinterdb_intro_example.c +++ b/examples/splinterdb_intro_example.c @@ -69,8 +69,7 @@ main() // Retrieve a key-value pair. splinterdb_lookup_result result; - splinterdb_lookup_result_init( - spl_handle, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(spl_handle, &result); fruit = "Orange"; key = slice_create((size_t)strlen(fruit), fruit); diff --git a/examples/splinterdb_optimize_example.c b/examples/splinterdb_optimize_example.c index 8dd5a8215..d1ce67139 100644 --- a/examples/splinterdb_optimize_example.c +++ b/examples/splinterdb_optimize_example.c @@ -96,11 +96,11 @@ main(int argc, char **argv) opts.min_key == NULL ? "-infinity" : opts.min_key, opts.max_key == NULL ? "+infinity" : opts.max_key); - rc = splinterdb_optimize(spl, - min_key, - max_key, - opts.full_leaf_compactions, - SPLINTERDB_NOTIFICATION_BLOCKING); + rc = + splinterdb_optimize(spl, + .min_key = min_key, + .max_key = max_key, + .full_leaf_compactions = opts.full_leaf_compactions); if (rc == 0) { printf("Optimize completed successfully.\n"); diff --git a/examples/splinterdb_wide_values_example.c b/examples/splinterdb_wide_values_example.c index 660e2119c..f3170cab6 100644 --- a/examples/splinterdb_wide_values_example.c +++ b/examples/splinterdb_wide_values_example.c @@ -76,7 +76,7 @@ main() splinterdb_lookup_result result; splinterdb_lookup_result_init( - spl_handle, &result, SPLINTERDB_LOOKUP_VALUE, sizeof(outbuf), outbuf); + spl_handle, &result, .buffer = slice_create(sizeof(outbuf), outbuf)); printf("Retrieve values of different lengths using output buffer of" " fixed size=%lu bytes:\n", diff --git a/include/splinterdb/splinterdb.h b/include/splinterdb/splinterdb.h index a2c27c3f8..ec2efa725 100644 --- a/include/splinterdb/splinterdb.h +++ b/include/splinterdb/splinterdb.h @@ -348,17 +348,35 @@ typedef struct splinterdb_lookup_result { char opaque[SPLINTERDB_LOOKUP_BUFSIZE]; } __attribute__((__aligned__(8))) splinterdb_lookup_result; +// Optional parameters for splinterdb_lookup_result_init. The zero value +// requests a value lookup with library-managed memory. +typedef struct splinterdb_lookup_result_init_options { + size_t struct_size; // set by the macro + splinterdb_lookup_flags flags; + slice buffer; +} splinterdb_lookup_result_init_options; + // Initialize a lookup result object. // -// flags may be SPLINTERDB_LOOKUP_VALUE or SPLINTERDB_LOOKUP_MIGHT_EXIST. +// splinterdb_lookup_result_init(kvs, &result); +// splinterdb_lookup_result_init(kvs, &result, +// .flags = SPLINTERDB_LOOKUP_MIGHT_EXIST, +// .buffer = slice_create(sizeof(buf), buf)); +// +// Use the macro rather than building the options by hand. +// +// options.flags may be SPLINTERDB_LOOKUP_VALUE or +// SPLINTERDB_LOOKUP_MIGHT_EXIST. // // When SPLINTERDB_LOOKUP_MIGHT_EXIST is set, splinterdb_lookup_found() reports // true if the key might exist, and false if the key definitely does not exist. // splinterdb_lookup_result_value() will return EINVAL for such results. // -// If buffer is NULL, then the library will allocate and manage memory. +// If options.buffer is NULL_SLICE (the default), then the library will allocate +// and manage memory. // -// If the caller provides a buffer, that will be used, unless a lookup +// If the caller provides a buffer (as a slice describing its address and +// length), that will be used, unless a lookup // requires a larger buffer, at which time the library will allocate. // Regardless, the library will never free a buffer supplied by the application. // @@ -372,13 +390,20 @@ typedef struct splinterdb_lookup_result { // While the current version of SplinterDB does not rely on Rule 2, future // versions may store pointers to Splinter's own memory in the lookup_result. void -splinterdb_lookup_result_init(const splinterdb *kvs, // IN - splinterdb_lookup_result *result, // IN/OUT - splinterdb_lookup_flags flags, // IN - uint64 buffer_len, // IN - char *buffer // IN +splinterdb_lookup_result_init_full( + const splinterdb *kvs, // IN + splinterdb_lookup_result *result, // IN/OUT + const splinterdb_lookup_result_init_options *options // IN ); +#define splinterdb_lookup_result_init(kvs, result, ...) \ + splinterdb_lookup_result_init_full( \ + (kvs), \ + (result), \ + &(const splinterdb_lookup_result_init_options){ \ + .struct_size = sizeof(splinterdb_lookup_result_init_options), \ + __VA_ARGS__}) + // Release any resources used by result // // This will never free a buffer passed to splinterdb_lookup_result_init @@ -457,12 +482,30 @@ splinterdb_update(splinterdb *kvsb, // SPLINTERDB_NOTIFICATION_FIRE_AND_FORGET returns without waiting and discards // completion status. A real notification receives completion asynchronously. // For either nonblocking mode, a nonzero return means submission failed. +typedef struct splinterdb_optimize_options { + size_t struct_size; // set by the macro + // Range to optimize. NULL_SLICE (the default) is unbounded on that side. + slice min_key; + slice max_key; + // Default: false. + _Bool full_leaf_compactions; + // Default: NULL, i.e. SPLINTERDB_NOTIFICATION_BLOCKING. + splinterdb_notification *notification; +} splinterdb_optimize_options; + int -splinterdb_optimize(splinterdb *kvs, - slice min_key, - slice max_key, - _Bool full_leaf_compactions, - splinterdb_notification *notification); +splinterdb_optimize_full(splinterdb *kvs, + const splinterdb_optimize_options *options); + +// Example: +// splinterdb_optimize(kvs); // whole DB, blocking +// splinterdb_optimize(kvs, .min_key = lo, .max_key = hi, +// .notification = &n); +#define splinterdb_optimize(kvs, ...) \ + splinterdb_optimize_full( \ + (kvs), \ + &(const splinterdb_optimize_options){ \ + .struct_size = sizeof(splinterdb_optimize_options), __VA_ARGS__}) ///////////////////////////////// // Durability diff --git a/src/splinterdb.c b/src/splinterdb.c index 535ef6fb8..540a3fbb6 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -866,16 +866,21 @@ splinterdb_close(splinterdb **kvs_in, bool32 force) // IN } void -splinterdb_lookup_result_init(const splinterdb *kvs, // IN - splinterdb_lookup_result *result, // IN/OUT - splinterdb_lookup_flags flags, // IN - uint64 buffer_len, // IN - char *buffer // IN +splinterdb_lookup_result_init_full( + const splinterdb *kvs, // IN + splinterdb_lookup_result *result, // IN/OUT + const splinterdb_lookup_result_init_options *options // IN ) { + platform_assert(options != NULL + && options->struct_size + >= sizeof(splinterdb_lookup_result_init_options)); lookup_result *_result = lookup_result_from_splinterdb(result); - lookup_result_init( - _result, kvs->spl.cfg.data_cfg, flags, buffer_len, buffer); + lookup_result_init(_result, + kvs->spl.cfg.data_cfg, + options->flags, + slice_length(options->buffer), + (char *)slice_data(options->buffer)); lookup_result_set_owner(_result, kvs); } @@ -1056,12 +1061,19 @@ splinterdb_update(splinterdb *kvsb, } int -splinterdb_optimize(splinterdb *kvs, - slice user_min_key, - slice user_max_key, - _Bool full_leaf_compactions, - splinterdb_notification *notification) +splinterdb_optimize_full(splinterdb *kvs, + const splinterdb_optimize_options *options) { + if (options == NULL + || options->struct_size < sizeof(splinterdb_optimize_options)) + { + return platform_status_to_int(STATUS_BAD_PARAM); + } + slice user_min_key = options->min_key; + slice user_max_key = options->max_key; + _Bool full_leaf_compactions = options->full_leaf_compactions; + splinterdb_notification *notification = options->notification; + int rc = splinterdb_ensure_thread_registered(); if (rc != 0) { return rc; diff --git a/tests/functional/scan_benchmark.c b/tests/functional/scan_benchmark.c index 57f3a35e0..fc77a01f5 100644 --- a/tests/functional/scan_benchmark.c +++ b/tests/functional/scan_benchmark.c @@ -513,8 +513,7 @@ scan_benchmark_run_optimize(const splinterdb_config *cfg) platform_default_log( "scan_benchmark: running blocking full-leaf optimize\n"); timestamp start_time = platform_get_timestamp(); - rc = splinterdb_optimize( - kvs, NULL_SLICE, NULL_SLICE, TRUE, SPLINTERDB_NOTIFICATION_BLOCKING); + rc = splinterdb_optimize(kvs, .full_leaf_compactions = TRUE); uint64 elapsed_ns = platform_timestamp_elapsed(start_time); platform_default_log("optimize complete: rc=%d, %.3fs elapsed\n", diff --git a/tests/unit/splinterdb_optimize_test.c b/tests/unit/splinterdb_optimize_test.c index fdae0413f..e2447ae9e 100644 --- a/tests/unit/splinterdb_optimize_test.c +++ b/tests/unit/splinterdb_optimize_test.c @@ -95,11 +95,7 @@ CTEST2(splinterdb_optimize, test_blocking_full_range) load_key_batches(data->kvsb, num_keys, 40); - int rc = splinterdb_optimize(data->kvsb, - NULL_SLICE, - NULL_SLICE, - TRUE, - SPLINTERDB_NOTIFICATION_BLOCKING); + int rc = splinterdb_optimize(data->kvsb, .full_leaf_compactions = TRUE); ASSERT_EQUAL(0, rc); verify_point_lookups(data->kvsb, num_keys); @@ -112,8 +108,7 @@ CTEST2(splinterdb_optimize, test_blocking_without_full_leaf_compactions) load_key_batches(data->kvsb, num_keys, 40); - int rc = - splinterdb_optimize(data->kvsb, NULL_SLICE, NULL_SLICE, FALSE, NULL); + int rc = splinterdb_optimize(data->kvsb); ASSERT_EQUAL(0, rc); verify_point_lookups(data->kvsb, num_keys); @@ -133,11 +128,7 @@ CTEST2(splinterdb_optimize, test_blocking_with_no_background_threads) load_key_batches(data->kvsb, num_keys, 40); - rc = splinterdb_optimize(data->kvsb, - NULL_SLICE, - NULL_SLICE, - TRUE, - SPLINTERDB_NOTIFICATION_BLOCKING); + rc = splinterdb_optimize(data->kvsb, .full_leaf_compactions = TRUE); ASSERT_EQUAL(0, rc); verify_point_lookups(data->kvsb, num_keys); @@ -173,8 +164,7 @@ static void verify_resized_value(splinterdb *kvsb, slice key, uint8 length) { splinterdb_lookup_result result; - splinterdb_lookup_result_init( - kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(kvsb, &result); ASSERT_EQUAL(0, splinterdb_lookup(kvsb, key, &result)); ASSERT_TRUE(splinterdb_lookup_found(&result)); slice value; @@ -205,12 +195,8 @@ CTEST2(splinterdb_optimize, test_merge_can_grow_and_shrink_values) data->kvsb, keys[i], slice_create(1, &lengths[round][i]), NULL)); } force_flush_current_memtable(data->kvsb); - ASSERT_EQUAL(0, - splinterdb_optimize(data->kvsb, - NULL_SLICE, - NULL_SLICE, - TRUE, - SPLINTERDB_NOTIFICATION_BLOCKING)); + ASSERT_EQUAL( + 0, splinterdb_optimize(data->kvsb, .full_leaf_compactions = TRUE)); for (uint64 i = 0; i < ARRAY_SIZE(keys); i++) { verify_resized_value(data->kvsb, keys[i], lengths[round][i]); } @@ -254,11 +240,12 @@ CTEST2(splinterdb_optimize, test_polling_subrange) splinterdb_notification notification; splinterdb_notification_init_polling(¬ification, &user_data); - int rc = splinterdb_optimize(data->kvsb, - slice_create(strlen(min_key), min_key), - slice_create(strlen(max_key), max_key), - TRUE, - ¬ification); + int rc = + splinterdb_optimize(data->kvsb, + .min_key = slice_create(strlen(min_key), min_key), + .max_key = slice_create(strlen(max_key), max_key), + .full_leaf_compactions = TRUE, + .notification = ¬ification); ASSERT_EQUAL(0, rc); ASSERT_TRUE(&user_data == splinterdb_notification_user_data(¬ification)); @@ -288,8 +275,12 @@ CTEST2(splinterdb_optimize, test_empty_and_rejected_ranges_all_modes) for (uint64 i = 0; i < ARRAY_SIZE(notifications); i++) { splinterdb_notification_init_polling(&polling, NULL); /* Empty ranges complete inline, before the blocking caller waits. */ - ASSERT_EQUAL( - 0, splinterdb_optimize(data->kvsb, low, low, TRUE, notifications[i])); + ASSERT_EQUAL(0, + splinterdb_optimize(data->kvsb, + .min_key = low, + .max_key = low, + .full_leaf_compactions = TRUE, + .notification = notifications[i])); if (notifications[i] == &polling) { int status = -1; ASSERT_TRUE(splinterdb_notification_poll(&polling, &status)); @@ -300,9 +291,12 @@ CTEST2(splinterdb_optimize, test_empty_and_rejected_ranges_all_modes) splinterdb_notification_init_polling(&polling, NULL); /* Rejected submission schedules no completion. The private blocking * notification must release both references on this path. */ - ASSERT_EQUAL( - EINVAL, - splinterdb_optimize(data->kvsb, high, low, TRUE, notifications[i])); + ASSERT_EQUAL(EINVAL, + splinterdb_optimize(data->kvsb, + .min_key = high, + .max_key = low, + .full_leaf_compactions = TRUE, + .notification = notifications[i])); ASSERT_FALSE(splinterdb_notification_poll(&polling, NULL)); splinterdb_notification_deinit(&polling); } @@ -319,11 +313,10 @@ CTEST2(splinterdb_optimize, test_fire_and_forget_without_background_threads) load_key_batches(data->kvsb, num_keys, 40); ASSERT_EQUAL(0, - splinterdb_optimize(data->kvsb, - NULL_SLICE, - NULL_SLICE, - TRUE, - SPLINTERDB_NOTIFICATION_FIRE_AND_FORGET)); + splinterdb_optimize( + data->kvsb, + .full_leaf_compactions = TRUE, + .notification = SPLINTERDB_NOTIFICATION_FIRE_AND_FORGET)); /* There is no notification recipient. Close must drain accepted work. */ ASSERT_EQUAL(0, splinterdb_close(&data->kvsb, FALSE)); ASSERT_EQUAL(0, splinterdb_open(&data->cfg, &data->kvsb)); @@ -462,8 +455,7 @@ static void verify_point_lookups(splinterdb *kvsb, uint32 num_keys) { splinterdb_lookup_result result; - splinterdb_lookup_result_init( - kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(kvsb, &result); for (uint32 key_no = 0; key_no < num_keys; key_no++) { char key[OPTIMIZE_TEST_KEY_SIZE]; @@ -533,12 +525,12 @@ optimize_thread(void *arg) return NULL; } - args->rc = - splinterdb_optimize(args->kvsb, - slice_create(strlen(args->min_key), args->min_key), - slice_create(strlen(args->max_key), args->max_key), - TRUE, - args->notification); + args->rc = splinterdb_optimize( + args->kvsb, + .min_key = slice_create(strlen(args->min_key), args->min_key), + .max_key = slice_create(strlen(args->max_key), args->max_key), + .full_leaf_compactions = TRUE, + .notification = args->notification); return NULL; } diff --git a/tests/unit/splinterdb_quick_test.c b/tests/unit/splinterdb_quick_test.c index 7ce23f348..f3b7c4560 100644 --- a/tests/unit/splinterdb_quick_test.c +++ b/tests/unit/splinterdb_quick_test.c @@ -405,8 +405,7 @@ CTEST2(splinterdb_quick, test_logged_large_value_replaces_old_value) ASSERT_EQUAL(0, rc); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, user_key, &result)); assert_lookup_result_matches_slice(&result, slice_create(value_len, value_data)); @@ -420,8 +419,7 @@ CTEST2(splinterdb_quick, test_logged_large_value_replaces_old_value) ASSERT_EQUAL(0, splinterdb_close(&data->kvsb, FALSE)); data->cfg.blob_max_descriptor_size = 128; ASSERT_EQUAL(0, splinterdb_open(&data->cfg, &data->kvsb)); - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, flat_key, &result)); assert_lookup_result_matches_slice(&result, flat_value); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, user_key, &result)); @@ -449,8 +447,7 @@ CTEST2(splinterdb_quick, test_basic_flow) slice user_key = slice_create(key_len, key_data); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); int rc = splinterdb_lookup(data->kvsb, user_key, &result); ASSERT_EQUAL(0, rc); @@ -512,8 +509,7 @@ auto_registered_result_thread(void *arg) args->tid_before = platform_get_tid(); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - args->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(args->kvsb, &result); splinterdb_lookup_result_deinit(&result); args->tid_after = platform_get_tid(); if (args->tid_after >= MAX_THREADS) { @@ -522,8 +518,7 @@ auto_registered_result_thread(void *arg) } slice key = slice_create(3, "key"); - splinterdb_lookup_result_init( - args->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(args->kvsb, &result); args->rc = splinterdb_lookup(args->kvsb, key, &result); if (args->rc == 0) { // Value materialization also needs to register before taking the lock. @@ -576,8 +571,7 @@ CTEST2(splinterdb_quick, test_stats_reset_clears_cache_io) ASSERT_TRUE(SUCCESS(cache_evict(cc, TRUE))); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, key, &result)); assert_lookup_result_matches_slice(&result, value); splinterdb_lookup_result_deinit(&result); @@ -662,8 +656,7 @@ CTEST2(splinterdb_quick, test_final_merge_error_propagation) ASSERT_EQUAL(0, splinterdb_update(data->kvsb, key, value, NULL)); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); cfg->fail = TRUE; ASSERT_EQUAL(ENOMEM, splinterdb_lookup(data->kvsb, key, &result)); @@ -727,8 +720,7 @@ CTEST2(splinterdb_quick, test_fetch_and_update_catches_up_after_rotation) ASSERT_EQUAL(0, splinterdb_update(data->kvsb, key, skipped, NULL)); splinterdb_lookup_result old_result; - splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &old_result); fetch_and_update_thread_args args = { .kvsb = data->kvsb, .key = key, @@ -794,8 +786,7 @@ CTEST2(splinterdb_quick, test_fetch_and_update_rejects_incorporated_boundary) ASSERT_EQUAL(0, splinterdb_update(data->kvsb, key, skipped, NULL)); splinterdb_lookup_result old_result; - splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &old_result); fetch_and_update_thread_args args = { .kvsb = data->kvsb, .key = key, @@ -864,8 +855,7 @@ CTEST2(splinterdb_quick, test_range_init_error_cleanup_preserves_branches) // branches, so point lookups and a new range scan must remain usable. cfg->fail = FALSE; splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, old_key, &result)); assert_lookup_result_matches_slice(&result, value); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, new_key, &result)); @@ -945,8 +935,7 @@ CTEST2(splinterdb_quick, test_apis_for_large_key) ASSERT_EQUAL(0, rc); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); // **** Lookup of max-size key should return correct value rc = splinterdb_lookup(data->kvsb, large_key, &result); @@ -989,8 +978,7 @@ CTEST2(splinterdb_quick, test_key_size_gt_max_inline_key_size) ASSERT_EQUAL(EINVAL, rc); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_delete(data->kvsb, too_large_key, NULL); ASSERT_EQUAL(EINVAL, rc); @@ -1016,8 +1004,7 @@ CTEST2(splinterdb_quick, test_value_size_gt_max_value_size) ASSERT_EQUAL(0, rc); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_lookup( data->kvsb, slice_create(sizeof("foo"), "foo"), &result); @@ -1036,8 +1023,7 @@ CTEST2(splinterdb_quick, test_value_size_gt_max_value_size) rc = splinterdb_open(&data->cfg, &data->kvsb); ASSERT_EQUAL(0, rc); - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_lookup( data->kvsb, slice_create(sizeof("foo"), "foo"), &result); ASSERT_EQUAL(0, rc); @@ -1066,8 +1052,7 @@ CTEST2(splinterdb_quick, test_large_lookup_survives_memtable_recycle) ASSERT_EQUAL(0, rc); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_lookup(data->kvsb, user_key, &result); ASSERT_EQUAL(0, rc); @@ -1189,11 +1174,10 @@ CTEST2(splinterdb_quick, test_variable_length_values) slice value; // allocate a result that has full access to the buffer - splinterdb_lookup_result_init(data->kvsb, - &result, - SPLINTERDB_LOOKUP_VALUE, - sizeof(big_buffer), - big_buffer); + splinterdb_lookup_result_init( + data->kvsb, + &result, + .buffer = slice_create(sizeof(big_buffer), big_buffer)); // look up a 0-length value rc = splinterdb_lookup(data->kvsb, key_empty, &result); @@ -1242,9 +1226,8 @@ CTEST2(splinterdb_quick, test_variable_length_values) uint64 short_buffer_len = TEST_MAX_VALUE_SIZE - 1; splinterdb_lookup_result_init(data->kvsb, &result, - SPLINTERDB_LOOKUP_VALUE, - short_buffer_len, - big_buffer); + .buffer = + slice_create(short_buffer_len, big_buffer)); // lookup tuple with max-sized-value, passing it the short buffer rc = splinterdb_lookup(data->kvsb, key_max, &result); @@ -1265,8 +1248,7 @@ CTEST2(splinterdb_quick, test_variable_length_values) // init another result, but don't give it a buffer - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); // lookup, see we get the full result back rc = splinterdb_lookup(data->kvsb, key_max, &result); ASSERT_EQUAL(0, rc); @@ -2304,8 +2286,7 @@ CTEST2(splinterdb_quick, test_iterator_delete_current) splinterdb_iterator_deinit(it); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); for (int i = 0; i < num_inserts; i++) { char key[TEST_INSERT_KEY_LENGTH] = {0}; @@ -2982,8 +2963,7 @@ CTEST2(splinterdb_quick, test_close_and_reopen) slice value; splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_lookup(data->kvsb, user_key, &result); ASSERT_EQUAL(0, rc); @@ -3028,8 +3008,7 @@ CTEST2(splinterdb_quick, test_logged_close_and_reopen) ASSERT_EQUAL(0, rc); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_lookup(data->kvsb, user_key, &result); ASSERT_EQUAL(0, rc); ASSERT_TRUE(splinterdb_lookup_found(&result)); @@ -3095,8 +3074,7 @@ CTEST2(splinterdb_quick, test_custom_data_config) // confirm its there splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); rc = splinterdb_lookup(data->kvsb, user_key, &result); ASSERT_EQUAL(0, rc); ASSERT_TRUE(splinterdb_lookup_found(&result)); @@ -3150,7 +3128,7 @@ CTEST2(splinterdb_quick, test_existence_only_memtable_lookup) splinterdb_lookup_result result; splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_MIGHT_EXIST, 0, NULL); + data->kvsb, &result, .flags = SPLINTERDB_LOOKUP_MIGHT_EXIST); int rc = splinterdb_lookup(data->kvsb, user_key, &result); ASSERT_EQUAL(0, rc); @@ -3202,7 +3180,7 @@ CTEST2(splinterdb_quick, test_existence_only_trunk_lookup_skips_branches) splinterdb_lookup_result existence_result; splinterdb_lookup_result_init( - data->kvsb, &existence_result, SPLINTERDB_LOOKUP_MIGHT_EXIST, 0, NULL); + data->kvsb, &existence_result, .flags = SPLINTERDB_LOOKUP_MIGHT_EXIST); rc = splinterdb_lookup(data->kvsb, user_key, &existence_result); ASSERT_EQUAL(0, rc); @@ -3217,8 +3195,7 @@ CTEST2(splinterdb_quick, test_existence_only_trunk_lookup_skips_branches) core_reset_stats(core); splinterdb_lookup_result normal_result; - splinterdb_lookup_result_init( - data->kvsb, &normal_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &normal_result); rc = splinterdb_lookup(data->kvsb, user_key, &normal_result); ASSERT_EQUAL(0, rc); @@ -3234,8 +3211,7 @@ CTEST2(splinterdb_quick, test_write_api_returns_old_value) slice value1 = slice_create(strlen("old-value-1"), "old-value-1"); splinterdb_lookup_result old_result; - splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &old_result); int rc = splinterdb_insert(data->kvsb, user_key, value0, &old_result); ASSERT_EQUAL(0, rc); @@ -3264,7 +3240,7 @@ CTEST2(splinterdb_quick, test_write_api_old_result_existence_only) splinterdb_lookup_result old_result; splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_MIGHT_EXIST, 0, NULL); + data->kvsb, &old_result, .flags = SPLINTERDB_LOOKUP_MIGHT_EXIST); int rc = splinterdb_insert(data->kvsb, user_key, value0, &old_result); ASSERT_EQUAL(0, rc); @@ -3301,8 +3277,7 @@ CTEST2(splinterdb_quick, test_write_api_old_result_custom_merge_semantics) slice msg_slice = slice_create(sizeof(msg), &msg); splinterdb_lookup_result old_result; - splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &old_result); rc = splinterdb_insert(data->kvsb, user_key, msg_slice, &old_result); ASSERT_EQUAL(0, rc); @@ -3362,8 +3337,7 @@ CTEST2(splinterdb_quick, test_write_api_old_result_merges_memtable_and_trunk) ASSERT_EQUAL(0, rc); splinterdb_lookup_result old_result; - splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &old_result); merge_accumulator expected; merge_accumulator_init(&expected, PROCESS_PRIVATE_HEAP_ID); @@ -3398,8 +3372,7 @@ CTEST2(splinterdb_quick, test_write_api_old_result_respects_trunk_delete_shadow) ASSERT_EQUAL(0, rc); splinterdb_lookup_result old_result; - splinterdb_lookup_result_init( - data->kvsb, &old_result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &old_result); rc = splinterdb_insert(data->kvsb, user_key, value1, &old_result); ASSERT_EQUAL(0, rc); @@ -3895,8 +3868,7 @@ auto_registered_lookup_thread(void *arg) slice user_key = slice_create(sizeof(key_data), key_data); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - args->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(args->kvsb, &result); args->tid_before = platform_get_tid(); args->rc = splinterdb_lookup(args->kvsb, user_key, &result); @@ -3933,8 +3905,7 @@ stats_reset_worker(void *arg) slice value = slice_create(value_len, value_data); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - params->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(params->kvsb, &result); __atomic_fetch_add(params->started, 1, __ATOMIC_RELEASE); while (!__atomic_load_n(params->stop, __ATOMIC_ACQUIRE)) { @@ -5288,8 +5259,7 @@ CTEST2(splinterdb_quick, test_blob_tree_crash_recovery) for (uint64 reopen = 0; reopen < 2; reopen++) { ASSERT_EQUAL(0, splinterdb_open(&data->cfg, &data->kvsb)); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, prefix, &result)); assert_lookup_result_matches_slice(&result, small); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, large, &result)); diff --git a/tests/unit/trunk_split_test.c b/tests/unit/trunk_split_test.c index 6e400e0cd..c84678e51 100644 --- a/tests/unit/trunk_split_test.c +++ b/tests/unit/trunk_split_test.c @@ -144,8 +144,7 @@ CTEST2(trunk_split, duplicate_keys_produce_valid_leaves) for (uint64 pass = 0; pass < 2; pass++) { for (char k = '!'; k <= '"'; k++) { splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->db, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->db, &result); ASSERT_EQUAL( 0, splinterdb_lookup(data->db, slice_create(1, &k), &result)); ASSERT_TRUE(splinterdb_lookup_found(&result)); From 5bf1a6d4f149d4af5fa5fd59b0566e2e7a74f559 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Wed, 14 Oct 2026 14:28:15 -0700 Subject: [PATCH 04/10] Internal iterator constructors: options structs for btree_iterator_init and core_range_iterator_init Adds key_lower_limit/key_upper_limit/key_start_position (internal-key analogues of the public types). Merges core_range_iterator_init_with_filter into core_range_iterator_init. Drops redundant range params from core_memtable_iterator_init. Co-Authored-By: Claude Sonnet 5.5 --- src/btree.c | 71 +++---- src/btree.h | 32 +-- src/core.c | 90 +++----- src/core.h | 36 ++-- src/iterator.h | 117 +++++++++++ src/splinterdb.c | 53 ++--- src/trunk.c | 28 ++- tests/functional/btree_test.c | 288 ++++++++++---------------- tests/functional/test_functionality.c | 25 +-- tests/unit/btree_stress_test.c | 189 ++++++----------- tests/unit/btree_test.c | 25 +-- tests/unit/splinter_test.c | 30 +-- tests/unit/trunk_split_test.c | 23 +- 13 files changed, 449 insertions(+), 558 deletions(-) diff --git a/src/btree.c b/src/btree.c index 572b4c2db..d89cd1512 100644 --- a/src/btree.c +++ b/src/btree.c @@ -5237,42 +5237,38 @@ btree_iterator_init_common(cache *cc, } platform_status -btree_iterator_init(cache *cc, - const btree_config *cfg, - btree_iterator *itor, - uint64 root_addr, - uint16 format_version, - page_type page_type, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_type, - key start_key, - bool32 copy_nodes, - uint32 height, - uint32 prefetch_lookahead) -{ +btree_iterator_init(cache *cc, + const btree_config *cfg, + btree_iterator *itor, + uint64 root_addr, + uint16 format_version, + page_type page_type, + const btree_iterator_init_options *options) +{ + resolved_key_range range = resolve_key_range( + options->lower_limit, options->upper_limit, options->start_position); + key start_key = range.start_key; + platform_status rc = btree_iterator_init_common(cc, cfg, itor, root_addr, format_version, page_type, - min_key_comparison, - min_key, - max_key_comparison, - max_key, + range.min_cmp, + range.min_key, + range.max_cmp, + range.max_key, start_key, - copy_nodes, - height, - prefetch_lookahead, + options->copy_nodes, + options->height, + options->prefetch_lookahead, &start_key); if (!SUCCESS(rc)) { return rc; } - rc = find_btree_node_and_get_idx_bounds(itor, start_key, start_type); + rc = find_btree_node_and_get_idx_bounds(itor, start_key, range.start_cmp); if (!SUCCESS(rc)) { btree_iterator_deinit(itor); return rc; @@ -5976,21 +5972,18 @@ btree_count_in_range_by_iterator(cache *cc, { btree_iterator btree_itor; iterator *itor = &btree_itor.super; - platform_status rc = btree_iterator_init(cc, - cfg, - &btree_itor, - root_addr, - format_version, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - min_key, - less_than, - max_key, - greater_than_or_equal, - min_key, - FALSE, - 0, - 1); + platform_status rc = + btree_iterator_init(cc, + cfg, + &btree_itor, + root_addr, + format_version, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(min_key), + .upper_limit = key_less_than(max_key), + .start_position = key_start_at_or_after(min_key), + .prefetch_lookahead = 1}); if (!SUCCESS(rc)) { return rc; } diff --git a/src/btree.h b/src/btree.h index 40db334be..608f6f286 100644 --- a/src/btree.h +++ b/src/btree.h @@ -432,22 +432,24 @@ btree_lookup_async(btree_lookup_async_state *state); * prefetches at most the next extent, and values >= 2 enable deep extent * prefetch. */ +typedef struct btree_iterator_init_options { + // Range and start position. Default: the whole tree, scanned forward. + key_lower_limit lower_limit; + key_upper_limit upper_limit; + key_start_position start_position; + bool32 copy_nodes; // default: FALSE + uint32 height; // default: 0 + uint32 prefetch_lookahead; // default: 0 (disabled) +} btree_iterator_init_options; + platform_status -btree_iterator_init(cache *cc, - const btree_config *cfg, - btree_iterator *itor, - uint64 root_addr, - uint16 format_version, - page_type page_type, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_type, - key start_key, - bool32 copy_nodes, - uint32 height, - uint32 prefetch_lookahead); +btree_iterator_init(cache *cc, + const btree_config *cfg, + btree_iterator *itor, + uint64 root_addr, + uint16 format_version, + page_type page_type, + const btree_iterator_init_options *options); // clang-format off DEFINE_ASYNC_STATE(btree_iterator_async_state, 6, diff --git a/src/core.c b/src/core.c index 8bf5fa940..ae23a16de 100644 --- a/src/core.c +++ b/src/core.c @@ -1235,13 +1235,7 @@ core_memtable_dec_ref(core_handle *spl, uint64 root_addr) static platform_status core_memtable_iterator_init(core_handle *spl, btree_iterator *itor, - uint64 root_addr, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_key_comparison, - key start_key) + uint64 root_addr) { return btree_iterator_init(spl->cc, spl->cfg.btree_cfg, @@ -1249,15 +1243,7 @@ core_memtable_iterator_init(core_handle *spl, root_addr, 0, PAGE_TYPE_MEMTABLE, - min_key_comparison, - min_key, - max_key_comparison, - max_key, - start_key_comparison, - start_key, - FALSE, - 0, - 0); + &(const btree_iterator_init_options){0}); } static void @@ -1463,15 +1449,8 @@ core_memtable_compact(core_handle *spl, uint64 generation, const threadid tid) btree_iterator btree_itor; iterator *itor = &btree_itor.super; - platform_status rc = core_memtable_iterator_init(spl, - &btree_itor, - memtable_root_addr, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY); + platform_status rc = + core_memtable_iterator_init(spl, &btree_itor, memtable_root_addr); if (!SUCCESS(rc)) { goto failed; } @@ -2115,41 +2094,22 @@ core_range_iterator_init_span(core_handle *spl, key start_key); platform_status -core_range_iterator_init(core_handle *spl, - core_range_iterator *range_itor, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_key_comparison, - key start_key) +core_range_iterator_init(core_handle *spl, + core_range_iterator *range_itor, + const core_range_iterator_init_options *options) { - return core_range_iterator_init_with_filter(spl, - range_itor, - min_key_comparison, - min_key, - max_key_comparison, - max_key, - start_key_comparison, - start_key, - NULL, - NULL, - NULL_KEY); -} + resolved_key_range range = resolve_key_range( + options->lower_limit, options->upper_limit, options->start_position); + comparison min_key_comparison = range.min_cmp; + key min_key = range.min_key; + comparison max_key_comparison = range.max_cmp; + key max_key = range.max_key; + comparison start_key_comparison = range.start_cmp; + key start_key = range.start_key; + splinterdb_key_predicate predicate = options->predicate; + void *predicate_context = options->predicate_context; + key routing_filter_key = options->routing_filter_key; -platform_status -core_range_iterator_init_with_filter(core_handle *spl, - core_range_iterator *range_itor, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_key_comparison, - key start_key, - splinterdb_key_predicate predicate, - void *predicate_context, - key routing_filter_key) -{ range_itor->filter = NULL; platform_status rc = STATUS_OK; if (predicate == NULL && !key_is_null(routing_filter_key)) { @@ -3210,14 +3170,12 @@ core_apply_to_range(core_handle *spl, return STATUS_NO_MEMORY; } - platform_status rc = core_range_iterator_init(spl, - range_itor, - greater_than_or_equal, - start_key, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - start_key); + platform_status rc = core_range_iterator_init( + spl, + range_itor, + &(const core_range_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(start_key), + .start_position = key_start_at_or_after(start_key)}); if (!SUCCESS(rc)) { platform_error_log("core_apply_to_range: range iterator init failed: " "%s\n", diff --git a/src/core.h b/src/core.h index 2018777db..a4f181478 100644 --- a/src/core.h +++ b/src/core.h @@ -370,28 +370,24 @@ DEFINE_ASYNC_STATE(core_lookup_async_state, 1, async_status core_lookup_async(core_lookup_async_state *state); -platform_status -core_range_iterator_init(core_handle *spl, - core_range_iterator *range_itor, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_key_comparison, - key start_key); +typedef struct core_range_iterator_init_options { + // Range and start position. Default: the whole tree, scanned forward. + key_lower_limit lower_limit; + key_upper_limit upper_limit; + key_start_position start_position; + + // Optional key filter, applied before message merging. A routing filter + // key (only valid with a predicate) additionally prunes branches; see + // splinterdb_iterator_init_options. + splinterdb_key_predicate predicate; + void *predicate_context; + key routing_filter_key; // default: NULL_KEY +} core_range_iterator_init_options; platform_status -core_range_iterator_init_with_filter(core_handle *spl, - core_range_iterator *range_itor, - comparison min_key_comparison, - key min_key, - comparison max_key_comparison, - key max_key, - comparison start_key_comparison, - key start_key, - splinterdb_key_predicate predicate, - void *predicate_context, - key routing_filter_key); +core_range_iterator_init(core_handle *spl, + core_range_iterator *range_itor, + const core_range_iterator_init_options *options); void core_range_iterator_deinit(core_range_iterator *range_itor); diff --git a/src/iterator.h b/src/iterator.h index d5ac16e44..ea32f50ec 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -64,6 +64,123 @@ comparison_invert(comparison c) } } +/* + * Range specification for iterator constructors. These mirror the public + * splinterdb_lower_limit / splinterdb_upper_limit / splinterdb_start_position + * types, but hold internal keys, so they can name infinities. Constructors + * take them in an options struct filled in with designated initializers; the + * zero value of each means "unbounded" / "start at the smallest key". A limit + * whose key is NULL_KEY is also unbounded. + */ +typedef struct key_lower_limit { + bool present; + comparison cmp; // greater_than or greater_than_or_equal + key k; +} key_lower_limit; + +typedef struct key_upper_limit { + bool present; + comparison cmp; // less_than or less_than_or_equal + key k; +} key_upper_limit; + +typedef struct key_start_position { + bool present; + comparison cmp; + key k; +} key_start_position; + +static inline key_lower_limit +key_greater_than(key k) +{ + return (key_lower_limit){TRUE, greater_than, k}; +} +static inline key_lower_limit +key_greater_than_or_equal(key k) +{ + return (key_lower_limit){TRUE, greater_than_or_equal, k}; +} +static inline key_lower_limit +make_key_lower_limit(comparison cmp, key k) +{ + return (key_lower_limit){TRUE, cmp, k}; +} + +static inline key_upper_limit +key_less_than(key k) +{ + return (key_upper_limit){TRUE, less_than, k}; +} +static inline key_upper_limit +key_less_than_or_equal(key k) +{ + return (key_upper_limit){TRUE, less_than_or_equal, k}; +} +static inline key_upper_limit +make_key_upper_limit(comparison cmp, key k) +{ + return (key_upper_limit){TRUE, cmp, k}; +} + +static inline key_start_position +key_start_at_or_after(key k) +{ + return (key_start_position){TRUE, greater_than_or_equal, k}; +} +static inline key_start_position +key_start_after(key k) +{ + return (key_start_position){TRUE, greater_than, k}; +} +static inline key_start_position +key_start_at_or_before(key k) +{ + return (key_start_position){TRUE, less_than_or_equal, k}; +} +static inline key_start_position +key_start_before(key k) +{ + return (key_start_position){TRUE, less_than, k}; +} +static inline key_start_position +make_key_start_position(comparison cmp, key k) +{ + return (key_start_position){TRUE, cmp, k}; +} + +// A range specification with every default filled in: infinite keys for +// unbounded sides, and the start key chosen by direction. +typedef struct resolved_key_range { + comparison min_cmp; + key min_key; + comparison max_cmp; + key max_key; + comparison start_cmp; + key start_key; +} resolved_key_range; + +static inline resolved_key_range +resolve_key_range(key_lower_limit lower, + key_upper_limit upper, + key_start_position start) +{ + resolved_key_range r; + bool32 lower_set = lower.present && !key_is_null(lower.k); + bool32 upper_set = upper.present && !key_is_null(upper.k); + r.min_cmp = lower_set ? lower.cmp : greater_than_or_equal; + r.min_key = lower_set ? lower.k : NEGATIVE_INFINITY_KEY; + r.max_cmp = upper_set ? upper.cmp : less_than; + r.max_key = upper_set ? upper.k : POSITIVE_INFINITY_KEY; + r.start_cmp = start.present ? start.cmp : greater_than_or_equal; + if (start.present && !key_is_null(start.k)) { + r.start_key = start.k; + } else { + r.start_key = comparison_is_forward(r.start_cmp) ? NEGATIVE_INFINITY_KEY + : POSITIVE_INFINITY_KEY; + } + return r; +} + // It is safe to call curr whenever iterator_in_range() returns true // otherwise the behavior of iterator_curr is undefined static inline void diff --git a/src/splinterdb.c b/src/splinterdb.c index 540a3fbb6..a653540a9 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -1221,39 +1221,30 @@ splinterdb_iterator_init_full( merge_accumulator_init(&it->materialized_message, kvs->spl.heap_id); core_range_iterator *range_itor = &(it->sri); - key min_key; - key max_key; - key start_key; - - min_key = slice_is_null(user_min_key) - ? NEGATIVE_INFINITY_KEY - : key_create_from_slice(TRUE, user_min_key); - max_key = slice_is_null(user_max_key) - ? POSITIVE_INFINITY_KEY - : key_create_from_slice(TRUE, user_max_key); - - if (slice_is_null(user_start_key)) { - if (start_key_comparison <= less_than_or_equal) { - start_key = POSITIVE_INFINITY_KEY; - } else { - start_key = NEGATIVE_INFINITY_KEY; - } - } else { - start_key = key_create_from_slice(TRUE, user_start_key); - } - - platform_status rc = core_range_iterator_init_with_filter( + // Internal keys for the user-supplied slices; NULL_SLICE bounds become + // NULL_KEY, which the core treats as unbounded. + key min_key = slice_is_null(user_min_key) + ? NULL_KEY + : key_create_from_slice(TRUE, user_min_key); + key max_key = slice_is_null(user_max_key) + ? NULL_KEY + : key_create_from_slice(TRUE, user_max_key); + key start_key = slice_is_null(user_start_key) + ? NULL_KEY + : key_create_from_slice(TRUE, user_start_key); + + platform_status rc = core_range_iterator_init( &kvs->spl, range_itor, - min_key_comparison, - min_key, - max_key_comparison, - max_key, - start_key_comparison, - start_key, - predicate, - predicate_context, - key_create_from_slice(TRUE, routing_filter_key)); + &(const core_range_iterator_init_options){ + .lower_limit = make_key_lower_limit(min_key_comparison, min_key), + .upper_limit = make_key_upper_limit(max_key_comparison, max_key), + .start_position = + make_key_start_position(start_key_comparison, start_key), + .predicate = predicate, + .predicate_context = predicate_context, + .routing_filter_key = + key_create_from_slice(TRUE, routing_filter_key)}); if (!SUCCESS(rc)) { merge_accumulator_deinit(&it->materialized_message); platform_free(kvs->spl.heap_id, it); diff --git a/src/trunk.c b/src/trunk.c index b517583db..193adb094 100644 --- a/src/trunk.c +++ b/src/trunk.c @@ -3186,21 +3186,19 @@ trunk_branch_merger_build_merge_itor(trunk_branch_merger *merger, "%s():%d: platform_malloc() failed", __func__, __LINE__); return STATUS_NO_MEMORY; } - rc = btree_iterator_init(cc, - merger->cfg->btree_cfg, - itor, - branch.addr, - branch.format_version, - branch.type, - greater_than_or_equal, - merger->min_key, - less_than, - merger->max_key, - greater_than_or_equal, - merger->min_key, - FALSE, - merger->height, - lookahead); + rc = btree_iterator_init( + cc, + merger->cfg->btree_cfg, + itor, + branch.addr, + branch.format_version, + branch.type, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(merger->min_key), + .upper_limit = key_less_than(merger->max_key), + .start_position = key_start_at_or_after(merger->min_key), + .height = merger->height, + .prefetch_lookahead = lookahead}); if (!SUCCESS(rc)) { platform_error_log("%s():%d: btree_iterator_init() failed: %s", __func__, diff --git a/tests/functional/btree_test.c b/tests/functional/btree_test.c index dba2d4bb6..d29a91372 100644 --- a/tests/functional/btree_test.c +++ b/tests/functional/btree_test.c @@ -645,21 +645,19 @@ test_btree_scan_once(cache *cc, { btree_iterator itor; timestamp start_time = platform_get_timestamp(); - platform_status rc = btree_iterator_init(cc, - btree_cfg, - &itor, - root_addr, - BTREE_BRANCH_FORMAT_VERSION, - type, - greater_than_or_equal, - min_key, - less_than, - max_key, - greater_than_or_equal, - min_key, - copy_nodes, - 0, - prefetch_lookahead); + platform_status rc = + btree_iterator_init(cc, + btree_cfg, + &itor, + root_addr, + BTREE_BRANCH_FORMAT_VERSION, + type, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(min_key), + .upper_limit = key_less_than(max_key), + .start_position = key_start_at_or_after(min_key), + .copy_nodes = copy_nodes, + .prefetch_lookahead = prefetch_lookahead}); *init_elapsed_ns += platform_timestamp_elapsed(start_time); if (!SUCCESS(rc)) { return rc; @@ -951,15 +949,7 @@ test_btree_scan_perf(cache *cc, root_addr, 0, PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + &(const btree_iterator_init_options){0}); if (!SUCCESS(rc)) { goto out; } @@ -1337,21 +1327,15 @@ test_btree_iterator_init_status(cache *cc, { btree_iterator itor; if (!async) { - platform_status rc = btree_iterator_init(cc, - cfg, - &itor, - root_addr, - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - start_type, - start_key, - FALSE, - 0, - 0); + platform_status rc = btree_iterator_init( + cc, + cfg, + &itor, + root_addr, + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .start_position = make_key_start_position(start_type, start_key)}); if (SUCCESS(rc)) { btree_iterator_deinit(&itor); } @@ -1436,21 +1420,14 @@ test_btree_query_validation(cache *cc, key_buffer_init(&touched_key, hid); btree_iterator itor; - platform_status rc = btree_iterator_init(cc, - cfg, - &itor, - root_addr, - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + platform_status rc = + btree_iterator_init(cc, + cfg, + &itor, + root_addr, + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){0}); if (!SUCCESS(rc) || !iterator_can_curr(&itor.super)) { rc = STATUS_TEST_FAILED; goto deinit_keys; @@ -1580,21 +1557,15 @@ test_btree_query_validation(cache *cc, cc, cfg, root_addr, PAGE_TYPE_BRANCH, key_buffer_key(&touched_key), TRUE); btree_iterator sequence_itor; - platform_status forward_rc = btree_iterator_init(cc, - cfg, - &sequence_itor, - root_addr, - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - key_buffer_key(&first_key), - FALSE, - 0, - 0); + platform_status forward_rc = btree_iterator_init( + cc, + cfg, + &sequence_itor, + root_addr, + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .start_position = key_start_at_or_after(key_buffer_key(&first_key))}); if (SUCCESS(forward_rc)) { while (SUCCESS(forward_rc) && sequence_itor.curr.addr == first_leaf_addr && iterator_can_next(&sequence_itor.super)) @@ -1741,22 +1712,17 @@ test_btree_query_validation(cache *cc, bool32 diagnostic_cycle_rejected = !btree_verify_tree(cc, cfg, root_addr, PAGE_TYPE_BRANCH); btree_iterator height_itor; - platform_status height_itor_rc = - btree_iterator_init(cc, - cfg, - &height_itor, - root_addr, - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - key_buffer_key(&touched_key), - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - key_buffer_key(&touched_key), - FALSE, - 0, - 0); + platform_status height_itor_rc = btree_iterator_init( + cc, + cfg, + &height_itor, + root_addr, + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(key_buffer_key(&touched_key)), + .start_position = + key_start_at_or_after(key_buffer_key(&touched_key))}); bool32 iterator_rejected = STATUS_IS_EQ(height_itor_rc, STATUS_INVALID_STATE); if (SUCCESS(height_itor_rc)) { @@ -1923,15 +1889,7 @@ test_btree_basic(cache *cc, root_addr, 0, PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + &(const btree_iterator_init_options){0}); platform_assert_status_ok(rc); platform_default_log("btree iterator init time %luns\n", platform_timestamp_elapsed(start_time)); @@ -2116,15 +2074,7 @@ test_btree_create_packed_trees(cache *cc, memtable_root_addr(mt), 0, PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + &(const btree_iterator_init_options){0}); platform_assert_status_ok(rc); btree_pack_req req; @@ -2169,21 +2119,18 @@ test_count_tuples_in_range(cache *cc, PAGE_TYPE_BRANCH); platform_assert(0); } - rc = btree_iterator_init(cc, - cfg, - &itor, - root_addr[i], - BTREE_BRANCH_FORMAT_VERSION, - type, - greater_than_or_equal, - low_key, - less_than, - high_key, - greater_than_or_equal, - low_key, - FALSE, - 0, - 1); + rc = btree_iterator_init( + cc, + cfg, + &itor, + root_addr[i], + BTREE_BRANCH_FORMAT_VERSION, + type, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(low_key), + .upper_limit = key_less_than(high_key), + .start_position = key_start_at_or_after(low_key), + .prefetch_lookahead = 1}); if (!SUCCESS(rc)) { return rc; } @@ -2269,21 +2216,18 @@ test_btree_print_all_keys(cache *cc, uint64 i; for (i = 0; i < num_trees; i++) { platform_default_log("tree number %lu\n", i); - platform_status rc = btree_iterator_init(cc, - cfg, - &itor, - root_addr[i], - BTREE_BRANCH_FORMAT_VERSION, - type, - greater_than_or_equal, - low_key, - less_than, - high_key, - greater_than_or_equal, - low_key, - FALSE, - 0, - 1); + platform_status rc = btree_iterator_init( + cc, + cfg, + &itor, + root_addr[i], + BTREE_BRANCH_FORMAT_VERSION, + type, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(low_key), + .upper_limit = key_less_than(high_key), + .start_position = key_start_at_or_after(low_key), + .prefetch_lookahead = 1}); platform_assert_status_ok(rc); while (iterator_can_curr(&itor.super)) { key curr_key; @@ -2349,21 +2293,18 @@ test_btree_merge_basic(cache *cc, key lo = key_buffer_key(&pivot_key[pivot_no]); key hi = key_buffer_key(&pivot_key[pivot_no + 1]); for (uint64 tree_no = 0; tree_no < arity; tree_no++) { - rc = btree_iterator_init(cc, - btree_cfg, - &btree_itor_arr[tree_no], - root_addr[tree_no], - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - lo, - less_than, - hi, - greater_than_or_equal, - lo, - FALSE, - 0, - 1); + rc = + btree_iterator_init(cc, + btree_cfg, + &btree_itor_arr[tree_no], + root_addr[tree_no], + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(lo), + .upper_limit = key_less_than(hi), + .start_position = key_start_at_or_after(lo), + .prefetch_lookahead = 1}); platform_assert_status_ok(rc); itor_arr[tree_no] = &btree_itor_arr[tree_no].super; } @@ -2582,21 +2523,15 @@ test_btree_rough_iterator(cache *cc, platform_assert(rough_itor); for (uint64 tree_no = 0; tree_no < num_trees; tree_no++) { - rc = btree_iterator_init(cc, - btree_cfg, - &rough_btree_itor[tree_no], - root_addr[tree_no], - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - TRUE, - 1, - 1); + rc = btree_iterator_init( + cc, + btree_cfg, + &rough_btree_itor[tree_no], + root_addr[tree_no], + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .copy_nodes = TRUE, .height = 1, .prefetch_lookahead = 1}); platform_assert_status_ok(rc); if (iterator_can_curr(&rough_btree_itor[tree_no].super)) { key curr_key; @@ -2749,21 +2684,18 @@ test_btree_merge_perf(cache *cc, key max_key = key_buffer_key(&pivot_key[pivot_no + 1]); for (uint64 tree_no = 0; tree_no < arity; tree_no++) { uint64 global_tree_no = merge_no * num_merges + tree_no; - rc = btree_iterator_init(cc, - btree_cfg, - &btree_itor_arr[tree_no], - root_addr[global_tree_no], - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - min_key, - less_than, - max_key, - greater_than_or_equal, - min_key, - FALSE, - 0, - 1); + rc = btree_iterator_init( + cc, + btree_cfg, + &btree_itor_arr[tree_no], + root_addr[global_tree_no], + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(min_key), + .upper_limit = key_less_than(max_key), + .start_position = key_start_at_or_after(min_key), + .prefetch_lookahead = 1}); platform_assert_status_ok(rc); itor_arr[tree_no] = &btree_itor_arr[tree_no].super; } diff --git a/tests/functional/test_functionality.c b/tests/functional/test_functionality.c index 56f4ea37f..a4e622495 100644 --- a/tests/functional/test_functionality.c +++ b/tests/functional/test_functionality.c @@ -33,14 +33,8 @@ search_for_key_via_iterator(core_handle *spl, key target) { core_range_iterator iter; - core_range_iterator_init(spl, - &iter, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY); + core_range_iterator_init( + spl, &iter, &(const core_range_iterator_init_options){0}); uint64 count = 0; while (iterator_can_curr((iterator *)&iter)) { key curr_key; @@ -240,14 +234,13 @@ verify_range_against_shadow(core_handle *spl, core_range_iterator *range_itor = TYPED_MALLOC(hid, range_itor); platform_assert(range_itor != NULL); - status = core_range_iterator_init(spl, - range_itor, - greater_than_or_equal, - start_key, - less_than, - end_key, - greater_than_or_equal, - start_key); + status = core_range_iterator_init( + spl, + range_itor, + &(const core_range_iterator_init_options){ + .lower_limit = key_greater_than_or_equal(start_key), + .upper_limit = key_less_than(end_key), + .start_position = key_start_at_or_after(start_key)}); if (!SUCCESS(status)) { platform_error_log("failed to create range itor: %s\n", platform_status_to_string(status)); diff --git a/tests/unit/btree_stress_test.c b/tests/unit/btree_stress_test.c index e567b6c78..aeb1aa62b 100644 --- a/tests/unit/btree_stress_test.c +++ b/tests/unit/btree_stress_test.c @@ -1033,21 +1033,15 @@ iterator_tests(cache *cc, else start_key = POSITIVE_INFINITY_KEY; - platform_status rc = btree_iterator_init(cc, - cfg, - &dbiter, - root_addr, - BTREE_BRANCH_FORMAT_VERSION, - type, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - start_key, - FALSE, - 0, - 0); + platform_status rc = btree_iterator_init( + cc, + cfg, + &dbiter, + root_addr, + BTREE_BRANCH_FORMAT_VERSION, + type, + &(const btree_iterator_init_options){ + .start_position = key_start_at_or_after(start_key)}); ASSERT_TRUE(SUCCESS(rc)); iterator *iter = (iterator *)&dbiter; @@ -1087,21 +1081,15 @@ iterator_seek_tests(cache *cc, // start in the "middle" of the range key start_key = gen_key(cfg, nkvs / 2, keybuf, keybuf_size); - platform_status rc = btree_iterator_init(cc, - cfg, - &dbiter, - root_addr, - 0, - PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - start_key, - FALSE, - 0, - 0); + platform_status rc = btree_iterator_init( + cc, + cfg, + &dbiter, + root_addr, + 0, + PAGE_TYPE_MEMTABLE, + &(const btree_iterator_init_options){ + .start_position = key_start_at_or_after(start_key)}); ASSERT_TRUE(SUCCESS(rc)); iterator *iter = (iterator *)&dbiter; @@ -1136,21 +1124,14 @@ pack_tests(cache *cc, btree_iterator dbiter; iterator *iter = (iterator *)&dbiter; - platform_status rc = btree_iterator_init(cc, - cfg, - &dbiter, - root_addr, - 0, - PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + platform_status rc = + btree_iterator_init(cc, + cfg, + &dbiter, + root_addr, + 0, + PAGE_TYPE_MEMTABLE, + &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); rc = STATUS_TEST_FAILED; @@ -1238,21 +1219,19 @@ bounds_init_with_copy(cache *cc, comparison start_cmp = reverse ? hi_cmp : lo_cmp; key start = reverse ? hi : lo; if (!async) { - return btree_iterator_init(cc, - cfg, - itor, - root, - BTREE_BRANCH_FORMAT_VERSION, - type, - lo_cmp, - lo, - hi_cmp, - hi, - start_cmp, - start, - copy_nodes, - 0, - prefetch_lookahead); + return btree_iterator_init( + cc, + cfg, + itor, + root, + BTREE_BRANCH_FORMAT_VERSION, + type, + &(const btree_iterator_init_options){ + .lower_limit = make_key_lower_limit(lo_cmp, lo), + .upper_limit = make_key_upper_limit(hi_cmp, hi), + .start_position = make_key_start_position(start_cmp, start), + .copy_nodes = copy_nodes, + .prefetch_lookahead = prefetch_lookahead}); } btree_iterator_async_state state; bool32 ready = FALSE; @@ -1390,15 +1369,8 @@ iterator_endpoint_fault_init(cache *cc, root, BTREE_BRANCH_FORMAT_VERSION, PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - upper, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + &(const btree_iterator_init_options){ + .upper_limit = key_less_than(upper)}); } btree_iterator_async_state state; @@ -1442,21 +1414,14 @@ CTEST2(btree_stress, iterator_endpoint_error_is_returned_by_init) } btree_iterator source; - platform_status rc = btree_iterator_init(cc, - cfg, - &source, - source_root, - 0, - PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + platform_status rc = + btree_iterator_init(cc, + cfg, + &source, + source_root, + 0, + PAGE_TYPE_MEMTABLE, + &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); btree_pack_req req; rc = btree_pack_req_init( @@ -1475,15 +1440,7 @@ CTEST2(btree_stress, iterator_endpoint_error_is_returned_by_init) packed_root, BTREE_BRANCH_FORMAT_VERSION, PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); uint64 first_leaf = probe.curr.addr; uint64 upper_num = 0; @@ -1550,21 +1507,14 @@ CTEST2(btree_stress, count_in_range_returns_lookup_error) } btree_iterator source; - platform_status rc = btree_iterator_init(cc, - cfg, - &source, - source_root, - 0, - PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + platform_status rc = + btree_iterator_init(cc, + cfg, + &source, + source_root, + 0, + PAGE_TYPE_MEMTABLE, + &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); btree_pack_req req; rc = btree_pack_req_init( @@ -1868,21 +1818,14 @@ CTEST2(btree_stress, copied_iterator_split_on_release) bounds_insert(cc, cfg, data->hid, data->test_scratch, &mini, root, n); } btree_iterator probe; - platform_status rc = btree_iterator_init(cc, - cfg, - &probe, - root, - 0, - PAGE_TYPE_MEMTABLE, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0); + platform_status rc = + btree_iterator_init(cc, + cfg, + &probe, + root, + 0, + PAGE_TYPE_MEMTABLE, + &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); btree_node leaf = probe.curr; ASSERT_TRUE(leaf.addr != root); diff --git a/tests/unit/btree_test.c b/tests/unit/btree_test.c index ca4737c77..01cc542a4 100644 --- a/tests/unit/btree_test.c +++ b/tests/unit/btree_test.c @@ -309,22 +309,15 @@ CTEST2(btree, test_invalid_branch_iterator) for (bool32 copy_nodes = FALSE; copy_nodes <= TRUE; copy_nodes++) { btree_iterator itor; memset(&itor, 0xa5, sizeof(itor)); - platform_status rc = - btree_iterator_init(NULL, - &data->dbtree_cfg, - &itor, - invalid_branch_refs[i].addr, - invalid_branch_refs[i].format_version, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - copy_nodes, - 0, - 1); + platform_status rc = btree_iterator_init( + NULL, + &data->dbtree_cfg, + &itor, + invalid_branch_refs[i].addr, + invalid_branch_refs[i].format_version, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){.copy_nodes = copy_nodes, + .prefetch_lookahead = 1}); ASSERT_TRUE(STATUS_IS_EQ(rc, STATUS_INVALID_STATE)); btree_iterator_deinit(&itor); diff --git a/tests/unit/splinter_test.c b/tests/unit/splinter_test.c index 8d68a138f..b3f1a04af 100644 --- a/tests/unit/splinter_test.c +++ b/tests/unit/splinter_test.c @@ -1681,14 +1681,8 @@ CTEST2(splinter, test_invalid_branch_reference_rejected) lookup_result_deinit(&result); core_range_iterator itor; - range_rc[invalid_field] = core_range_iterator_init(&spl, - &itor, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY); + range_rc[invalid_field] = core_range_iterator_init( + &spl, &itor, &(const core_range_iterator_init_options){0}); if (SUCCESS(range_rc[invalid_field])) { core_range_iterator_deinit(&itor); } @@ -2243,14 +2237,8 @@ CTEST2(splinter, test_reopen_with_changed_filter_config) } lookup_result_deinit(&result); core_range_iterator itor; - ASSERT_TRUE(SUCCESS(core_range_iterator_init(&spl, - &itor, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY))); + ASSERT_TRUE(SUCCESS(core_range_iterator_init( + &spl, &itor, &(const core_range_iterator_init_options){0}))); uint64 count = 0; while (iterator_can_next(&itor.super)) { count++; @@ -6213,14 +6201,8 @@ CTEST2(splinter, test_cache_read_errors_reach_core_operations) core_range_iterator itor; cc->ops = &core_read_fault_ops; - rc = core_range_iterator_init(&spl, - &itor, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY); + rc = core_range_iterator_init( + &spl, &itor, &(const core_range_iterator_init_options){0}); cc->ops = core_read_saved_ops; ASSERT_TRUE(STATUS_IS_EQ(rc, STATUS_TIMEDOUT)); core_read_fail_async = FALSE; diff --git a/tests/unit/trunk_split_test.c b/tests/unit/trunk_split_test.c index c84678e51..603bca761 100644 --- a/tests/unit/trunk_split_test.c +++ b/tests/unit/trunk_split_test.c @@ -237,21 +237,14 @@ CTEST2(trunk_split, split_scan_propagates_io_error) // Find the second leaf in a valid branch. Bounds/count lookups visit the // endpoints; the split scan must fetch this interior page while advancing. btree_iterator itor; - ASSERT_TRUE(SUCCESS(btree_iterator_init(context->cc, - data->cfg.btree_cfg, - &itor, - branch, - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0))); + ASSERT_TRUE( + SUCCESS(btree_iterator_init(context->cc, + data->cfg.btree_cfg, + &itor, + branch, + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){0}))); uint64 first_page = itor.curr.addr; while (itor.curr.addr == first_page && iterator_can_next(&itor.super)) { ASSERT_TRUE(SUCCESS(iterator_next(&itor.super))); From 321a879527b84c6b5a5046cd4cdef2849cb2e18b Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Wed, 14 Oct 2026 14:28:49 -0700 Subject: [PATCH 05/10] Internal options for merge_iterator_create (folds in _with_filter) and btree_pack_req_init Co-Authored-By: Claude Sonnet 5.5 --- src/btree.h | 34 ++++++++++++++++----- src/core.c | 24 +++++++-------- src/merge.c | 39 +++++++----------------- src/merge.h | 51 +++++++++++++++++++------------- src/trunk.c | 6 ++-- tests/functional/btree_test.c | 17 ++++------- tests/unit/btree_stress_test.c | 11 +++---- tests/unit/merge_test.c | 54 +++++++++++++++++----------------- tests/unit/splinter_test.c | 20 ++++--------- tests/unit/trunk_split_test.c | 5 ++-- 10 files changed, 125 insertions(+), 136 deletions(-) diff --git a/src/btree.h b/src/btree.h index 608f6f286..58da385a0 100644 --- a/src/btree.h +++ b/src/btree.h @@ -496,16 +496,34 @@ btree_iterator_init_async_result(btree_iterator_async_state *state); void btree_iterator_deinit(btree_iterator *itor); +typedef struct btree_pack_req_init_options { + unsigned int seed; // default: 0 + bool32 collect_fingerprints; // default: FALSE +} btree_pack_req_init_options; + +// Usage: btree_pack_req_init(req, cc, cfg, itor, max_tuples, hid, +// .seed = seed, .collect_fingerprints = TRUE); +// The options are optional. +#define btree_pack_req_init(req, cc, cfg, itor, max_tuples, hid, ...) \ + btree_pack_req_init_full((req), \ + (cc), \ + (cfg), \ + (itor), \ + (max_tuples), \ + (hid), \ + &(const btree_pack_req_init_options){__VA_ARGS__}) + static inline platform_status -btree_pack_req_init(btree_pack_req *req, - cache *cc, - const btree_config *cfg, - iterator *itor, - uint64 max_tuples, - unsigned int seed, - bool32 collect_fingerprints, - platform_heap_id hid) +btree_pack_req_init_full(btree_pack_req *req, + cache *cc, + const btree_config *cfg, + iterator *itor, + uint64 max_tuples, + platform_heap_id hid, + const btree_pack_req_init_options *options) { + unsigned int seed = options->seed; + bool32 collect_fingerprints = options->collect_fingerprints; memset(req, 0, sizeof(*req)); req->cc = cc; req->cfg = cfg; diff --git a/src/core.c b/src/core.c index ae23a16de..4de81030c 100644 --- a/src/core.c +++ b/src/core.c @@ -1462,9 +1462,8 @@ core_memtable_compact(core_handle *spl, uint64 generation, const threadid tid) spl->cfg.btree_cfg, itor, rflimit, - rfcfg->seed, - FALSE, - spl->heap_id); + spl->heap_id, + .seed = rfcfg->seed); if (!SUCCESS(rc)) { goto deinit_iterator; } @@ -2431,16 +2430,15 @@ core_range_iterator_init_span(core_handle *spl, greater_than <= start_key_comparison, &range_itor->merge_itor); } else { - rc = - merge_iterator_create_with_filter(PROCESS_PRIVATE_HEAP_ID, - spl->cfg.data_cfg, - range_itor->num_branches, - range_itor->itor, - MERGE_FULL, - greater_than <= start_key_comparison, - core_range_iterator_key_matches, - range_itor->filter, - &range_itor->merge_itor); + rc = merge_iterator_create(PROCESS_PRIVATE_HEAP_ID, + spl->cfg.data_cfg, + range_itor->num_branches, + range_itor->itor, + MERGE_FULL, + greater_than <= start_key_comparison, + &range_itor->merge_itor, + .keep_key = core_range_iterator_key_matches, + .filter_context = range_itor->filter); } if (!SUCCESS(rc)) { core_range_iterator_span_deinit(range_itor); diff --git a/src/merge.c b/src/merge.c index 10b3abc4f..81094c730 100644 --- a/src/merge.c +++ b/src/merge.c @@ -589,36 +589,17 @@ setup_ordered_iterators(merge_iterator *merge_itor) *----------------------------------------------------------------------------- */ platform_status -merge_iterator_create(platform_heap_id hid, - const data_config *cfg, - int num_trees, - iterator **itor_arr, - merge_behavior merge_mode, - bool32 forwards, - merge_iterator **out_itor) -{ - return merge_iterator_create_with_filter(hid, - cfg, - num_trees, - itor_arr, - merge_mode, - forwards, - NULL, - NULL, - out_itor); -} - -platform_status -merge_iterator_create_with_filter(platform_heap_id hid, - const data_config *cfg, - int num_trees, - iterator **itor_arr, - merge_behavior merge_mode, - bool32 forwards, - merge_key_filter keep_key, - void *filter_context, - merge_iterator **out_itor) +merge_iterator_create_full(platform_heap_id hid, + const data_config *cfg, + int num_trees, + iterator **itor_arr, + merge_behavior merge_mode, + bool32 forwards, + merge_iterator **out_itor, + const merge_iterator_options *options) { + merge_key_filter keep_key = options->keep_key; + void *filter_context = options->filter_context; int i; platform_status rc = STATUS_OK; merge_iterator *merge_itor; diff --git a/src/merge.h b/src/merge.h index f5e6e9ea7..d5a14fb84 100644 --- a/src/merge.h +++ b/src/merge.h @@ -84,28 +84,37 @@ typedef struct merge_iterator { merge_accumulator merge_buffer; } merge_iterator; +typedef struct merge_iterator_options { + /* Reject keys before any message merge/finalization. The predicate and its + * context must remain valid until destruction and be stable for both scan + * directions. NULL (the default) retains the ordinary iterator path. */ + merge_key_filter keep_key; + void *filter_context; +} merge_iterator_options; + +/* Usage: merge_iterator_create(hid, cfg, n, itors, MERGE_FULL, TRUE, &out); + * merge_iterator_create(hid, cfg, n, itors, MERGE_FULL, TRUE, &out, + * .keep_key = pred, .filter_context = ctx); */ +#define merge_iterator_create( \ + hid, cfg, num_trees, itor_arr, merge_mode, forwards, out_itor, ...) \ + merge_iterator_create_full((hid), \ + (cfg), \ + (num_trees), \ + (itor_arr), \ + (merge_mode), \ + (forwards), \ + (out_itor), \ + &(const merge_iterator_options){__VA_ARGS__}) + platform_status -merge_iterator_create(platform_heap_id hid, - const data_config *cfg, - int num_trees, - iterator **itor_arr, - merge_behavior merge_mode, - bool32 forwards, - merge_iterator **out_itor); - -/* Reject keys before any message merge/finalization. The predicate and its - * context must remain valid until destruction and be stable for both scan - * directions. NULL retains the ordinary iterator path. */ -platform_status -merge_iterator_create_with_filter(platform_heap_id hid, - const data_config *cfg, - int num_trees, - iterator **itor_arr, - merge_behavior merge_mode, - bool32 forwards, - merge_key_filter keep_key, - void *filter_context, - merge_iterator **out_itor); +merge_iterator_create_full(platform_heap_id hid, + const data_config *cfg, + int num_trees, + iterator **itor_arr, + merge_behavior merge_mode, + bool32 forwards, + merge_iterator **out_itor, + const merge_iterator_options *options); platform_status merge_iterator_destroy(platform_heap_id hid, merge_iterator **merge_itor); diff --git a/src/trunk.c b/src/trunk.c index 193adb094..2f7a741ae 100644 --- a/src/trunk.c +++ b/src/trunk.c @@ -4909,9 +4909,9 @@ bundle_compaction_task(task *arg) context->cfg->btree_cfg, &merger.merge_itor->super, bc->input.stats.num_tuples, - state->hash_seed, - TRUE, - context->hid); + context->hid, + .seed = state->hash_seed, + .collect_fingerprints = TRUE); if (!SUCCESS(rc)) { platform_error_log( "btree_pack_req_init failed for state: %p bc: %p: %s\n", diff --git a/tests/functional/btree_test.c b/tests/functional/btree_test.c index d29a91372..185e805f9 100644 --- a/tests/functional/btree_test.c +++ b/tests/functional/btree_test.c @@ -151,8 +151,7 @@ test_btree_pack_height_limit(cache *cc, } btree_pack_req req; - platform_status rc = - btree_pack_req_init(&req, cc, cfg, &itor.super, 0, 0, FALSE, hid); + platform_status rc = btree_pack_req_init(&req, cc, cfg, &itor.super, 0, hid); if (!SUCCESS(rc)) { return rc; } @@ -955,8 +954,7 @@ test_btree_scan_perf(cache *cc, } btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, btree_cfg, (iterator *)&itor, 0, 0, FALSE, hid); + rc = btree_pack_req_init(&req, cc, btree_cfg, (iterator *)&itor, 0, hid); if (!SUCCESS(rc)) { btree_iterator_deinit(&itor); goto out; @@ -1894,8 +1892,7 @@ test_btree_basic(cache *cc, platform_default_log("btree iterator init time %luns\n", platform_timestamp_elapsed(start_time)); btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, btree_cfg, (iterator *)&itor, 0, 0, FALSE, NULL); + rc = btree_pack_req_init(&req, cc, btree_cfg, (iterator *)&itor, 0, NULL); platform_assert_status_ok(rc); btree_print_tree_stats(Platform_default_log_handle, @@ -2078,8 +2075,7 @@ test_btree_create_packed_trees(cache *cc, platform_assert_status_ok(rc); btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, btree_cfg, &itor.super, 0, 0, FALSE, hid); + rc = btree_pack_req_init(&req, cc, btree_cfg, &itor.super, 0, hid); platform_assert_status_ok(rc); rc = btree_pack(&req); @@ -2321,8 +2317,7 @@ test_btree_merge_basic(cache *cc, } btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, btree_cfg, &merge_itor->super, 0, 0, FALSE, hid); + rc = btree_pack_req_init(&req, cc, btree_cfg, &merge_itor->super, 0, hid); platform_assert_status_ok(rc); btree_pack(&req); output_addr[pivot_no] = req.root_addr; @@ -2713,7 +2708,7 @@ test_btree_merge_perf(cache *cc, btree_pack_req req; rc = btree_pack_req_init( - &req, cc, btree_cfg, &merge_itor->super, 0, 0, FALSE, hid); + &req, cc, btree_cfg, &merge_itor->super, 0, hid); platform_assert_status_ok(rc); btree_pack(&req); diff --git a/tests/unit/btree_stress_test.c b/tests/unit/btree_stress_test.c index aeb1aa62b..e570f88d4 100644 --- a/tests/unit/btree_stress_test.c +++ b/tests/unit/btree_stress_test.c @@ -1136,7 +1136,7 @@ pack_tests(cache *cc, rc = STATUS_TEST_FAILED; btree_pack_req req; - rc = btree_pack_req_init(&req, cc, cfg, iter, nkvs, 0, FALSE, hid); + rc = btree_pack_req_init(&req, cc, cfg, iter, nkvs, hid); ASSERT_TRUE(SUCCESS(rc)); if (!SUCCESS(btree_pack(&req))) { @@ -1424,8 +1424,7 @@ CTEST2(btree_stress, iterator_endpoint_error_is_returned_by_init) &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, cfg, &source.super, count, 0, FALSE, data->hid); + rc = btree_pack_req_init(&req, cc, cfg, &source.super, count, data->hid); ASSERT_TRUE(SUCCESS(rc)); rc = btree_pack(&req); uint64 packed_root = req.root_addr; @@ -1517,8 +1516,7 @@ CTEST2(btree_stress, count_in_range_returns_lookup_error) &(const btree_iterator_init_options){0}); ASSERT_TRUE(SUCCESS(rc)); btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, cfg, &source.super, count, 0, FALSE, data->hid); + rc = btree_pack_req_init(&req, cc, cfg, &source.super, count, data->hid); ASSERT_TRUE(SUCCESS(rc)); rc = btree_pack(&req); uint64 packed_root = req.root_addr; @@ -1920,8 +1918,7 @@ CTEST2(btree_stress, copied_iterator_prefetch) 0); ASSERT_TRUE(SUCCESS(rc)); btree_pack_req req; - rc = btree_pack_req_init( - &req, cc, cfg, &source.super, count, 0, FALSE, data->hid); + rc = btree_pack_req_init(&req, cc, cfg, &source.super, count, data->hid); ASSERT_TRUE(SUCCESS(rc)); rc = btree_pack(&req); uint64 packed_root = req.root_addr; diff --git a/tests/unit/merge_test.c b/tests/unit/merge_test.c index 07c81ce0a..27eb09557 100644 --- a/tests/unit/merge_test.c +++ b/tests/unit/merge_test.c @@ -523,15 +523,15 @@ CTEST(merge, filtering_does_not_eagerly_drain_nonmatching_sources) } merge_iterator *merge = NULL; ASSERT_TRUE( - SUCCESS(merge_iterator_create_with_filter(PROCESS_PRIVATE_HEAP_ID, - &cfg, - ARRAY_SIZE(inputs), - inputs, - MERGE_FULL, - forwards, - merge_test_keep_one_key, - &selected, - &merge))); + SUCCESS(merge_iterator_create(PROCESS_PRIVATE_HEAP_ID, + &cfg, + ARRAY_SIZE(inputs), + inputs, + MERGE_FULL, + forwards, + &merge, + .keep_key = merge_test_keep_one_key, + .filter_context = &selected))); merge_test_check_curr(merge, selected, "v"); ASSERT_EQUAL(1, children[1].steps); // Cross each bound and reverse; rejected update histories stay hidden. @@ -574,15 +574,15 @@ CTEST(merge, filtering_propagates_errors_while_skipping_rejected_keys) iterator *inputs[] = {&child.super}; merge_iterator *merge = NULL; ASSERT_TRUE( - SUCCESS(merge_iterator_create_with_filter(PROCESS_PRIVATE_HEAP_ID, - &cfg, - 1, - inputs, - MERGE_FULL, - forwards, - merge_test_keep_edges, - NULL, - &merge))); + SUCCESS(merge_iterator_create(PROCESS_PRIVATE_HEAP_ID, + &cfg, + 1, + inputs, + MERGE_FULL, + forwards, + &merge, + .keep_key = merge_test_keep_edges, + .filter_context = NULL))); merge_test_check_curr(merge, forwards ? 'a' : 'c', "v"); platform_status rc = forwards ? iterator_next(&merge->super) : iterator_prev(&merge->super); @@ -598,15 +598,15 @@ CTEST(merge, filtering_propagates_errors_while_skipping_rejected_keys) child.steps = 0; child.failed = FALSE; char selected = forwards ? 'c' : 'a'; - rc = merge_iterator_create_with_filter(PROCESS_PRIVATE_HEAP_ID, - &cfg, - 1, - inputs, - MERGE_FULL, - forwards, - merge_test_keep_one_key, - &selected, - &merge); + rc = merge_iterator_create(PROCESS_PRIVATE_HEAP_ID, + &cfg, + 1, + inputs, + MERGE_FULL, + forwards, + &merge, + .keep_key = merge_test_keep_one_key, + .filter_context = &selected); ASSERT_EQUAL(STATUS_IO_ERROR.r, rc.r); ASSERT_NULL(merge); ASSERT_TRUE(child.failed); diff --git a/tests/unit/splinter_test.c b/tests/unit/splinter_test.c index b3f1a04af..5445f6b3e 100644 --- a/tests/unit/splinter_test.c +++ b/tests/unit/splinter_test.c @@ -1362,9 +1362,8 @@ CTEST2(splinter, test_btree_pack_abort_releases_reserved_extents) &data->system_cfg->btree_cfg, &itor.super, 1, - 42, - FALSE, - data->hid); + data->hid, + .seed = 42); ASSERT_TRUE(SUCCESS(rc)); uint64 before = allocator_in_use(alp); @@ -1404,9 +1403,8 @@ CTEST2(splinter, test_btree_pack_rejects_counter_overflow) &data->system_cfg->btree_cfg, &itor.super, 1, - 42, - FALSE, - data->hid); + data->hid, + .seed = 42); ASSERT_TRUE(SUCCESS(rc)); uint64 before = allocator_in_use(alp); @@ -6581,14 +6579,8 @@ CTEST2(splinter, test_blob_height_three_and_large_stats) .msg = message_create(MESSAGE_TYPE_INSERT, cc, desc)}; btree_pack_req *req = TYPED_MALLOC(data->hid, req); ASSERT_NOT_NULL(req); - ASSERT_TRUE(SUCCESS(btree_pack_req_init(req, - cc, - &data->system_cfg->btree_cfg, - &iter.super, - 16, - 0, - FALSE, - data->hid))); + ASSERT_TRUE(SUCCESS(btree_pack_req_init( + req, cc, &data->system_cfg->btree_cfg, &iter.super, 16, data->hid))); ASSERT_TRUE(SUCCESS(btree_pack(req))); ASSERT_EQUAL(16 * length, req->message_bytes); ASSERT_TRUE(req->height > 0); diff --git a/tests/unit/trunk_split_test.c b/tests/unit/trunk_split_test.c index 603bca761..57e1d5483 100644 --- a/tests/unit/trunk_split_test.c +++ b/tests/unit/trunk_split_test.c @@ -69,9 +69,8 @@ split_test_pack(trunk_context *context, uint64 count) context->cfg->btree_cfg, &itor.super, count, - 42, - FALSE, - context->hid))); + context->hid, + .seed = 42))); ASSERT_TRUE(SUCCESS(btree_pack(&req))); uint64 addr = req.root_addr; btree_pack_req_deinit(&req, context->hid); From 4c45b3776b604277d90660ae2253e2f587a3e545 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Sun, 25 Oct 2026 11:54:46 -0700 Subject: [PATCH 06/10] Convert call sites added upstream since the branch point to the new APIs Co-Authored-By: Claude Sonnet 5.5 --- benchmarks/btree_prefix_bench.c | 28 ++-- tests/unit/splinter_test.c | 10 +- tests/unit/splinterdb_quick_test.c | 198 +++++++++++------------------ 3 files changed, 85 insertions(+), 151 deletions(-) diff --git a/benchmarks/btree_prefix_bench.c b/benchmarks/btree_prefix_bench.c index fc223d2d5..4156d5796 100644 --- a/benchmarks/btree_prefix_bench.c +++ b/benchmarks/btree_prefix_bench.c @@ -508,22 +508,16 @@ init_iterator(benchmark *b, page_type type, key start) { - check(btree_iterator_init((cache *)&b->cc, - &b->btree_cfg, - it, - root, - BTREE_BRANCH_FORMAT_VERSION, - type, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - start, - FALSE, - 0, - 0), - "iterator init"); + check( + btree_iterator_init((cache *)&b->cc, + &b->btree_cfg, + it, + root, + BTREE_BRANCH_FORMAT_VERSION, + type, + &(const btree_iterator_init_options){ + .start_position = key_start_at_or_after(start)}), + "iterator init"); } static void @@ -602,8 +596,6 @@ pack_once(benchmark *b, bool measured) &b->btree_cfg, &it.super, b->opt.keys, - 0, - FALSE, b->hid), "pack req init"); counters before = {0}; diff --git a/tests/unit/splinter_test.c b/tests/unit/splinter_test.c index 5445f6b3e..ecc2210c2 100644 --- a/tests/unit/splinter_test.c +++ b/tests/unit/splinter_test.c @@ -1792,14 +1792,8 @@ CTEST2(splinter, test_persisted_old_branch_version_rejected) &result); lookup_result_deinit(&result); core_range_iterator itor; - range_rc = core_range_iterator_init(&spl, - &itor, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY); + range_rc = core_range_iterator_init( + &spl, &itor, &(const core_range_iterator_init_options){0}); if (SUCCESS(range_rc)) { core_range_iterator_deinit(&itor); } diff --git a/tests/unit/splinterdb_quick_test.c b/tests/unit/splinterdb_quick_test.c index f3b7c4560..f63a46970 100644 --- a/tests/unit/splinterdb_quick_test.c +++ b/tests/unit/splinterdb_quick_test.c @@ -1346,26 +1346,17 @@ CTEST2(splinterdb_quick, test_branch_prefix_pack_initializes_free_gap) core->cfg.btree_cfg, &input.super, ARRAY_SIZE(input.keys), - 0, - FALSE, core->heap_id))); ASSERT_TRUE(SUCCESS(btree_pack(&req))); btree_iterator scan; - ASSERT_TRUE(SUCCESS(btree_iterator_init(core->cc, - core->cfg.btree_cfg, - &scan, - req.root_addr, - BTREE_BRANCH_FORMAT_VERSION, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0))); + ASSERT_TRUE( + SUCCESS(btree_iterator_init(core->cc, + core->cfg.btree_cfg, + &scan, + req.root_addr, + BTREE_BRANCH_FORMAT_VERSION, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){0}))); btree_hdr *hdr = scan.curr.hdr; ASSERT_EQUAL(0, hdr->height); ASSERT_TRUE(hdr->prefix_length != 0); @@ -1442,21 +1433,15 @@ prefix_test_init_failure(core_handle *core, platform_memory_fault_reset_counters(); platform_memory_fault_enable(); btree_iterator itor; - platform_status rc = btree_iterator_init(core->cc, - core->cfg.btree_cfg, - &itor, - branch->addr, - branch->format_version, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - start_key, - FALSE, - 0, - 0); + platform_status rc = btree_iterator_init( + core->cc, + core->cfg.btree_cfg, + &itor, + branch->addr, + branch->format_version, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){ + .start_position = key_start_at_or_after(start_key)}); platform_memory_fault_counters counters = platform_memory_fault_get_counters(); platform_memory_fault_disable(); @@ -1498,14 +1483,8 @@ CTEST2(splinterdb_quick, test_branch_prefix_minimum) core_handle *core = (core_handle *)splinterdb_get_trunk_handle(data->kvsb); ASSERT_TRUE(SUCCESS(core_checkpoint(core, 0))); core_range_iterator scan; - ASSERT_TRUE(SUCCESS(core_range_iterator_init(core, - &scan, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY))); + ASSERT_TRUE(SUCCESS(core_range_iterator_init( + core, &scan, &(const core_range_iterator_init_options){0}))); uint64 ordinal = 0; uint32 compressed_groups = 0, uncompressed_groups = 0; while (iterator_can_curr(&scan.super)) { @@ -1611,15 +1590,9 @@ prefix_variable_test_growth_failure(core_handle *core, branch->addr, branch->format_version, PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - key_create_from_slice(TRUE, prefix_variable_test_key(keybuf, ordinal)), - FALSE, - 0, - 0))); + &(const btree_iterator_init_options){ + .start_position = key_start_at_or_after(key_create_from_slice( + TRUE, prefix_variable_test_key(keybuf, ordinal)))}))); prefix_variable_check_current(&itor.super, ordinal); ASSERT_TRUE(itor.curr.hdr->prefix_length != 0); ASSERT_TRUE(itor.idx + 1 < itor.curr.hdr->num_entries); @@ -1662,14 +1635,8 @@ CTEST2(splinterdb_quick, test_branch_prefix_variable_iterator) core_handle *core = (core_handle *)splinterdb_get_trunk_handle(data->kvsb); ASSERT_TRUE(SUCCESS(core_checkpoint(core, 0))); core_range_iterator probe; - ASSERT_TRUE(SUCCESS(core_range_iterator_init(core, - &probe, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY))); + ASSERT_TRUE(SUCCESS(core_range_iterator_init( + core, &probe, &(const core_range_iterator_init_options){0}))); uint64 total = 0, grows = 0, shrinks = 0, leaf_changes = 0; uint64 compressed_lengths = 0; @@ -1794,21 +1761,14 @@ prefix_test_malformed_leaf_entries(core_handle *core, trunk_branch_info *branch) ASSERT_EQUAL(sizeof(saved_page), page_size); for (uint64 fault = 0; fault < NUM_FAULTS; fault++) { btree_iterator itor; - ASSERT_TRUE(SUCCESS(btree_iterator_init(core->cc, - core->cfg.btree_cfg, - &itor, - branch->addr, - branch->format_version, - PAGE_TYPE_BRANCH, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - FALSE, - 0, - 0))); + ASSERT_TRUE( + SUCCESS(btree_iterator_init(core->cc, + core->cfg.btree_cfg, + &itor, + branch->addr, + branch->format_version, + PAGE_TYPE_BRANCH, + &(const btree_iterator_init_options){0}))); btree_hdr *hdr = itor.curr.hdr; uint64 next = itor.idx + 1; ASSERT_TRUE(next < hdr->num_entries); @@ -1900,14 +1860,8 @@ CTEST2(splinterdb_quick, test_branch_prefix_malformed_leaf_entries) core_handle *core = (core_handle *)splinterdb_get_trunk_handle(data->kvsb); ASSERT_TRUE(SUCCESS(core_checkpoint(core, 0))); core_range_iterator probe; - ASSERT_TRUE(SUCCESS(core_range_iterator_init(core, - &probe, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY))); + ASSERT_TRUE(SUCCESS(core_range_iterator_init( + core, &probe, &(const core_range_iterator_init_options){0}))); bool32 tested = FALSE; for (uint64 b = 0; b < probe.num_branches; b++) { if (probe.btree_itor_initialized[b] @@ -1964,14 +1918,8 @@ CTEST2(splinterdb_quick, test_branch_prefix_roundtrip) ASSERT_TRUE(SUCCESS(core_checkpoint(core, 0))); core_range_iterator probe; - ASSERT_TRUE(SUCCESS(core_range_iterator_init(core, - &probe, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY))); + ASSERT_TRUE(SUCCESS(core_range_iterator_init( + core, &probe, &(const core_range_iterator_init_options){0}))); bool32 compressed = FALSE; for (uint64 i = 0; i < probe.num_branches; i++) { if (probe.btree_itor_initialized[i] @@ -2101,9 +2049,7 @@ CTEST2(splinterdb_quick, test_branch_prefix_roundtrip) splinterdb_lookup_result_deinit(&result); splinterdb_iterator *it; - ASSERT_EQUAL(0, - splinterdb_iterator_init( - data->kvsb, &it, greater_than_or_equal, NULL_SLICE)); + ASSERT_EQUAL(0, splinterdb_iterator_init(data->kvsb, &it)); slice k, v; ASSERT_TRUE(splinterdb_iterator_valid(it)); splinterdb_iterator_get_current(it, &k, &v); @@ -2147,15 +2093,17 @@ CTEST2(splinterdb_quick, test_branch_prefix_roundtrip) for (uint64 reverse = 0; reverse < 2; reverse++) { ASSERT_EQUAL( 0, - splinterdb_iterator_init_with_bounds( + splinterdb_iterator_init( data->kvsb, &it, - min_inclusive ? greater_than_or_equal : greater_than, - min, - max_inclusive ? less_than_or_equal : less_than, - max, - reverse ? less_than_or_equal : greater_than_or_equal, - NULL_SLICE)); + .lower_limit = splinterdb_make_lower_limit( + min_inclusive ? greater_than_or_equal : greater_than, + min), + .upper_limit = splinterdb_make_upper_limit( + max_inclusive ? less_than_or_equal : less_than, max), + .start_position = splinterdb_make_start_position( + reverse ? less_than_or_equal : greater_than_or_equal, + NULL_SLICE))); for (uint64 i = first; i <= last; i++) { uint64 logical = reverse ? last - (i - first) : i; ASSERT_TRUE(splinterdb_iterator_valid(it)); @@ -2185,14 +2133,16 @@ CTEST2(splinterdb_quick, test_branch_prefix_roundtrip) }; ASSERT_EQUAL( 0, - splinterdb_iterator_init_with_filter( + splinterdb_iterator_init( data->kvsb, &it, - reverse ? less_than_or_equal : greater_than_or_equal, - NULL_SLICE, - prefix_test_predicate, - &predicate, - prune ? predicate.expected : NULL_SLICE)); + .start_position = splinterdb_make_start_position( + reverse ? less_than_or_equal : greater_than_or_equal, + NULL_SLICE), + .predicate = prefix_test_predicate, + .predicate_context = &predicate, + .routing_filter_key = + prune ? predicate.expected : NULL_SLICE)); ASSERT_TRUE(splinterdb_iterator_valid(it)); splinterdb_iterator_get_current(it, &k, &v); ASSERT_EQUAL(0, slice_lex_cmp(k, predicate.expected)); @@ -2215,8 +2165,9 @@ CTEST2(splinterdb_quick, test_branch_prefix_roundtrip) splinterdb_iterator_init( data->kvsb, &it, - side ? less_than_or_equal : greater_than_or_equal, - slice_create(sizeof(keybuf), keybuf))); + .start_position = splinterdb_make_start_position( + side ? less_than_or_equal : greater_than_or_equal, + slice_create(sizeof(keybuf), keybuf)))); ASSERT_TRUE(splinterdb_iterator_valid(it)); splinterdb_iterator_get_current(it, &k, &v); if (side) { @@ -3458,14 +3409,8 @@ CTEST2(splinterdb_quick, test_immutable_branch_custom_comparator) core = (core_handle *)splinterdb_get_trunk_handle(data->kvsb); core_range_iterator probe; - ASSERT_TRUE(SUCCESS(core_range_iterator_init(core, - &probe, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY, - less_than, - POSITIVE_INFINITY_KEY, - greater_than_or_equal, - NEGATIVE_INFINITY_KEY))); + ASSERT_TRUE(SUCCESS(core_range_iterator_init( + core, &probe, &(const core_range_iterator_init_options){0}))); bool32 visited[COUNT] = {0}; lookup_result result; lookup_result_init( @@ -3592,11 +3537,12 @@ CTEST2(splinterdb_quick, test_immutable_branch_custom_comparator) for (bool32 reverse = FALSE; reverse <= TRUE; reverse++) { splinterdb_iterator *it; ASSERT_EQUAL(0, - splinterdb_iterator_init(data->kvsb, - &it, - reverse ? less_than_or_equal - : greater_than_or_equal, - NULL_SLICE)); + splinterdb_iterator_init( + data->kvsb, + &it, + .start_position = splinterdb_make_start_position( + reverse ? less_than_or_equal : greater_than_or_equal, + NULL_SLICE))); for (uint64 i = 0; i < COUNT; i++) { uint64 expected = reverse ? COUNT - 1 - i : i; slice k, v; @@ -3619,12 +3565,14 @@ CTEST2(splinterdb_quick, test_immutable_branch_custom_comparator) for (uint64 i = 0; i < COUNT; i += 31) { uint64 expected = i + !reverse; - ASSERT_EQUAL(0, - splinterdb_iterator_init( - data->kvsb, - &it, - reverse ? less_than_or_equal : greater_than_or_equal, - nonlexical_branch_test_key(keybuf, 2 * i + 1))); + ASSERT_EQUAL( + 0, + splinterdb_iterator_init( + data->kvsb, + &it, + .start_position = splinterdb_make_start_position( + reverse ? less_than_or_equal : greater_than_or_equal, + nonlexical_branch_test_key(keybuf, 2 * i + 1)))); if (expected < COUNT) { slice k, v; ASSERT_TRUE(splinterdb_iterator_valid(it)); From 62a19373df79a1b357ff0151635bc3185aefedbe Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Sun, 25 Oct 2026 12:22:32 -0700 Subject: [PATCH 07/10] Convert remaining positional lookup_result_init calls (clang -Wmissing-braces) Co-Authored-By: Claude Sonnet 5.5 --- tests/unit/splinterdb_forked_child_test.c | 3 +-- tests/unit/splinterdb_quick_test.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/unit/splinterdb_forked_child_test.c b/tests/unit/splinterdb_forked_child_test.c index d9b916ade..69a8461f4 100644 --- a/tests/unit/splinterdb_forked_child_test.c +++ b/tests/unit/splinterdb_forked_child_test.c @@ -151,8 +151,7 @@ CTEST2(splinterdb_forked_child, automatic_log_barriers_survive_process_exit) splinterdb *db; ASSERT_EQUAL(0, splinterdb_open(&cfg, &db)); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - db, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(db, &result); for (uint64 i = 0; i < count; i++) { ASSERT_EQUAL( 0, splinterdb_lookup(db, slice_create(sizeof(i), &i), &result)); diff --git a/tests/unit/splinterdb_quick_test.c b/tests/unit/splinterdb_quick_test.c index f63a46970..6188b0061 100644 --- a/tests/unit/splinterdb_quick_test.c +++ b/tests/unit/splinterdb_quick_test.c @@ -2015,8 +2015,7 @@ CTEST2(splinterdb_quick, test_branch_prefix_roundtrip) ASSERT_EQUAL(0, splinterdb_open(&data->cfg, &data->kvsb)); core = (core_handle *)splinterdb_get_trunk_handle(data->kvsb); splinterdb_lookup_result result; - splinterdb_lookup_result_init( - data->kvsb, &result, SPLINTERDB_LOOKUP_VALUE, 0, NULL); + splinterdb_lookup_result_init(data->kvsb, &result); for (uint64 i = 0; i < count; i++) { slice k = prefix_test_key(keybuf, i * 2); ASSERT_EQUAL(0, splinterdb_lookup(data->kvsb, k, &result)); From 1718894ed18cf9afcbf26f44c4763e9b3d844dc9 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Sun, 25 Oct 2026 12:23:37 -0700 Subject: [PATCH 08/10] formatting Signed-off-by: Rob Johnson --- benchmarks/btree_prefix_bench.c | 11 ++++------- src/merge.c | 6 +++--- src/splinterdb.c | 10 +++++----- tests/unit/splinterdb_quick_test.c | 6 +++--- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/benchmarks/btree_prefix_bench.c b/benchmarks/btree_prefix_bench.c index 4156d5796..8d7cd0ea7 100644 --- a/benchmarks/btree_prefix_bench.c +++ b/benchmarks/btree_prefix_bench.c @@ -591,13 +591,10 @@ pack_once(benchmark *b, bool measured) array_iterator it = { .super = {.ops = &array_ops}, .tuples = b->tuples, .count = b->opt.keys}; btree_pack_req *req = allocate(1, sizeof(*req)); - check(btree_pack_req_init(req, - (cache *)&b->cc, - &b->btree_cfg, - &it.super, - b->opt.keys, - b->hid), - "pack req init"); + check( + btree_pack_req_init( + req, (cache *)&b->cc, &b->btree_cfg, &it.super, b->opt.keys, b->hid), + "pack req init"); counters before = {0}; if (measured) { before = begin_interval(b); diff --git a/src/merge.c b/src/merge.c index 81094c730..7f9a365a3 100644 --- a/src/merge.c +++ b/src/merge.c @@ -600,9 +600,9 @@ merge_iterator_create_full(platform_heap_id hid, { merge_key_filter keep_key = options->keep_key; void *filter_context = options->filter_context; - int i; - platform_status rc = STATUS_OK; - merge_iterator *merge_itor; + int i; + platform_status rc = STATUS_OK; + merge_iterator *merge_itor; if (!out_itor || !itor_arr || !cfg || num_trees < 0 || num_trees > ARRAY_SIZE(merge_itor->ordered_iterator_stored)) diff --git a/src/splinterdb.c b/src/splinterdb.c index a653540a9..b2da31b97 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -1182,11 +1182,11 @@ splinterdb_iterator_init_full( slice user_min_key = options->lower_limit.key; slice user_max_key = options->upper_limit.key; slice user_start_key = options->start_position.key; - splinterdb_key_predicate predicate = options->predicate; - void *predicate_context = options->predicate_context; - slice routing_filter_key = options->routing_filter_key; - comparison min_key_comparison = greater_than_or_equal; - comparison max_key_comparison = less_than; + splinterdb_key_predicate predicate = options->predicate; + void *predicate_context = options->predicate_context; + slice routing_filter_key = options->routing_filter_key; + comparison min_key_comparison = greater_than_or_equal; + comparison max_key_comparison = less_than; comparison start_key_comparison = greater_than_or_equal; if (options->lower_limit.present) { min_key_comparison = options->lower_limit.cmp; diff --git a/tests/unit/splinterdb_quick_test.c b/tests/unit/splinterdb_quick_test.c index 6188b0061..35a014ccc 100644 --- a/tests/unit/splinterdb_quick_test.c +++ b/tests/unit/splinterdb_quick_test.c @@ -1432,8 +1432,8 @@ prefix_test_init_failure(core_handle *core, platform_memory_fault_config_set(&fault); platform_memory_fault_reset_counters(); platform_memory_fault_enable(); - btree_iterator itor; - platform_status rc = btree_iterator_init( + btree_iterator itor; + platform_status rc = btree_iterator_init( core->cc, core->cfg.btree_cfg, &itor, @@ -1441,7 +1441,7 @@ prefix_test_init_failure(core_handle *core, branch->format_version, PAGE_TYPE_BRANCH, &(const btree_iterator_init_options){ - .start_position = key_start_at_or_after(start_key)}); + .start_position = key_start_at_or_after(start_key)}); platform_memory_fault_counters counters = platform_memory_fault_get_counters(); platform_memory_fault_disable(); From 02be24a4cc303921a50c5333a34e64e60801e505 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Sun, 25 Oct 2026 19:25:29 -0700 Subject: [PATCH 09/10] core_range_iterator_init: treat a zero routing_filter_key as unset A zero-initialized key has kind 0, which is not NULL_KEY, so every iterator created without a filter was rejected with STATUS_BAD_PARAM. Co-Authored-By: Claude Sonnet 5.5 --- src/core.c | 5 ++++- src/core.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core.c b/src/core.c index 4de81030c..d2d85d5a8 100644 --- a/src/core.c +++ b/src/core.c @@ -2107,7 +2107,10 @@ core_range_iterator_init(core_handle *spl, key start_key = range.start_key; splinterdb_key_predicate predicate = options->predicate; void *predicate_context = options->predicate_context; - key routing_filter_key = options->routing_filter_key; + // A zero-initialized key (kind 0) is not NULL_KEY; treat it as unset. + key routing_filter_key = options->routing_filter_key.kind == 0 + ? NULL_KEY + : options->routing_filter_key; range_itor->filter = NULL; platform_status rc = STATUS_OK; diff --git a/src/core.h b/src/core.h index a4f181478..3704f96f5 100644 --- a/src/core.h +++ b/src/core.h @@ -381,7 +381,7 @@ typedef struct core_range_iterator_init_options { // splinterdb_iterator_init_options. splinterdb_key_predicate predicate; void *predicate_context; - key routing_filter_key; // default: NULL_KEY + key routing_filter_key; // default: unset (zero) } core_range_iterator_init_options; platform_status From 0f3d8125d4b8b9cb855aed3d1b7fe61ad753c720 Mon Sep 17 00:00:00 2001 From: Rob Johnson Date: Sun, 25 Oct 2026 20:03:56 -0700 Subject: [PATCH 10/10] Make USER_KEY zero so a zero key is NULL_KEY; drop limit "present" flags Renumber key_type to NEGATIVE_INFINITY = -1, USER_KEY = 0, POSITIVE_INFINITY = 1, so a zero-initialized key is exactly NULL_KEY and there is one representation of "no key". The order is preserved for data_key_compare's kind subtraction; static asserts guard both properties. key_type values are never persisted. With that, limits no longer need a "present" flag: a null key means unbounded and the comparison is ignored. Start positions become a direction plus an exclusive flag instead of a comparison, so their zero value is the default forward, inclusive scan. Public limits are validated only when they have a key. Removes the kind == 0 workaround in core_range_iterator_init. Co-Authored-By: Claude Opus 5.5 --- include/splinterdb/splinterdb.h | 50 +++++++++++++------------ src/core.c | 5 +-- src/core.h | 2 +- src/data_internal.h | 15 ++++++-- src/iterator.h | 66 ++++++++++++++++++++------------- src/splinterdb.c | 61 ++++++++++++------------------ 6 files changed, 105 insertions(+), 94 deletions(-) diff --git a/include/splinterdb/splinterdb.h b/include/splinterdb/splinterdb.h index ec2efa725..e058cdec7 100644 --- a/include/splinterdb/splinterdb.h +++ b/include/splinterdb/splinterdb.h @@ -616,66 +616,64 @@ typedef bool (*splinterdb_key_predicate)(void *context, slice key); // A limit on the range an iterator returns. Build these with the helpers // below (splinterdb_greater_than, splinterdb_less_than_or_equal, ...) rather -// than by hand. The zero value means "no limit". A limit whose key is -// NULL_SLICE also means "no limit". +// than by hand. The zero value, or any limit whose key is NULL_SLICE, means +// "no limit"; the comparison is then ignored. typedef struct splinterdb_lower_limit { - bool present; comparison cmp; // greater_than or greater_than_or_equal slice key; } splinterdb_lower_limit; typedef struct splinterdb_upper_limit { - bool present; comparison cmp; // less_than or less_than_or_equal slice key; } splinterdb_upper_limit; -// Where an iterator starts, and therefore which way it initially travels. The -// zero value means "at or after the smallest key", i.e. a forward scan of the -// whole range. +// Where an iterator starts, and therefore which way it initially travels. +// Build these with the helpers below. The zero value means "at or after the +// smallest key", i.e. a forward scan of the whole range. typedef struct splinterdb_start_position { - bool present; - comparison cmp; - slice key; + slice key; + bool backward; // scan toward smaller keys + bool exclusive; // skip a key equal to key } splinterdb_start_position; // Lower limits: only return keys > key, or >= key. static inline splinterdb_lower_limit splinterdb_greater_than(slice key) { - return (splinterdb_lower_limit){true, greater_than, key}; + return (splinterdb_lower_limit){.cmp = greater_than, .key = key}; } static inline splinterdb_lower_limit splinterdb_greater_than_or_equal(slice key) { - return (splinterdb_lower_limit){true, greater_than_or_equal, key}; + return (splinterdb_lower_limit){.cmp = greater_than_or_equal, .key = key}; } // Generic form, for when the comparison is only known at run time. Passing a -// comparison other than greater_than/greater_than_or_equal makes -// splinterdb_iterator_init fail with EINVAL. +// comparison other than greater_than/greater_than_or_equal with a non-null key +// makes splinterdb_iterator_init fail with EINVAL. static inline splinterdb_lower_limit splinterdb_make_lower_limit(comparison cmp, slice key) { - return (splinterdb_lower_limit){true, cmp, key}; + return (splinterdb_lower_limit){.cmp = cmp, .key = key}; } // Upper limits: only return keys < key, or <= key. static inline splinterdb_upper_limit splinterdb_less_than(slice key) { - return (splinterdb_upper_limit){true, less_than, key}; + return (splinterdb_upper_limit){.cmp = less_than, .key = key}; } static inline splinterdb_upper_limit splinterdb_less_than_or_equal(slice key) { - return (splinterdb_upper_limit){true, less_than_or_equal, key}; + return (splinterdb_upper_limit){.cmp = less_than_or_equal, .key = key}; } // Generic form. Passing a comparison other than less_than/less_than_or_equal -// makes splinterdb_iterator_init fail with EINVAL. +// with a non-null key makes splinterdb_iterator_init fail with EINVAL. static inline splinterdb_upper_limit splinterdb_make_upper_limit(comparison cmp, slice key) { - return (splinterdb_upper_limit){true, cmp, key}; + return (splinterdb_upper_limit){.cmp = cmp, .key = key}; } // Start positions. The iterator starts at the closest key satisfying the @@ -686,28 +684,32 @@ splinterdb_make_upper_limit(comparison cmp, slice key) static inline splinterdb_start_position splinterdb_start_at_or_after(slice key) { - return (splinterdb_start_position){true, greater_than_or_equal, key}; + return (splinterdb_start_position){.key = key}; } static inline splinterdb_start_position splinterdb_start_after(slice key) { - return (splinterdb_start_position){true, greater_than, key}; + return (splinterdb_start_position){.key = key, .exclusive = true}; } static inline splinterdb_start_position splinterdb_start_at_or_before(slice key) { - return (splinterdb_start_position){true, less_than_or_equal, key}; + return (splinterdb_start_position){.key = key, .backward = true}; } static inline splinterdb_start_position splinterdb_start_before(slice key) { - return (splinterdb_start_position){true, less_than, key}; + return (splinterdb_start_position){ + .key = key, .backward = true, .exclusive = true}; } // Generic form, for when the comparison is only known at run time. static inline splinterdb_start_position splinterdb_make_start_position(comparison cmp, slice key) { - return (splinterdb_start_position){true, cmp, key}; + return (splinterdb_start_position){ + .key = key, + .backward = cmp == less_than || cmp == less_than_or_equal, + .exclusive = cmp == less_than || cmp == greater_than}; } // Optional parameters for splinterdb_iterator_init. A zero-initialized options diff --git a/src/core.c b/src/core.c index d2d85d5a8..4de81030c 100644 --- a/src/core.c +++ b/src/core.c @@ -2107,10 +2107,7 @@ core_range_iterator_init(core_handle *spl, key start_key = range.start_key; splinterdb_key_predicate predicate = options->predicate; void *predicate_context = options->predicate_context; - // A zero-initialized key (kind 0) is not NULL_KEY; treat it as unset. - key routing_filter_key = options->routing_filter_key.kind == 0 - ? NULL_KEY - : options->routing_filter_key; + key routing_filter_key = options->routing_filter_key; range_itor->filter = NULL; platform_status rc = STATUS_OK; diff --git a/src/core.h b/src/core.h index 3704f96f5..a4f181478 100644 --- a/src/core.h +++ b/src/core.h @@ -381,7 +381,7 @@ typedef struct core_range_iterator_init_options { // splinterdb_iterator_init_options. splinterdb_key_predicate predicate; void *predicate_context; - key routing_filter_key; // default: unset (zero) + key routing_filter_key; // default: NULL_KEY } core_range_iterator_init_options; platform_status diff --git a/src/data_internal.h b/src/data_internal.h index 925e2bcd2..62b8a8789 100644 --- a/src/data_internal.h +++ b/src/data_internal.h @@ -29,10 +29,16 @@ * does not require them to deal with infinite keys. */ +/* + * USER_KEY must be 0, so that a zero-initialized key is NULL_KEY (a user key + * with a NULL_SLICE); there is exactly one representation of "no key". + * data_key_compare() orders keys of different kinds by subtracting their + * kinds, so the values must also stay in -inf < user < +inf order. + */ typedef enum { - NEGATIVE_INFINITY = 1, - USER_KEY = 2, - POSITIVE_INFINITY = 3, + NEGATIVE_INFINITY = -1, + USER_KEY = 0, + POSITIVE_INFINITY = 1, } key_type; typedef struct key { @@ -51,6 +57,9 @@ typedef struct key { .user_slice = INVALID_SLICE}) #define NULL_KEY \ ((key){.kind = USER_KEY, .is_query_key = FALSE, .user_slice = NULL_SLICE}) +_Static_assert(USER_KEY == 0, "a zero-initialized key must be NULL_KEY"); +_Static_assert(NEGATIVE_INFINITY < USER_KEY && USER_KEY < POSITIVE_INFINITY, + "data_key_compare orders kinds by subtraction"); static inline bool32 key_is_negative_infinity(key k) diff --git a/src/iterator.h b/src/iterator.h index ea32f50ec..1b91ffeb2 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -68,84 +68,100 @@ comparison_invert(comparison c) * Range specification for iterator constructors. These mirror the public * splinterdb_lower_limit / splinterdb_upper_limit / splinterdb_start_position * types, but hold internal keys, so they can name infinities. Constructors - * take them in an options struct filled in with designated initializers; the - * zero value of each means "unbounded" / "start at the smallest key". A limit - * whose key is NULL_KEY is also unbounded. + * take them in an options struct filled in with designated initializers. The + * zero value of each is the default: a limit whose key is NULL_KEY is + * unbounded (and its comparison is ignored), and the zero start position is + * "at or after the smallest key". */ typedef struct key_lower_limit { - bool present; comparison cmp; // greater_than or greater_than_or_equal key k; } key_lower_limit; typedef struct key_upper_limit { - bool present; comparison cmp; // less_than or less_than_or_equal key k; } key_upper_limit; +/* + * The start is a direction and an inclusivity rather than a comparison, so + * that the zero value (forward, inclusive) is the default. + */ typedef struct key_start_position { - bool present; - comparison cmp; - key k; + key k; + bool32 backward; // scan toward smaller keys + bool32 exclusive; // skip a key equal to k } key_start_position; static inline key_lower_limit key_greater_than(key k) { - return (key_lower_limit){TRUE, greater_than, k}; + return (key_lower_limit){.cmp = greater_than, .k = k}; } static inline key_lower_limit key_greater_than_or_equal(key k) { - return (key_lower_limit){TRUE, greater_than_or_equal, k}; + return (key_lower_limit){.cmp = greater_than_or_equal, .k = k}; } static inline key_lower_limit make_key_lower_limit(comparison cmp, key k) { - return (key_lower_limit){TRUE, cmp, k}; + return (key_lower_limit){.cmp = cmp, .k = k}; } static inline key_upper_limit key_less_than(key k) { - return (key_upper_limit){TRUE, less_than, k}; + return (key_upper_limit){.cmp = less_than, .k = k}; } static inline key_upper_limit key_less_than_or_equal(key k) { - return (key_upper_limit){TRUE, less_than_or_equal, k}; + return (key_upper_limit){.cmp = less_than_or_equal, .k = k}; } static inline key_upper_limit make_key_upper_limit(comparison cmp, key k) { - return (key_upper_limit){TRUE, cmp, k}; + return (key_upper_limit){.cmp = cmp, .k = k}; } static inline key_start_position key_start_at_or_after(key k) { - return (key_start_position){TRUE, greater_than_or_equal, k}; + return (key_start_position){.k = k}; } static inline key_start_position key_start_after(key k) { - return (key_start_position){TRUE, greater_than, k}; + return (key_start_position){.k = k, .exclusive = TRUE}; } static inline key_start_position key_start_at_or_before(key k) { - return (key_start_position){TRUE, less_than_or_equal, k}; + return (key_start_position){.k = k, .backward = TRUE}; } static inline key_start_position key_start_before(key k) { - return (key_start_position){TRUE, less_than, k}; + return (key_start_position){.k = k, .backward = TRUE, .exclusive = TRUE}; } static inline key_start_position make_key_start_position(comparison cmp, key k) { - return (key_start_position){TRUE, cmp, k}; + return (key_start_position){.k = k, + .backward = !comparison_is_forward(cmp), + .exclusive = + cmp == greater_than || cmp == less_than}; +} + +static inline comparison +key_start_position_comparison(key_start_position start) +{ + if (start.backward) { + return start.exclusive ? less_than : less_than_or_equal; + } else { + return start.exclusive ? greater_than : greater_than_or_equal; + } } // A range specification with every default filled in: infinite keys for @@ -165,18 +181,18 @@ resolve_key_range(key_lower_limit lower, key_start_position start) { resolved_key_range r; - bool32 lower_set = lower.present && !key_is_null(lower.k); - bool32 upper_set = upper.present && !key_is_null(upper.k); + bool32 lower_set = !key_is_null(lower.k); + bool32 upper_set = !key_is_null(upper.k); r.min_cmp = lower_set ? lower.cmp : greater_than_or_equal; r.min_key = lower_set ? lower.k : NEGATIVE_INFINITY_KEY; r.max_cmp = upper_set ? upper.cmp : less_than; r.max_key = upper_set ? upper.k : POSITIVE_INFINITY_KEY; - r.start_cmp = start.present ? start.cmp : greater_than_or_equal; - if (start.present && !key_is_null(start.k)) { + r.start_cmp = key_start_position_comparison(start); + if (!key_is_null(start.k)) { r.start_key = start.k; } else { - r.start_key = comparison_is_forward(r.start_cmp) ? NEGATIVE_INFINITY_KEY - : POSITIVE_INFINITY_KEY; + r.start_key = + start.backward ? POSITIVE_INFINITY_KEY : NEGATIVE_INFINITY_KEY; } return r; } diff --git a/src/splinterdb.c b/src/splinterdb.c index b2da31b97..bf5008e42 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -1179,28 +1179,21 @@ splinterdb_iterator_init_full( { return platform_status_to_int(STATUS_BAD_PARAM); } - slice user_min_key = options->lower_limit.key; - slice user_max_key = options->upper_limit.key; - slice user_start_key = options->start_position.key; - splinterdb_key_predicate predicate = options->predicate; - void *predicate_context = options->predicate_context; - slice routing_filter_key = options->routing_filter_key; - comparison min_key_comparison = greater_than_or_equal; - comparison max_key_comparison = less_than; - comparison start_key_comparison = greater_than_or_equal; - if (options->lower_limit.present) { - min_key_comparison = options->lower_limit.cmp; - } - if (options->upper_limit.present) { - max_key_comparison = options->upper_limit.cmp; - } - if (options->start_position.present) { - start_key_comparison = options->start_position.cmp; - } - + slice user_min_key = options->lower_limit.key; + slice user_max_key = options->upper_limit.key; + slice user_start_key = options->start_position.key; + splinterdb_key_predicate predicate = options->predicate; + void *predicate_context = options->predicate_context; + slice routing_filter_key = options->routing_filter_key; + + // A limit's comparison only matters when it has a key. if ((predicate == NULL && !slice_is_null(routing_filter_key)) - || !splinterdb_iterator_min_comparison_is_valid(min_key_comparison) - || !splinterdb_iterator_max_comparison_is_valid(max_key_comparison)) + || (!slice_is_null(user_min_key) + && !splinterdb_iterator_min_comparison_is_valid( + options->lower_limit.cmp)) + || (!slice_is_null(user_max_key) + && !splinterdb_iterator_max_comparison_is_valid( + options->upper_limit.cmp))) { return platform_status_to_int(STATUS_BAD_PARAM); } @@ -1221,26 +1214,20 @@ splinterdb_iterator_init_full( merge_accumulator_init(&it->materialized_message, kvs->spl.heap_id); core_range_iterator *range_itor = &(it->sri); - // Internal keys for the user-supplied slices; NULL_SLICE bounds become - // NULL_KEY, which the core treats as unbounded. - key min_key = slice_is_null(user_min_key) - ? NULL_KEY - : key_create_from_slice(TRUE, user_min_key); - key max_key = slice_is_null(user_max_key) - ? NULL_KEY - : key_create_from_slice(TRUE, user_max_key); - key start_key = slice_is_null(user_start_key) - ? NULL_KEY - : key_create_from_slice(TRUE, user_start_key); - + // NULL_SLICE becomes NULL_KEY, which the core treats as "no key". platform_status rc = core_range_iterator_init( &kvs->spl, range_itor, &(const core_range_iterator_init_options){ - .lower_limit = make_key_lower_limit(min_key_comparison, min_key), - .upper_limit = make_key_upper_limit(max_key_comparison, max_key), - .start_position = - make_key_start_position(start_key_comparison, start_key), + .lower_limit = + make_key_lower_limit(options->lower_limit.cmp, + key_create_from_slice(TRUE, user_min_key)), + .upper_limit = + make_key_upper_limit(options->upper_limit.cmp, + key_create_from_slice(TRUE, user_max_key)), + .start_position = {.k = key_create_from_slice(TRUE, user_start_key), + .backward = options->start_position.backward, + .exclusive = options->start_position.exclusive}, .predicate = predicate, .predicate_context = predicate_context, .routing_filter_key =