From b169b2eb0a3065ff78a29108fd7cbb571e7be3f1 Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Tue, 23 Aug 2022 13:45:13 -0700 Subject: [PATCH 1/5] Allow descending order of chain query filter --- crates/holochain_state/src/source_chain.rs | 15 ++++++------- crates/holochain_zome_types/src/query.rs | 21 +++++++++++++++++++ .../test_utils/wasm/wasm_workspace/Cargo.lock | 8 +++---- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/crates/holochain_state/src/source_chain.rs b/crates/holochain_state/src/source_chain.rs index c222154bd9..a10a998a7d 100644 --- a/crates/holochain_state/src/source_chain.rs +++ b/crates/holochain_state/src/source_chain.rs @@ -764,14 +764,15 @@ where }); sql.push_str( " - ) - AND - (:entry_type IS NULL OR Action.entry_type = :entry_type) - AND - (:action_type IS NULL OR Action.type = :action_type) - ORDER BY Action.seq ASC - ", + ) + AND + (:entry_type IS NULL OR Action.entry_type = :entry_type) + AND + (:action_type IS NULL OR Action.type = :action_type) + ORDER BY Action.seq + ", ); + sql.push_str(if query.descending {" DESC"} else {" ASC"}); let mut stmt = txn.prepare(&sql)?; let records = stmt .query_and_then( diff --git a/crates/holochain_zome_types/src/query.rs b/crates/holochain_zome_types/src/query.rs index c410c2c654..be1fbe0e0a 100644 --- a/crates/holochain_zome_types/src/query.rs +++ b/crates/holochain_zome_types/src/query.rs @@ -78,6 +78,8 @@ pub struct ChainQueryFilter { pub action_type: Option, /// Include the entries in the records pub include_entries: bool, + /// The query should be ordered in descending order (default is ascending) + pub descending: bool, } #[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize, SerializedBytes)] @@ -207,6 +209,18 @@ impl ChainQueryFilter { self } + /// Set the order to ascending. + pub fn ascending(mut self) -> Self { + self.descending = false; + self + } + + /// Set the order to ascending. + pub fn descending(mut self) -> Self { + self.descending = true; + self + } + /// If the sequence range supports fork disambiguation, apply it to remove /// actions that are not in the correct branch. /// Numerical range bounds do NOT support fork disambiguation, and neither @@ -401,6 +415,13 @@ mod tests { map_query(&query_2, &actions), [false, true, false, true, false, false, false].to_vec() ); + assert_eq!( + map_query(&query_2.descending(), &actions), + [false, true, false, true, false, false, false] + .into_iter() + .rev() + .collect::>() + ); } #[test] diff --git a/crates/test_utils/wasm/wasm_workspace/Cargo.lock b/crates/test_utils/wasm/wasm_workspace/Cargo.lock index 89a60cf6f8..23df0031ce 100644 --- a/crates/test_utils/wasm/wasm_workspace/Cargo.lock +++ b/crates/test_utils/wasm/wasm_workspace/Cargo.lock @@ -684,7 +684,7 @@ dependencies = [ [[package]] name = "hdi" -version = "0.0.18" +version = "0.0.19" dependencies = [ "hdk_derive", "holo_hash", @@ -699,7 +699,7 @@ dependencies = [ [[package]] name = "hdk" -version = "0.0.146" +version = "0.0.147" dependencies = [ "getrandom", "hdi", @@ -718,7 +718,7 @@ dependencies = [ [[package]] name = "hdk_derive" -version = "0.0.44" +version = "0.0.45" dependencies = [ "darling 0.14.1", "heck 0.4.0", @@ -823,7 +823,7 @@ dependencies = [ [[package]] name = "holochain_test_wasm_common" -version = "0.0.47" +version = "0.0.48" dependencies = [ "hdk", "serde", From a0dcbdaaed95fd4c333c5f245b5754f851741c9d Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Tue, 23 Aug 2022 13:57:05 -0700 Subject: [PATCH 2/5] CHANGELOG --- crates/holochain_zome_types/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/holochain_zome_types/CHANGELOG.md b/crates/holochain_zome_types/CHANGELOG.md index 4038af9e4f..e7c548dcc1 100644 --- a/crates/holochain_zome_types/CHANGELOG.md +++ b/crates/holochain_zome_types/CHANGELOG.md @@ -4,6 +4,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased](https://github.com/holochain/holochain/holochain_zome_types-v0.0.2-alpha.1...HEAD) +`ChainQueryFilter` has a `descending()` function which will cause the query results to be returned in descending order. This can be reversed by calling `ascending()`. The default order is still ascending. [\#1539](https://github.com/holochain/holochain/pull/1539) + ## 0.0.44 ## 0.0.43 From e7483de798f1ca5cd5674dd4353e70501075a8c7 Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Tue, 23 Aug 2022 14:16:17 -0700 Subject: [PATCH 3/5] Fix bad construction --- crates/holochain_state/src/source_chain.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/holochain_state/src/source_chain.rs b/crates/holochain_state/src/source_chain.rs index a10a998a7d..930db2d930 100644 --- a/crates/holochain_state/src/source_chain.rs +++ b/crates/holochain_state/src/source_chain.rs @@ -2001,6 +2001,7 @@ pub mod tests { entry_type: entry_type.clone(), entry_hashes: entry_hashes.clone(), include_entries, + descending: false, }; if sequence_range != ChainQueryFilterRange::Unbounded && (action_type.is_some() From a029220ab61e4b6bb8d4291c78fd5716526aa6d5 Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Tue, 23 Aug 2022 14:26:48 -0700 Subject: [PATCH 4/5] Rename ChainQueryFilter field --- crates/holochain_state/src/source_chain.rs | 4 ++-- crates/holochain_zome_types/src/query.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/holochain_state/src/source_chain.rs b/crates/holochain_state/src/source_chain.rs index 930db2d930..51aaeea3ba 100644 --- a/crates/holochain_state/src/source_chain.rs +++ b/crates/holochain_state/src/source_chain.rs @@ -772,7 +772,7 @@ where ORDER BY Action.seq ", ); - sql.push_str(if query.descending {" DESC"} else {" ASC"}); + sql.push_str(if query.order_descending {" DESC"} else {" ASC"}); let mut stmt = txn.prepare(&sql)?; let records = stmt .query_and_then( @@ -2001,7 +2001,7 @@ pub mod tests { entry_type: entry_type.clone(), entry_hashes: entry_hashes.clone(), include_entries, - descending: false, + order_descending: false, }; if sequence_range != ChainQueryFilterRange::Unbounded && (action_type.is_some() diff --git a/crates/holochain_zome_types/src/query.rs b/crates/holochain_zome_types/src/query.rs index be1fbe0e0a..160279e182 100644 --- a/crates/holochain_zome_types/src/query.rs +++ b/crates/holochain_zome_types/src/query.rs @@ -79,7 +79,7 @@ pub struct ChainQueryFilter { /// Include the entries in the records pub include_entries: bool, /// The query should be ordered in descending order (default is ascending) - pub descending: bool, + pub order_descending: bool, } #[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize, SerializedBytes)] @@ -211,13 +211,13 @@ impl ChainQueryFilter { /// Set the order to ascending. pub fn ascending(mut self) -> Self { - self.descending = false; + self.order_descending = false; self } /// Set the order to ascending. pub fn descending(mut self) -> Self { - self.descending = true; + self.order_descending = true; self } From 476fff92d07ba70d75201687dee1a83f61566faf Mon Sep 17 00:00:00 2001 From: Michael Dougherty Date: Thu, 8 Sep 2022 12:02:31 -0700 Subject: [PATCH 5/5] Add proper test of chain query filter ordering --- crates/holochain_state/src/source_chain.rs | 54 +++++++++++++++++++--- crates/holochain_zome_types/src/query.rs | 15 +++--- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/crates/holochain_state/src/source_chain.rs b/crates/holochain_state/src/source_chain.rs index 51aaeea3ba..c5f7eee188 100644 --- a/crates/holochain_state/src/source_chain.rs +++ b/crates/holochain_state/src/source_chain.rs @@ -1938,23 +1938,23 @@ pub mod tests { .await .unwrap(); - test_db.dump_tmp(); + // test_db.dump_tmp(); let chain = SourceChain::new(vault, dht_db.to_db(), dht_db_cache, keystore, alice.clone()) .await .unwrap(); - let elements = chain.query(ChainQueryFilter::default()).await.unwrap(); + let records = chain.query(ChainQueryFilter::default()).await.unwrap(); - // All of the range queries which should return a full set of elements + // All of the range queries which should return a full set of records let full_ranges = [ ChainQueryFilterRange::Unbounded, ChainQueryFilterRange::ActionSeqRange(0, 2), ChainQueryFilterRange::ActionHashRange( - elements[0].action_address().clone(), - elements[2].action_address().clone(), + records[0].action_address().clone(), + records[2].action_address().clone(), ), - ChainQueryFilterRange::ActionHashTerminated(elements[2].action_address().clone(), 2), + ChainQueryFilterRange::ActionHashTerminated(records[2].action_address().clone(), 2), ]; // A variety of combinations of query parameters @@ -1979,7 +1979,7 @@ pub mod tests { ( Some(ActionType::Create), Some(EntryType::AgentPubKey), - vec![elements[2].action().entry_hash().unwrap().clone()], + vec![records[2].action().entry_hash().unwrap().clone()], true, ), 1, @@ -2026,4 +2026,44 @@ pub mod tests { } } } + + #[tokio::test(flavor = "multi_thread")] + async fn source_chain_query_ordering() { + let test_db = test_authored_db(); + let dht_db = test_dht_db(); + let dht_db_cache = DhtDbQueryCache::new(dht_db.to_db().into()); + let keystore = test_keystore(); + let vault = test_db.to_db(); + let alice = keystore.new_sign_keypair_random().await.unwrap(); + let dna_hash = fixt!(DnaHash); + + genesis( + vault.clone().into(), + dht_db.to_db(), + &dht_db_cache, + keystore.clone(), + dna_hash.clone(), + alice.clone(), + None, + ) + .await + .unwrap(); + + let chain = SourceChain::new(vault, dht_db.to_db(), dht_db_cache, keystore, alice.clone()) + .await + .unwrap(); + + let asc = chain.query(ChainQueryFilter::default()).await.unwrap(); + let desc = chain + .query(ChainQueryFilter::default().descending()) + .await + .unwrap(); + + assert_eq!(asc.len(), 3); + assert_ne!(asc, desc); + + let mut desc_sorted = desc; + desc_sorted.sort_by_key(|r| r.signed_action.action().action_seq()); + assert_eq!(asc, desc_sorted); + } } diff --git a/crates/holochain_zome_types/src/query.rs b/crates/holochain_zome_types/src/query.rs index 160279e182..23f9c0976a 100644 --- a/crates/holochain_zome_types/src/query.rs +++ b/crates/holochain_zome_types/src/query.rs @@ -57,7 +57,10 @@ impl Default for ChainQueryFilterRange { } } -/// Query arguments +/// Specifies arguments to a query of the source chain, including ordering and filtering. +/// +/// This struct is used to construct an actual SQL query on the database, and also has methods +/// to allow filtering in-memory. #[derive( serde::Serialize, serde::Deserialize, SerializedBytes, Default, PartialEq, Clone, Debug, )] @@ -78,7 +81,8 @@ pub struct ChainQueryFilter { pub action_type: Option, /// Include the entries in the records pub include_entries: bool, - /// The query should be ordered in descending order (default is ascending) + /// The query should be ordered in descending order (default is ascending), + /// when run as a database query. There is no provisioning for in-memory ordering. pub order_descending: bool, } @@ -415,13 +419,6 @@ mod tests { map_query(&query_2, &actions), [false, true, false, true, false, false, false].to_vec() ); - assert_eq!( - map_query(&query_2.descending(), &actions), - [false, true, false, true, false, false, false] - .into_iter() - .rev() - .collect::>() - ); } #[test]