From 56d3b0e861b53eb9d5d33b6e57d43571313a4f98 Mon Sep 17 00:00:00 2001 From: Tobie Morgan Hitchcock Date: Thu, 25 Aug 2022 14:49:33 +0100 Subject: [PATCH] =?UTF-8?q?Fix=20bug=20where=20records=20couldn=E2=80=99t?= =?UTF-8?q?=20be=20updated=20after=20defining=20an=20index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #57 --- lib/src/dbs/options.rs | 20 +++- lib/src/doc/field.rs | 4 + lib/src/doc/index.rs | 8 +- lib/src/sql/statements/define.rs | 16 +++- lib/tests/define.rs | 154 +++++++++++++++++++++++++++++++ 5 files changed, 196 insertions(+), 6 deletions(-) diff --git a/lib/src/dbs/options.rs b/lib/src/dbs/options.rs index 121b78c9..0d3e52e2 100644 --- a/lib/src/dbs/options.rs +++ b/lib/src/dbs/options.rs @@ -29,14 +29,16 @@ pub struct Options { pub force: bool, // Should we run permissions checks? pub perms: bool, + // Should we error if tables don't exist? + pub strict: bool, // Should we process field queries? pub fields: bool, // Should we process event queries? pub events: bool, // Should we process table queries? pub tables: bool, - // Should we error if tables don't exist? - pub strict: bool, + // Should we process index queries? + pub indexes: bool, // Should we process function futures? pub futures: bool, } @@ -58,10 +60,11 @@ impl Options { perms: true, debug: false, force: false, + strict: false, fields: true, events: true, tables: true, - strict: false, + indexes: true, futures: false, auth: Arc::new(auth), } @@ -158,6 +161,17 @@ impl Options { } } + // Create a new Options object for a subquery + pub fn indexes(&self, v: bool) -> Options { + Options { + auth: self.auth.clone(), + ns: self.ns.clone(), + db: self.db.clone(), + indexes: v, + ..*self + } + } + // Create a new Options object for a subquery pub fn import(&self, v: bool) -> Options { Options { diff --git a/lib/src/doc/field.rs b/lib/src/doc/field.rs index 43041117..26bc7b7a 100644 --- a/lib/src/doc/field.rs +++ b/lib/src/doc/field.rs @@ -15,6 +15,10 @@ impl<'a> Document<'a> { txn: &Transaction, stm: &Statement<'_>, ) -> Result<(), Error> { + // Check fields + if !opt.fields { + return Ok(()); + } // Loop through all field statements for fd in self.fd(opt, txn).await?.iter() { // Loop over each field in document diff --git a/lib/src/doc/index.rs b/lib/src/doc/index.rs index 6a554127..92b034b5 100644 --- a/lib/src/doc/index.rs +++ b/lib/src/doc/index.rs @@ -14,6 +14,10 @@ impl<'a> Document<'a> { txn: &Transaction, _stm: &Statement<'_>, ) -> Result<(), Error> { + // Check events + if !opt.indexes { + return Ok(()); + } // Check if forced if !opt.force && !self.changed() { return Ok(()); @@ -50,7 +54,7 @@ impl<'a> Document<'a> { if self.initial.is_some() { #[rustfmt::skip] let key = crate::key::index::new(opt.ns(), opt.db(), &ix.what, &ix.name, o, None); - run.delc(key, Some(rid)).await?; + let _ = run.delc(key, Some(rid)).await; // Ignore this error } // Create the new index data if self.current.is_some() { @@ -69,7 +73,7 @@ impl<'a> Document<'a> { if self.initial.is_some() { #[rustfmt::skip] let key = crate::key::index::new(opt.ns(), opt.db(), &ix.what, &ix.name, o, Some(&rid.id)); - run.delc(key, Some(rid)).await?; + let _ = run.delc(key, Some(rid)).await; // Ignore this error } // Create the new index data if self.current.is_some() { diff --git a/lib/src/sql/statements/define.rs b/lib/src/sql/statements/define.rs index 54c1f5f9..fa28a1bf 100644 --- a/lib/src/sql/statements/define.rs +++ b/lib/src/sql/statements/define.rs @@ -599,8 +599,14 @@ impl DefineTableStatement { } // Release the transaction drop(run); - // Force tables to reprocess + // Force queries to run let opt = &opt.force(true); + // Don't process field queries + let opt = &opt.fields(false); + // Don't process event queries + let opt = &opt.events(false); + // Don't process index queries + let opt = &opt.indexes(false); // Process each foreign table for v in view.what.0.iter() { // Process the view data @@ -983,6 +989,14 @@ impl DefineIndexStatement { run.delr(beg..end, u32::MAX).await?; // Release the transaction drop(run); + // Force queries to run + let opt = &opt.force(true); + // Don't process field queries + let opt = &opt.fields(false); + // Don't process event queries + let opt = &opt.events(false); + // Don't process table queries + let opt = &opt.tables(false); // Update the index data let stm = UpdateStatement { what: Values(vec![Value::Table(self.what.clone().into())]), diff --git a/lib/tests/define.rs b/lib/tests/define.rs index 3d28afb7..1c072d2c 100644 --- a/lib/tests/define.rs +++ b/lib/tests/define.rs @@ -472,6 +472,56 @@ async fn define_statement_field_type_value_assert() -> Result<(), Error> { Ok(()) } +#[tokio::test] +async fn define_statement_index_single_simple() -> Result<(), Error> { + let sql = " + CREATE user:1 SET age = 23; + CREATE user:2 SET age = 10; + DEFINE INDEX test ON user FIELDS age; + DEFINE INDEX test ON user COLUMNS age; + INFO FOR TABLE user; + UPDATE user:1 SET age = 24; + UPDATE user:2 SET age = 11; + "; + let dbs = Datastore::new("memory").await?; + let ses = Session::for_kv().with_ns("test").with_db("test"); + let res = &mut dbs.execute(&sql, &ses, None, false).await?; + assert_eq!(res.len(), 7); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "{ + ev: {}, + fd: {}, + ft: {}, + ix: { test: 'DEFINE INDEX test ON user FIELDS age' }, + }", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse("[{ id: user:1, age: 24 }]"); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse("[{ id: user:2, age: 11 }]"); + assert_eq!(tmp, val); + // + Ok(()) +} + #[tokio::test] async fn define_statement_index_single() -> Result<(), Error> { let sql = " @@ -665,3 +715,107 @@ async fn define_statement_index_multiple_unique() -> Result<(), Error> { // Ok(()) } + +#[tokio::test] +async fn define_statement_index_single_unique_existing() -> Result<(), Error> { + let sql = " + CREATE user:1 SET email = 'info@surrealdb.com'; + CREATE user:2 SET email = 'test@surrealdb.com'; + CREATE user:3 SET email = 'test@surrealdb.com'; + DEFINE INDEX test ON user FIELDS email UNIQUE; + DEFINE INDEX test ON user COLUMNS email UNIQUE; + INFO FOR TABLE user; + "; + let dbs = Datastore::new("memory").await?; + let ses = Session::for_kv().with_ns("test").with_db("test"); + let res = &mut dbs.execute(&sql, &ses, None, false).await?; + assert_eq!(res.len(), 6); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(matches!( + tmp.err(), + Some(e) if e.to_string() == "Database index `test` already contains `user:3`" + )); + // + let tmp = res.remove(0).result; + assert!(matches!( + tmp.err(), + Some(e) if e.to_string() == "Database index `test` already contains `user:3`" + )); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "{ + ev: {}, + fd: {}, + ft: {}, + ix: {}, + }", + ); + assert_eq!(tmp, val); + // + Ok(()) +} + +#[tokio::test] +async fn define_statement_index_multiple_unique_existing() -> Result<(), Error> { + let sql = " + CREATE user:1 SET account = 'apple', email = 'test@surrealdb.com'; + CREATE user:2 SET account = 'tesla', email = 'test@surrealdb.com'; + CREATE user:3 SET account = 'apple', email = 'test@surrealdb.com'; + CREATE user:4 SET account = 'tesla', email = 'test@surrealdb.com'; + DEFINE INDEX test ON user FIELDS account, email UNIQUE; + DEFINE INDEX test ON user COLUMNS account, email UNIQUE; + INFO FOR TABLE user; + "; + let dbs = Datastore::new("memory").await?; + let ses = Session::for_kv().with_ns("test").with_db("test"); + let res = &mut dbs.execute(&sql, &ses, None, false).await?; + assert_eq!(res.len(), 7); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(tmp.is_ok()); + // + let tmp = res.remove(0).result; + assert!(matches!( + tmp.err(), + Some(e) if e.to_string() == "Database index `test` already contains `user:3`" + )); + // + let tmp = res.remove(0).result; + assert!(matches!( + tmp.err(), + Some(e) if e.to_string() == "Database index `test` already contains `user:3`" + )); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "{ + ev: {}, + fd: {}, + ft: {}, + ix: {}, + }", + ); + assert_eq!(tmp, val); + // + Ok(()) +}