Improve id field handling in the statement data clause (#2487)

This commit is contained in:
Tobie Morgan Hitchcock 2023-08-21 23:47:35 +01:00 committed by GitHub
parent 27cc21876d
commit afdd0b3c85
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 312 additions and 218 deletions

View file

@ -77,11 +77,189 @@ impl Iterator {
Self::default()
}
/// Prepares a value for processing
/// Ingests an iterable for processing
pub fn ingest(&mut self, val: Iterable) {
self.entries.push(val)
}
/// Prepares a value for processing
pub async fn prepare(
&mut self,
ctx: &Context<'_>,
opt: &Options,
txn: &Transaction,
stm: &Statement<'_>,
val: Value,
) -> Result<(), Error> {
// Match the values
match val {
Value::Table(v) => match stm.data() {
// There is a data clause so fetch a record id
Some(data) => match stm {
Statement::Create(_) => {
let id = match data.rid(ctx, opt, txn).await? {
// Generate a new id from the id field
Some(id) => id.generate(&v, false)?,
// Generate a new random table id
None => v.generate(),
};
self.ingest(Iterable::Thing(id))
}
_ => {
// Ingest the table for scanning
self.ingest(Iterable::Table(v))
}
},
// There is no data clause so create a record id
None => match stm {
Statement::Create(_) => {
// Generate a new random table id
self.ingest(Iterable::Thing(v.generate()))
}
_ => {
// Ingest the table for scanning
self.ingest(Iterable::Table(v))
}
},
},
Value::Thing(v) => {
// Check if there is a data clause
if let Some(data) = stm.data() {
// Check if there is an id field specified
if let Some(id) = data.rid(ctx, opt, txn).await? {
// Check to see the type of the id
match id {
// The id is a match, so don't error
Value::Thing(id) if id == v => (),
// The id does not match
id => {
return Err(Error::IdMismatch {
value: id.to_string(),
});
}
}
}
}
// Add the record to the iterator
self.ingest(Iterable::Thing(v));
}
Value::Model(v) => {
// Check if there is a data clause
if let Some(data) = stm.data() {
// Check if there is an id field specified
if let Some(id) = data.rid(ctx, opt, txn).await? {
return Err(Error::IdMismatch {
value: id.to_string(),
});
}
}
// Add the records to the iterator
for v in v {
self.ingest(Iterable::Thing(v))
}
}
Value::Range(v) => {
// Check if this is a create statement
if let Statement::Create(_) = stm {
return Err(Error::InvalidStatementTarget {
value: v.to_string(),
});
}
// Check if there is a data clause
if let Some(data) = stm.data() {
// Check if there is an id field specified
if let Some(id) = data.rid(ctx, opt, txn).await? {
return Err(Error::IdMismatch {
value: id.to_string(),
});
}
}
// Add the record to the iterator
self.ingest(Iterable::Range(*v));
}
Value::Edges(v) => {
// Check if this is a create statement
if let Statement::Create(_) = stm {
return Err(Error::InvalidStatementTarget {
value: v.to_string(),
});
}
// Check if there is a data clause
if let Some(data) = stm.data() {
// Check if there is an id field specified
if let Some(id) = data.rid(ctx, opt, txn).await? {
return Err(Error::IdMismatch {
value: id.to_string(),
});
}
}
// Add the record to the iterator
self.ingest(Iterable::Edges(*v));
}
Value::Object(v) => {
// Check if there is a data clause
if let Some(data) = stm.data() {
// Check if there is an id field specified
if let Some(id) = data.rid(ctx, opt, txn).await? {
return Err(Error::IdMismatch {
value: id.to_string(),
});
}
}
// Check if the object has an id field
match v.rid() {
Some(id) => {
// Add the record to the iterator
self.ingest(Iterable::Thing(id))
}
None => {
return Err(Error::InvalidStatementTarget {
value: v.to_string(),
});
}
}
}
Value::Array(v) => {
// Check if there is a data clause
if let Some(data) = stm.data() {
// Check if there is an id field specified
if let Some(id) = data.rid(ctx, opt, txn).await? {
return Err(Error::IdMismatch {
value: id.to_string(),
});
}
}
// Add the records to the iterator
for v in v {
match v {
Value::Thing(v) => self.ingest(Iterable::Thing(v)),
Value::Edges(v) => self.ingest(Iterable::Edges(*v)),
Value::Object(v) => match v.rid() {
Some(v) => self.ingest(Iterable::Thing(v)),
None => {
return Err(Error::InvalidStatementTarget {
value: v.to_string(),
})
}
},
_ => {
return Err(Error::InvalidStatementTarget {
value: v.to_string(),
})
}
}
}
}
v => {
return Err(Error::InvalidStatementTarget {
value: v.to_string(),
})
}
};
// All ingested ok
Ok(())
}
/// Process the records and output
pub async fn output(
&mut self,

View file

@ -10,6 +10,7 @@ use crate::sql::idiom::Idiom;
use crate::sql::number::Number;
use crate::sql::operator::Operator;
use crate::sql::part::Part;
use crate::sql::paths::ID;
use crate::sql::statement::Statement as Query;
use crate::sql::statements::delete::DeleteStatement;
use crate::sql::statements::ifelse::IfelseStatement;
@ -175,7 +176,6 @@ impl<'a> Document<'a> {
tb: ft.name.to_raw(),
id: rid.id.clone(),
};
// Use the current record data
// Check if a WHERE clause is specified
match &tb.cond {
// There is a WHERE clause specified
@ -192,17 +192,7 @@ impl<'a> Document<'a> {
// Update the value in the table
_ => Query::Update(UpdateStatement {
what: Values(vec![Value::from(rid)]),
data: Some(Data::ReplaceExpression(
tb.expr
.compute(
ctx,
opt,
txn,
Some(&self.current),
false,
)
.await?,
)),
data: Some(self.full(ctx, opt, txn, &tb.expr).await?),
..UpdateStatement::default()
}),
};
@ -232,11 +222,7 @@ impl<'a> Document<'a> {
// Update the value in the table
_ => Query::Update(UpdateStatement {
what: Values(vec![Value::from(rid)]),
data: Some(Data::ReplaceExpression(
tb.expr
.compute(ctx, opt, txn, Some(&self.current), false)
.await?,
)),
data: Some(self.full(ctx, opt, txn, &tb.expr).await?),
..UpdateStatement::default()
}),
};
@ -251,6 +237,18 @@ impl<'a> Document<'a> {
Ok(())
}
//
async fn full(
&self,
ctx: &Context<'_>,
opt: &Options,
txn: &Transaction,
exp: &Fields,
) -> Result<Data, Error> {
let mut data = exp.compute(ctx, opt, txn, Some(&self.current), false).await?;
data.cut(ID.as_ref());
Ok(Data::ReplaceExpression(data))
}
//
async fn data(
&self,
ctx: &Context<'_>,
@ -275,7 +273,13 @@ impl<'a> Document<'a> {
alias,
} = field
{
// Get the name of the field
let idiom = alias.clone().unwrap_or_else(|| expr.to_idiom());
// Ignore any id field
if idiom.is_id() {
continue;
}
// Process the field projection
match expr {
Value::Function(f) if f.is_rolling() => match f.name() {
"count" => {

View file

@ -344,44 +344,50 @@ pub enum Error {
#[error("Reached excessive computation depth due to functions, subqueries, or futures")]
ComputationDepthExceeded,
/// Can not execute CREATE query using the specified value
#[error("Can not execute CREATE query using value '{value}'")]
/// Can not execute statement using the specified value
#[error("Can not execute statement using value '{value}'")]
InvalidStatementTarget {
value: String,
},
/// Can not execute CREATE statement using the specified value
#[error("Can not execute CREATE statement using value '{value}'")]
CreateStatement {
value: String,
},
/// Can not execute UPDATE query using the specified value
#[error("Can not execute UPDATE query using value '{value}'")]
/// Can not execute UPDATE statement using the specified value
#[error("Can not execute UPDATE statement using value '{value}'")]
UpdateStatement {
value: String,
},
/// Can not execute RELATE query using the specified value
#[error("Can not execute RELATE query using value '{value}'")]
/// Can not execute RELATE statement using the specified value
#[error("Can not execute RELATE statement using value '{value}'")]
RelateStatement {
value: String,
},
/// Can not execute DELETE query using the specified value
#[error("Can not execute DELETE query using value '{value}'")]
/// Can not execute DELETE statement using the specified value
#[error("Can not execute DELETE statement using value '{value}'")]
DeleteStatement {
value: String,
},
/// Can not execute INSERT query using the specified value
#[error("Can not execute INSERT query using value '{value}'")]
/// Can not execute INSERT statement using the specified value
#[error("Can not execute INSERT statement using value '{value}'")]
InsertStatement {
value: String,
},
/// Can not execute LIVE query using the specified value
#[error("Can not execute LIVE query using value '{value}'")]
/// Can not execute LIVE statement using the specified value
#[error("Can not execute LIVE statement using value '{value}'")]
LiveStatement {
value: String,
},
/// Can not execute KILL query using the specified id
#[error("Can not execute KILL query using id '{value}'")]
/// Can not execute KILL statement using the specified id
#[error("Can not execute KILL statement using id '{value}'")]
KillStatement {
value: String,
},
@ -430,8 +436,14 @@ pub enum Error {
check: String,
},
/// Found a record id for the record but we are creating a specific record
#[error("Found {value} for the id field, but a specific record has been specified")]
IdMismatch {
value: String,
},
/// Found a record id for the record but this is not a valid id
#[error("Found '{value}' for the record ID but this is not a valid id")]
#[error("Found {value} for the Record ID but this is not a valid id")]
IdInvalid {
value: String,
},

View file

@ -8,8 +8,6 @@ use crate::sql::error::IResult;
use crate::sql::fmt::Fmt;
use crate::sql::idiom::{plain as idiom, Idiom};
use crate::sql::operator::{assigner, Operator};
use crate::sql::table::Table;
use crate::sql::thing::Thing;
use crate::sql::value::{value, Value};
use nom::branch::alt;
use nom::bytes::complete::tag_no_case;
@ -46,31 +44,30 @@ impl Data {
ctx: &Context<'_>,
opt: &Options,
txn: &Transaction,
tb: &Table,
) -> Result<Thing, Error> {
) -> Result<Option<Value>, Error> {
match self {
Self::MergeExpression(v) => {
// This MERGE expression has an 'id' field
v.compute(ctx, opt, txn, None).await?.rid().generate(tb, false)
Ok(v.compute(ctx, opt, txn, None).await?.rid().some())
}
Self::ReplaceExpression(v) => {
// This REPLACE expression has an 'id' field
v.compute(ctx, opt, txn, None).await?.rid().generate(tb, false)
Ok(v.compute(ctx, opt, txn, None).await?.rid().some())
}
Self::ContentExpression(v) => {
// This CONTENT expression has an 'id' field
v.compute(ctx, opt, txn, None).await?.rid().generate(tb, false)
Ok(v.compute(ctx, opt, txn, None).await?.rid().some())
}
Self::SetExpression(v) => match v.iter().find(|f| f.0.is_id()) {
Some((_, _, v)) => {
// This SET expression has an 'id' field
v.compute(ctx, opt, txn, None).await?.generate(tb, false)
Ok(v.compute(ctx, opt, txn, None).await?.some())
}
// This SET expression had no 'id' field
_ => Ok(tb.generate()),
_ => Ok(None),
},
// Generate a random id for all other data clauses
_ => Ok(tb.generate()),
_ => Ok(None),
}
}
}

View file

@ -2,7 +2,7 @@ use crate::ctx::Context;
use crate::dbs::Iterator;
use crate::dbs::Options;
use crate::dbs::Statement;
use crate::dbs::{Iterable, Transaction};
use crate::dbs::Transaction;
use crate::doc::CursorDoc;
use crate::err::Error;
use crate::sql::comment::shouldbespace;
@ -55,72 +55,22 @@ impl CreateStatement {
opt.valid_for_db()?;
// Create a new iterator
let mut i = Iterator::new();
// Assign the statement
let stm = Statement::from(self);
// Ensure futures are stored
let opt = &opt.new_with_futures(false);
// Loop over the create targets
for w in self.what.0.iter() {
let v = w.compute(ctx, opt, txn, doc).await?;
match v {
Value::Table(v) => match &self.data {
// There is a data clause so check for a record id
Some(data) => match data.rid(ctx, opt, txn, &v).await {
// There was a problem creating the record id
Err(e) => return Err(e),
// There is an id field so use the record id
Ok(v) => i.ingest(Iterable::Thing(v)),
},
// There is no data clause so create a record id
None => i.ingest(Iterable::Thing(v.generate())),
i.prepare(ctx, opt, txn, &stm, v).await.map_err(|e| match e {
Error::InvalidStatementTarget {
value: v,
} => Error::CreateStatement {
value: v,
},
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Model(v) => {
for v in v {
i.ingest(Iterable::Thing(v));
}
}
Value::Array(v) => {
for v in v {
match v {
Value::Table(v) => i.ingest(Iterable::Thing(v.generate())),
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Model(v) => {
for v in v {
i.ingest(Iterable::Thing(v));
}
}
Value::Object(v) => match v.rid() {
Some(v) => i.ingest(Iterable::Thing(v)),
None => {
return Err(Error::CreateStatement {
value: v.to_string(),
})
}
},
v => {
return Err(Error::CreateStatement {
value: v.to_string(),
})
}
};
}
}
Value::Object(v) => match v.rid() {
Some(v) => i.ingest(Iterable::Thing(v)),
None => {
return Err(Error::CreateStatement {
value: v.to_string(),
})
}
},
v => {
return Err(Error::CreateStatement {
value: v.to_string(),
})
}
};
e => e,
})?;
}
// Assign the statement
let stm = Statement::from(self);
// Output the results
i.output(ctx, opt, txn, &stm).await
}

View file

@ -2,7 +2,7 @@ use crate::ctx::Context;
use crate::dbs::Iterator;
use crate::dbs::Options;
use crate::dbs::Statement;
use crate::dbs::{Iterable, Transaction};
use crate::dbs::Transaction;
use crate::doc::CursorDoc;
use crate::err::Error;
use crate::sql::comment::shouldbespace;
@ -55,65 +55,22 @@ impl DeleteStatement {
opt.valid_for_db()?;
// Create a new iterator
let mut i = Iterator::new();
// Assign the statement
let stm = Statement::from(self);
// Ensure futures are stored
let opt = &opt.new_with_futures(false);
// Loop over the delete targets
for w in self.what.0.iter() {
let v = w.compute(ctx, opt, txn, doc).await?;
match v {
Value::Table(v) => i.ingest(Iterable::Table(v)),
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Range(v) => i.ingest(Iterable::Range(*v)),
Value::Edges(v) => i.ingest(Iterable::Edges(*v)),
Value::Model(v) => {
for v in v {
i.ingest(Iterable::Thing(v));
}
}
Value::Array(v) => {
for v in v {
match v {
Value::Table(v) => i.ingest(Iterable::Table(v)),
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Edges(v) => i.ingest(Iterable::Edges(*v)),
Value::Model(v) => {
for v in v {
i.ingest(Iterable::Thing(v));
}
}
Value::Object(v) => match v.rid() {
Some(v) => i.ingest(Iterable::Thing(v)),
None => {
return Err(Error::DeleteStatement {
value: v.to_string(),
})
}
},
v => {
return Err(Error::DeleteStatement {
value: v.to_string(),
})
}
};
}
}
Value::Object(v) => match v.rid() {
Some(v) => i.ingest(Iterable::Thing(v)),
None => {
return Err(Error::DeleteStatement {
value: v.to_string(),
})
}
i.prepare(ctx, opt, txn, &stm, v).await.map_err(|e| match e {
Error::InvalidStatementTarget {
value: v,
} => Error::DeleteStatement {
value: v,
},
v => {
return Err(Error::DeleteStatement {
value: v.to_string(),
})
}
};
e => e,
})?;
}
// Assign the statement
let stm = Statement::from(self);
// Output the results
i.output(ctx, opt, txn, &stm).await
}

View file

@ -164,12 +164,13 @@ impl RelateStatement {
// The relation does not have a specific record id
Value::Table(tb) => match &self.data {
// There is a data clause so check for a record id
Some(data) => match data.rid(ctx, opt, txn, tb).await {
// There was a problem creating the record id
Err(e) => return Err(e),
// There is an id field so use the record id
Ok(t) => i.ingest(Iterable::Relatable(f, t, w)),
},
Some(data) => {
let id = match data.rid(ctx, opt, txn).await? {
Some(id) => id.generate(tb, false)?,
None => tb.generate(),
};
i.ingest(Iterable::Relatable(f, id, w))
}
// There is no data clause so create a record id
None => i.ingest(Iterable::Relatable(f, tb.generate(), w)),
},

View file

@ -111,7 +111,6 @@ impl SelectStatement {
Value::Array(v) => {
for v in v {
match v {
Value::Table(v) => i.ingest(Iterable::Table(v)),
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Edges(v) => i.ingest(Iterable::Edges(*v)),
Value::Model(v) => {

View file

@ -2,7 +2,7 @@ use crate::ctx::Context;
use crate::dbs::Iterator;
use crate::dbs::Options;
use crate::dbs::Statement;
use crate::dbs::{Iterable, Transaction};
use crate::dbs::Transaction;
use crate::doc::CursorDoc;
use crate::err::Error;
use crate::sql::comment::shouldbespace;
@ -56,65 +56,22 @@ impl UpdateStatement {
opt.valid_for_db()?;
// Create a new iterator
let mut i = Iterator::new();
// Assign the statement
let stm = Statement::from(self);
// Ensure futures are stored
let opt = &opt.new_with_futures(false);
// Loop over the update targets
for w in self.what.0.iter() {
let v = w.compute(ctx, opt, txn, doc).await?;
match v {
Value::Table(v) => i.ingest(Iterable::Table(v)),
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Range(v) => i.ingest(Iterable::Range(*v)),
Value::Edges(v) => i.ingest(Iterable::Edges(*v)),
Value::Model(v) => {
for v in v {
i.ingest(Iterable::Thing(v));
}
}
Value::Array(v) => {
for v in v {
match v {
Value::Table(v) => i.ingest(Iterable::Table(v)),
Value::Thing(v) => i.ingest(Iterable::Thing(v)),
Value::Edges(v) => i.ingest(Iterable::Edges(*v)),
Value::Model(v) => {
for v in v {
i.ingest(Iterable::Thing(v));
}
}
Value::Object(v) => match v.rid() {
Some(v) => i.ingest(Iterable::Thing(v)),
None => {
return Err(Error::UpdateStatement {
value: v.to_string(),
})
}
},
v => {
return Err(Error::UpdateStatement {
value: v.to_string(),
})
}
};
}
}
Value::Object(v) => match v.rid() {
Some(v) => i.ingest(Iterable::Thing(v)),
None => {
return Err(Error::UpdateStatement {
value: v.to_string(),
})
}
i.prepare(ctx, opt, txn, &stm, v).await.map_err(|e| match e {
Error::InvalidStatementTarget {
value: v,
} => Error::UpdateStatement {
value: v,
},
v => {
return Err(Error::UpdateStatement {
value: v.to_string(),
})
}
};
e => e,
})?;
}
// Assign the statement
let stm = Statement::from(self);
// Output the results
i.output(ctx, opt, txn, &stm).await
}

View file

@ -18,7 +18,7 @@ impl Value {
id: id.as_int().into(),
}),
// There is a string for the id field
Value::Strand(id) => Ok(Thing {
Value::Strand(id) if !id.is_empty() => Ok(Thing {
tb: tb.to_string(),
id: id.into(),
}),

View file

@ -781,6 +781,14 @@ impl Value {
Ok(self)
}
/// Convert this Value to an Option
pub fn some(self) -> Option<Value> {
match self {
Value::None => None,
val => Some(val),
}
}
// -----------------------------------
// Simple value detection
// -----------------------------------

View file

@ -11,6 +11,7 @@ use surrealdb::sql::Value;
#[tokio::test]
async fn create_with_id() -> Result<(), Error> {
let sql = "
-- Should succeed
CREATE person:test SET name = 'Tester';
CREATE person SET id = person:tobie, name = 'Tobie';
CREATE person CONTENT { id: person:jaime, name: 'Jaime' };
@ -21,11 +22,17 @@ async fn create_with_id() -> Result<(), Error> {
CREATE test CONTENT { id: other:715917898417176677 };
CREATE test CONTENT { id: other:715917898.417176677 };
CREATE test CONTENT { id: other:9223372036854775808 };
-- Should error as id is empty
CREATE person SET id = '';
CREATE person CONTENT { id: '', name: 'Tester' };
-- Should error as id is mismatched
CREATE person:other SET id = 'tobie';
CREATE person:other CONTENT { id: 'tobie', name: 'Tester' };
";
let dbs = Datastore::new("memory").await?;
let ses = Session::owner().with_ns("test").with_db("test");
let res = &mut dbs.execute(sql, &ses, None).await?;
assert_eq!(res.len(), 10);
assert_eq!(res.len(), 14);
//
let tmp = res.remove(0).result?;
let val = Value::parse(
@ -134,6 +141,30 @@ async fn create_with_id() -> Result<(), Error> {
);
assert_eq!(tmp, val);
//
let tmp = res.remove(0).result;
assert!(matches!(
tmp.err(),
Some(e) if e.to_string() == r#"Found '' for the Record ID but this is not a valid id"#
));
//
let tmp = res.remove(0).result;
assert!(matches!(
tmp.err(),
Some(e) if e.to_string() == r#"Found '' for the Record ID but this is not a valid id"#
));
//
let tmp = res.remove(0).result;
assert!(matches!(
tmp.err(),
Some(e) if e.to_string() == r#"Found 'tobie' for the id field, but a specific record has been specified"#
));
//
let tmp = res.remove(0).result;
assert!(matches!(
tmp.err(),
Some(e) if e.to_string() == r#"Found 'tobie' for the id field, but a specific record has been specified"#
));
//
Ok(())
}