Skip to content

Commit d007bed

Browse files
QuantumExplorerQuantumExplorer
authored
fix: limit will now decrease when subquery has no elements (#277)
1 parent b79c463 commit d007bed

File tree

7 files changed

+171
-82
lines changed

7 files changed

+171
-82
lines changed

‎grovedb/src/element/query.rs

Lines changed: 79 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,20 @@ use crate::{
5959
#[cfg(any(feature = "full", feature = "verify"))]
6060
use crate::{Element, SizedQuery};
6161

62+
#[cfg(feature = "full")]
63+
#[derive(Copy, Clone, Debug)]
64+
pub struct QueryOptions {
65+
pub allow_get_raw: bool,
66+
pub allow_cache: bool,
67+
/// Should we decrease the limit of elements found when we have no
68+
/// subelements in the subquery? This should generally be set to true,
69+
/// as having it false could mean very expensive queries. The queries
70+
/// would be expensive because we could go through many many trees where the
71+
/// sub elements have no matches, hence the limit would not decrease and
72+
/// hence we would continue on the increasingly expensive query.
73+
pub decrease_limit_on_range_with_no_sub_elements: bool,
74+
}
75+
6276
#[cfg(feature = "full")]
6377
/// Path query push arguments
6478
pub struct PathQueryPushArgs<'db, 'ctx, 'a>
@@ -73,8 +87,7 @@ where
7387
pub subquery_path: Option<Path>,
7488
pub subquery: Option<Query>,
7589
pub left_to_right: bool,
76-
pub allow_get_raw: bool,
77-
pub allow_cache: bool,
90+
pub query_options: QueryOptions,
7891
pub result_type: QueryResultType,
7992
pub results: &'a mut Vec<QueryResultElement>,
8093
pub limit: &'a mut Option<u16>,
@@ -97,6 +110,7 @@ impl Element {
97110
merk_path,
98111
&sized_query,
99112
true,
113+
true,
100114
result_type,
101115
transaction,
102116
)
@@ -139,8 +153,7 @@ impl Element {
139153
storage: &RocksDbStorage,
140154
path: &[&[u8]],
141155
sized_query: &SizedQuery,
142-
allow_get_raw: bool,
143-
allow_cache: bool,
156+
query_options: QueryOptions,
144157
result_type: QueryResultType,
145158
transaction: TransactionArg,
146159
add_element_function: fn(PathQueryPushArgs) -> CostResult<(), Error>,
@@ -166,8 +179,7 @@ impl Element {
166179
transaction,
167180
&mut limit,
168181
&mut offset,
169-
allow_get_raw,
170-
allow_cache,
182+
query_options,
171183
result_type,
172184
add_element_function,
173185
)
@@ -189,8 +201,7 @@ impl Element {
189201
transaction,
190202
&mut limit,
191203
&mut offset,
192-
allow_get_raw,
193-
allow_cache,
204+
query_options,
194205
result_type,
195206
add_element_function,
196207
)
@@ -216,6 +227,7 @@ impl Element {
216227
storage: &RocksDbStorage,
217228
path_query: &PathQuery,
218229
allow_cache: bool,
230+
decrease_limit_on_range_with_no_sub_elements: bool,
219231
result_type: QueryResultType,
220232
transaction: TransactionArg,
221233
) -> CostResult<(QueryResultElements, u16), Error> {
@@ -228,8 +240,11 @@ impl Element {
228240
storage,
229241
path_slices.as_slice(),
230242
&path_query.query,
231-
false,
232-
allow_cache,
243+
QueryOptions {
244+
allow_get_raw: false,
245+
allow_cache,
246+
decrease_limit_on_range_with_no_sub_elements,
247+
},
233248
result_type,
234249
transaction,
235250
Element::path_query_push,
@@ -243,6 +258,7 @@ impl Element {
243258
storage: &RocksDbStorage,
244259
path_query: &PathQuery,
245260
allow_cache: bool,
261+
decrease_limit_on_range_with_no_sub_elements: bool,
246262
result_type: QueryResultType,
247263
transaction: TransactionArg,
248264
) -> CostResult<(QueryResultElements, u16), Error> {
@@ -255,8 +271,11 @@ impl Element {
255271
storage,
256272
path_slices.as_slice(),
257273
&path_query.query,
258-
true,
259-
allow_cache,
274+
QueryOptions {
275+
allow_get_raw: true,
276+
allow_cache,
277+
decrease_limit_on_range_with_no_sub_elements,
278+
},
260279
result_type,
261280
transaction,
262281
Element::path_query_push,
@@ -270,15 +289,19 @@ impl Element {
270289
path: &[&[u8]],
271290
sized_query: &SizedQuery,
272291
allow_cache: bool,
292+
decrease_limit_on_range_with_no_sub_elements: bool,
273293
result_type: QueryResultType,
274294
transaction: TransactionArg,
275295
) -> CostResult<(QueryResultElements, u16), Error> {
276296
Element::get_query_apply_function(
277297
storage,
278298
path,
279299
sized_query,
280-
false,
281-
allow_cache,
300+
QueryOptions {
301+
allow_get_raw: false,
302+
allow_cache,
303+
decrease_limit_on_range_with_no_sub_elements,
304+
},
282305
result_type,
283306
transaction,
284307
Element::path_query_push,
@@ -299,13 +322,17 @@ impl Element {
299322
subquery_path,
300323
subquery,
301324
left_to_right,
302-
allow_get_raw,
303-
allow_cache,
325+
query_options,
304326
result_type,
305327
results,
306328
limit,
307329
offset,
308330
} = args;
331+
let QueryOptions {
332+
allow_get_raw,
333+
allow_cache,
334+
decrease_limit_on_range_with_no_sub_elements,
335+
} = query_options;
309336
if element.is_tree() {
310337
let mut path_vec = path.to_vec();
311338
let key = cost_return_on_error_no_add!(
@@ -331,13 +358,19 @@ impl Element {
331358
storage,
332359
&inner_path_query,
333360
allow_cache,
361+
decrease_limit_on_range_with_no_sub_elements,
334362
result_type,
335363
transaction
336364
)
337365
);
338366

339367
if let Some(limit) = limit {
340-
*limit = limit.saturating_sub(sub_elements.len() as u16);
368+
if sub_elements.is_empty() && decrease_limit_on_range_with_no_sub_elements {
369+
// we should decrease by 1 in this case
370+
*limit = limit.saturating_sub(1);
371+
} else {
372+
*limit = limit.saturating_sub(sub_elements.len() as u16);
373+
}
341374
}
342375
if let Some(offset) = offset {
343376
*offset = offset.saturating_sub(skipped);
@@ -455,8 +488,7 @@ impl Element {
455488
subquery_path,
456489
subquery,
457490
left_to_right,
458-
allow_get_raw,
459-
allow_cache,
491+
query_options,
460492
result_type,
461493
results,
462494
limit,
@@ -483,8 +515,7 @@ impl Element {
483515
subquery_path,
484516
subquery,
485517
left_to_right,
486-
allow_get_raw,
487-
allow_cache,
518+
query_options,
488519
result_type,
489520
results,
490521
limit,
@@ -530,6 +561,12 @@ impl Element {
530561
(subquery_path, subquery)
531562
}
532563

564+
/// `decrease_limit_on_range_with_no_sub_elements` should generally be set
565+
/// to true, as having it false could mean very expensive queries.
566+
/// The queries would be expensive because we could go through many many
567+
/// trees where the sub elements have no matches, hence the limit would
568+
/// not decrease and hence we would continue on the increasingly
569+
/// expensive query.
533570
#[cfg(feature = "full")]
534571
// TODO: refactor
535572
fn query_item(
@@ -541,8 +578,7 @@ impl Element {
541578
transaction: TransactionArg,
542579
limit: &mut Option<u16>,
543580
offset: &mut Option<u16>,
544-
allow_get_raw: bool,
545-
allow_cache: bool,
581+
query_options: QueryOptions,
546582
result_type: QueryResultType,
547583
add_element_function: fn(PathQueryPushArgs) -> CostResult<(), Error>,
548584
) -> CostResult<(), Error> {
@@ -560,7 +596,10 @@ impl Element {
560596
None,
561597
transaction,
562598
subtree,
563-
{ Element::get(&subtree, key, allow_cache).unwrap_add_cost(&mut cost) }
599+
{
600+
Element::get(&subtree, key, query_options.allow_cache)
601+
.unwrap_add_cost(&mut cost)
602+
}
564603
);
565604
match element_res {
566605
Ok(element) => {
@@ -575,8 +614,7 @@ impl Element {
575614
subquery_path,
576615
subquery,
577616
left_to_right: sized_query.query.left_to_right,
578-
allow_get_raw,
579-
allow_cache,
617+
query_options,
580618
result_type,
581619
results,
582620
limit,
@@ -630,8 +668,7 @@ impl Element {
630668
subquery_path,
631669
subquery,
632670
left_to_right: sized_query.query.left_to_right,
633-
allow_get_raw,
634-
allow_cache,
671+
query_options,
635672
result_type,
636673
results,
637674
limit,
@@ -939,6 +976,7 @@ mod tests {
939976
&[TEST_LEAF],
940977
&ascending_query,
941978
true,
979+
true,
942980
QueryKeyElementPairResultType,
943981
None,
944982
)
@@ -973,6 +1011,7 @@ mod tests {
9731011
&[TEST_LEAF],
9741012
&backwards_query,
9751013
true,
1014+
true,
9761015
QueryKeyElementPairResultType,
9771016
None,
9781017
)
@@ -1062,6 +1101,7 @@ mod tests {
10621101
&[TEST_LEAF],
10631102
&ascending_query,
10641103
true,
1104+
true,
10651105
QueryKeyElementPairResultType,
10661106
None,
10671107
)
@@ -1079,6 +1119,7 @@ mod tests {
10791119
&[TEST_LEAF],
10801120
&backwards_query,
10811121
true,
1122+
true,
10821123
QueryKeyElementPairResultType,
10831124
None,
10841125
)
@@ -1099,6 +1140,7 @@ mod tests {
10991140
&[TEST_LEAF],
11001141
&backwards_query,
11011142
true,
1143+
true,
11021144
QueryKeyElementPairResultType,
11031145
None,
11041146
)
@@ -1161,6 +1203,7 @@ mod tests {
11611203
&[TEST_LEAF],
11621204
&backwards_query,
11631205
true,
1206+
true,
11641207
QueryKeyElementPairResultType,
11651208
None,
11661209
)
@@ -1187,6 +1230,7 @@ mod tests {
11871230
&[TEST_LEAF],
11881231
&backwards_query,
11891232
true,
1233+
true,
11901234
QueryKeyElementPairResultType,
11911235
None,
11921236
)
@@ -1208,6 +1252,7 @@ mod tests {
12081252
&[TEST_LEAF],
12091253
&limit_query,
12101254
true,
1255+
true,
12111256
QueryKeyElementPairResultType,
12121257
None,
12131258
)
@@ -1229,6 +1274,7 @@ mod tests {
12291274
&[TEST_LEAF],
12301275
&limit_query,
12311276
true,
1277+
true,
12321278
QueryKeyElementPairResultType,
12331279
None,
12341280
)
@@ -1249,6 +1295,7 @@ mod tests {
12491295
&[TEST_LEAF],
12501296
&limit_offset_query,
12511297
true,
1298+
true,
12521299
QueryKeyElementPairResultType,
12531300
None,
12541301
)
@@ -1274,6 +1321,7 @@ mod tests {
12741321
&[TEST_LEAF],
12751322
&limit_offset_backwards_query,
12761323
true,
1324+
true,
12771325
QueryKeyElementPairResultType,
12781326
None,
12791327
)
@@ -1298,6 +1346,7 @@ mod tests {
12981346
&[TEST_LEAF],
12991347
&limit_full_query,
13001348
true,
1349+
true,
13011350
QueryKeyElementPairResultType,
13021351
None,
13031352
)
@@ -1323,6 +1372,7 @@ mod tests {
13231372
&[TEST_LEAF],
13241373
&limit_offset_backwards_query,
13251374
true,
1375+
true,
13261376
QueryKeyElementPairResultType,
13271377
None,
13281378
)
@@ -1348,6 +1398,7 @@ mod tests {
13481398
&[TEST_LEAF],
13491399
&limit_backwards_query,
13501400
true,
1401+
true,
13511402
QueryKeyElementPairResultType,
13521403
None,
13531404
)

0 commit comments

Comments
 (0)