Skip to content

Commit d411dd3

Browse files
fix(drive): unordered composite lookups inherit the page's direction
A documents sub-query the caller left unordered on its bound field was appended ascending, which the direction rule then refused under a descending page: every feed page (`$createdAt desc`) with default lookups failed with "outer ordering must match the page's direction". The appended clause now takes the page's direction, so a minimal request never conflicts with its page; an explicit ordering that disagrees is still refused, on every entry point. Also documents a lookup's limit for what it is: a cap on the rows the lookup returns in total, in walk order, like an ordinary `IN` query's, not a per-value bound. Test: a descending by-ids page with unordered cross-contract profiles, the viewer's likes and a count merges, proves and verifies with the lookups walking descending; a limited same-contract lookup does too, returning its capped rows from the top of the walk; an explicit ascending ordering under the descending page is refused by all three entry points. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent d6ccf87 commit d411dd3

2 files changed

Lines changed: 189 additions & 6 deletions

File tree

packages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/composite_query_e2e_tests.rs

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,3 +1195,165 @@ fn should_check_count_and_document_descents_against_the_actual_bound_values() {
11951195
);
11961196
assert_eq!(verified.sub_results, materialized.sub_results);
11971197
}
1198+
1199+
/// A minimal request never conflicts with the page's direction: a
1200+
/// documents sub-query the caller left unordered on its bound field walks
1201+
/// the page's way, so a descending page with default lookups merges and
1202+
/// verifies, while an explicit ordering that disagrees is still refused.
1203+
#[test]
1204+
fn should_inherit_the_page_direction_for_unordered_lookups() {
1205+
let (drive, feed, dashpay) = setup();
1206+
seed_feed(&drive, &feed, &dashpay);
1207+
let pv = platform_version();
1208+
let descending_page = || {
1209+
let mut page = page_by_hashtag(&feed, "dash", Some(3));
1210+
page.internal_clauses = InternalClauses::extract_from_clauses(
1211+
vec![WhereClause {
1212+
field: "$id".into(),
1213+
operator: WhereOperator::In,
1214+
value: Value::Array(vec![
1215+
Value::Identifier(POST_A),
1216+
Value::Identifier(POST_B),
1217+
Value::Identifier(POST_C),
1218+
]),
1219+
}],
1220+
pv,
1221+
)
1222+
.expect("by-id page");
1223+
page.order_by.insert(
1224+
"$id".into(),
1225+
OrderClause {
1226+
field: "$id".into(),
1227+
ascending: false,
1228+
},
1229+
);
1230+
page
1231+
};
1232+
let like_counts = || {
1233+
bound(
1234+
&feed,
1235+
"like",
1236+
SubQueryKind::Count,
1237+
BindingSource::Page,
1238+
"$id",
1239+
"postId",
1240+
None,
1241+
)
1242+
};
1243+
let round_trip = |query: &DriveCompositeDocumentQuery, what: &str| {
1244+
let materialized = drive
1245+
.query_composite_documents(query, None, None, pv)
1246+
.unwrap_or_else(|e| panic!("{what} materializes: {e}"))
1247+
.result;
1248+
assert_eq!(
1249+
ids(&materialized.page_documents),
1250+
vec![POST_C, POST_B, POST_A]
1251+
);
1252+
let (proof, _) = drive
1253+
.query_composite_documents_with_proof(query, pv)
1254+
.unwrap_or_else(|e| panic!("{what} proves: {e}"));
1255+
let (_, verified) = query
1256+
.verify_composite_documents_proof(&proof, pv)
1257+
.unwrap_or_else(|e| panic!("{what} verifies: {e}"));
1258+
assert_eq!(verified.page_documents, materialized.page_documents);
1259+
assert_eq!(verified.sub_results, materialized.sub_results);
1260+
materialized
1261+
};
1262+
1263+
// The feed shape: cross-contract profiles, the viewer's marks (both
1264+
// value-bounded) and a count, none of them ordered by the caller.
1265+
let mut viewer_likes = bound(
1266+
&feed,
1267+
"like",
1268+
SubQueryKind::Documents,
1269+
BindingSource::Page,
1270+
"$id",
1271+
"postId",
1272+
None,
1273+
);
1274+
viewer_likes.where_clauses = vec![WhereClause {
1275+
field: "$ownerId".into(),
1276+
operator: WhereOperator::Equal,
1277+
value: Value::Identifier(OWNER_1),
1278+
}];
1279+
let feed_shape = DriveCompositeDocumentQuery {
1280+
page: descending_page(),
1281+
sub_queries: vec![
1282+
bound(
1283+
&dashpay,
1284+
"profile",
1285+
SubQueryKind::Documents,
1286+
BindingSource::Page,
1287+
"$ownerId",
1288+
"$ownerId",
1289+
None,
1290+
),
1291+
viewer_likes,
1292+
like_counts(),
1293+
],
1294+
};
1295+
let result = round_trip(&feed_shape, "the descending feed shape");
1296+
// The lookups inherited the page's direction: descending by their
1297+
// bound field.
1298+
assert_eq!(
1299+
owner_ids(result.sub_results[0].documents()),
1300+
vec![OWNER_3, OWNER_1],
1301+
"profiles walk owners descending"
1302+
);
1303+
assert_eq!(
1304+
post_ids_of(&result.sub_results[1], "postId"),
1305+
vec![POST_B, POST_A],
1306+
"the viewer's likes, posts descending"
1307+
);
1308+
assert_eq!(
1309+
counts(&result.sub_results[2]),
1310+
BTreeMap::from([(POST_A, 2), (POST_B, 1)])
1311+
);
1312+
1313+
// A limited lookup under the page's own contract. Its limit caps the
1314+
// rows it returns in total, in walk order, like an ordinary `IN`
1315+
// query's: walking posts descending, the one row is B's.
1316+
let limited_lookup = DriveCompositeDocumentQuery {
1317+
page: descending_page(),
1318+
sub_queries: vec![
1319+
bound(
1320+
&feed,
1321+
"repost",
1322+
SubQueryKind::Documents,
1323+
BindingSource::Page,
1324+
"$id",
1325+
"postId",
1326+
Some(1),
1327+
),
1328+
like_counts(),
1329+
],
1330+
};
1331+
let result = round_trip(&limited_lookup, "the limited lookup");
1332+
assert_eq!(
1333+
post_ids_of(&result.sub_results[0], "postId"),
1334+
vec![POST_B],
1335+
"the single repost row comes from the highest post id"
1336+
);
1337+
1338+
// An explicit ordering that disagrees with the page is still refused,
1339+
// on every entry point.
1340+
let mut conflicting = limited_lookup.clone();
1341+
conflicting.sub_queries[0].order_by.push(OrderClause {
1342+
field: "postId".into(),
1343+
ascending: true,
1344+
});
1345+
for result in [
1346+
drive
1347+
.query_composite_documents(&conflicting, None, None, pv)
1348+
.map(|_| ()),
1349+
drive
1350+
.query_composite_documents_with_proof(&conflicting, pv)
1351+
.map(|_| ()),
1352+
conflicting
1353+
.verify_composite_documents_proof(&[], pv)
1354+
.map(|_| ()),
1355+
] {
1356+
assert!(matches!(result, Err(Error::Query(_))), "{result:?}");
1357+
assert!(result.unwrap_err().to_string().contains("outer ordering"));
1358+
}
1359+
}

