From 3ccadb0740ca2ecc91c28f19274f9e6dece71e4d Mon Sep 17 00:00:00 2001
From: Emmanuel Keller <emmanuel.keller@surrealdb.com>
Date: Fri, 24 May 2024 19:09:10 +0100
Subject: [PATCH] [Bug fix] Index creation schemafull checks (#4091)

---
 core/src/sql/statements/define/index.rs | 25 +++++++++++++++-
 lib/tests/define.rs                     | 39 +++++++++++++++++++++----
 lib/tests/helpers.rs                    |  7 +++--
 3 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/core/src/sql/statements/define/index.rs b/core/src/sql/statements/define/index.rs
index 3c723fac..e32ed530 100644
--- a/core/src/sql/statements/define/index.rs
+++ b/core/src/sql/statements/define/index.rs
@@ -5,7 +5,7 @@ use crate::err::Error;
 use crate::iam::{Action, ResourceKind};
 use crate::sql::statements::info::InfoStructure;
 use crate::sql::{
-	statements::UpdateStatement, Base, Ident, Idioms, Index, Object, Strand, Value, Values,
+	statements::UpdateStatement, Base, Ident, Idioms, Index, Object, Part, Strand, Value, Values,
 };
 use derive::Store;
 use reblessive::tree::Stk;
@@ -52,6 +52,29 @@ impl DefineIndexStatement {
 				value: self.name.to_string(),
 			});
 		}
+		// If we are strict, check that the table exists
+		run.check_ns_db_tb(opt.ns(), opt.db(), &self.what, opt.strict).await?;
+		// Does the table exists?
+		match run.get_and_cache_tb(opt.ns(), opt.db(), &self.what).await {
+			Ok(db) => {
+				// Are we SchemaFull?
+				if db.full {
+					// Check that the fields exists
+					for idiom in self.cols.iter() {
+						if let Some(Part::Field(id)) = idiom.first() {
+							run.get_tb_field(opt.ns(), opt.db(), &self.what, id).await?;
+						}
+					}
+				}
+			}
+			// If the TB was not found, we're fine
+			Err(Error::TbNotFound {
+				..
+			}) => {}
+			// Any other error should be returned
+			Err(e) => return Err(e),
+		}
+
 		// Process the statement
 		let key = crate::key::table::ix::new(opt.ns(), opt.db(), &self.what, &self.name);
 		run.add_ns(opt.ns(), opt.strict).await?;
diff --git a/lib/tests/define.rs b/lib/tests/define.rs
index 767e356e..62defd23 100644
--- a/lib/tests/define.rs
+++ b/lib/tests/define.rs
@@ -854,11 +854,7 @@ async fn define_statement_index_multiple() -> Result<(), Error> {
 	let res = &mut dbs.execute(sql, &ses, None).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());
+	skip_ok(res, 2)?;
 	//
 	let tmp = res.remove(0).result?;
 	let val = Value::parse(
@@ -1236,6 +1232,39 @@ async fn define_statement_index_multiple_unique_embedded_multiple() -> Result<()
 	Ok(())
 }
 
+#[tokio::test]
+async fn define_statement_index_on_schemafull_without_permission() -> Result<(), Error> {
+	let sql = "
+		DEFINE TABLE test SCHEMAFULL PERMISSIONS NONE;
+		DEFINE INDEX idx ON test FIELDS foo;
+	";
+	let dbs = new_ds().await?;
+	let ses = Session::owner().with_ns("test").with_db("test");
+	let mut res = &mut dbs.execute(sql, &ses, None).await?;
+	assert_eq!(res.len(), 2);
+	//
+	skip_ok(&mut res, 1)?;
+	//
+	let tmp = res.remove(0).result;
+	let s = format!("{:?}", tmp);
+	assert!(
+		tmp.is_err_and(|e| {
+			if let Error::FdNotFound {
+				value,
+			} = e
+			{
+				assert_eq!(value, "foo", "Wrong field: {value}");
+				true
+			} else {
+				false
+			}
+		}),
+		"Expected error, but got: {:?}",
+		s
+	);
+	Ok(())
+}
+
 #[tokio::test]
 async fn define_statement_analyzer() -> Result<(), Error> {
 	let sql = r#"
diff --git a/lib/tests/helpers.rs b/lib/tests/helpers.rs
index dd138ff7..2384f7a4 100644
--- a/lib/tests/helpers.rs
+++ b/lib/tests/helpers.rs
@@ -197,8 +197,11 @@ pub fn with_enough_stack(
 
 #[allow(dead_code)]
 pub fn skip_ok(res: &mut Vec<Response>, skip: usize) -> Result<(), Error> {
-	for _ in 0..skip {
-		let _ = res.remove(0).result?;
+	for i in 0..skip {
+		let r = res.remove(0).result;
+		let _ = r.is_err_and(|e| {
+			panic!("Statement #{i} fails with: {e}");
+		});
 	}
 	Ok(())
 }