packages/rs-drive/src/query/drive_composite_document_query/mod.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,18 @@ pub struct DriveSubQuery<'a> {
139139
/// The fixed clauses (everything but the derived `IN`), typed.
140140
/// Must be empty for a by-id join, which resolves every derived id.
141141
pub where_clauses: Vec<WhereClause>,
142-
/// Ordering; documents only. The outer query direction must agree
143-
/// with the page's direction so merging preserves the requested rows.
142+
/// Ordering; documents only. Every component of the merged proof
143+
/// walks in the page's direction, so a documents sub-query must agree
144+
/// with it: a bound field the caller did not order by is appended in
145+
/// the page's direction (a minimal request never conflicts), and an
146+
/// explicit ordering that disagrees is refused, because changing it
147+
/// for the proof would change the rows its limit selects.
144148
pub order_by: Vec<OrderClause>,
145-
/// Required for a documents lookup on a non-unique index (it bounds
146-
/// the walk under each value); forbidden for a value-bounded lookup,
147-
/// a by-id join (completeness is set-based) and a count.
149+
/// Required for a documents lookup on a non-unique index: it caps the
150+
/// rows the lookup returns in total, in walk order, exactly as the
151+
/// limit of an ordinary `IN` query does (at most `MAX_BOUND_VALUES`).
152+
/// Forbidden for a value-bounded lookup, a by-id join (completeness is
153+
/// set-based) and a count.
148154
pub limit: Option<u16>,
149155
/// The derived clause, or `None` for a sibling.
150156
pub binding: Option<SubQueryBinding>,
@@ -887,12 +893,17 @@ impl<'a> DriveCompositeDocumentQuery<'a> {
887893
// An `IN` on a secondary index orders by the bound field;
888894
// supply the ordering when the caller did not, so the
889895
// request stays minimal and both sides build the same query.
896+
// It inherits the page's direction: the merged proof walks
897+
// every component the page's way, and a documents sub-query
898+
// may not be turned around behind the caller's back (see
899+
// `sub_query_proof_path_query`), so this default is what
900+
// keeps an unordered lookup mergeable under a descending page.
890901
if !order_by.contains_key(&binding.field) {
891902
order_by.insert(
892903
binding.field.clone(),
893904
OrderClause {
894905
field: binding.field.clone(),
895-
ascending: true,
906+
ascending: self.page_direction(platform_version)?,
896907
},
897908
);
898909
}
@@ -974,6 +985,16 @@ impl<'a> DriveCompositeDocumentQuery<'a> {
974985
}
975986
}
976987

988+
/// The page's walk direction: what every component of the merged
989+
/// proof walks in, and what an unordered documents sub-query inherits.
990+
fn page_direction(&self, platform_version: &PlatformVersion) -> Result<bool, Error> {
991+
Ok(self
992+
.page_path_query(platform_version)?
993+
.query
994+
.query
995+
.left_to_right)
996+
}
997+
977998
/// Aligns set-based components for merging without changing a
978999
/// documents query's ordering or the rows selected by its limit.
9791000
/// Validation, proof generation and bootstrap use the same check,

0 commit comments

Comments
 (0)