Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fix] Handle $ signs in values on Mongo #3211

Merged
merged 2 commits into from Sep 20, 2022
Merged

[Fix] Handle $ signs in values on Mongo #3211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb9bae6
test and first fix idea
do4gr Sep 20, 2022
b377292
rename function
do4gr Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/mod.rs
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@ mod max_integer;
mod prisma_10098;
mod prisma_10935;
mod prisma_12929;
mod prisma_13089;
mod prisma_13097;
mod prisma_14001;
mod prisma_14696;
Expand Down
44 changes: 44 additions & 0 deletions query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_13089.rs
View file
Open in desktop
@@ -0,0 +1,44 @@
use query_engine_tests::*;

#[test_suite(schema(schema))]
mod prisma_13097 {
fn schema() -> String {
r#"
model TestModel {
id Int @id @map("_id")
text String
}
"#
.to_owned()
}

#[connector_test]
async fn filtering_with_dollar_values(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, "mutation { createOneTestModel (data: {id: 1, text: \"foo\"}) { id, text }}"),
@r###"{"data":{"createOneTestModel":{"id":1,"text":"foo"}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, "mutation { createOneTestModel (data: {id: 2, text: \"$foo\"}) { id, text }}"),
@r###"{"data":{"createOneTestModel":{"id":2,"text":"$foo"}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, "query { findFirstTestModel (where: {text: \"foo\"}) { id, text }}"),
@r###"{"data":{"findFirstTestModel":{"id":1,"text":"foo"}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, "query { findFirstTestModel (where: {text: \"$foo\"}) { id, text }}"),
@r###"{"data":{"findFirstTestModel":{"id":2,"text":"$foo"}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, "query { findManyTestModel { id, text }}"),
@r###"{"data":{"findManyTestModel":[{"id":1,"text":"foo"},{"id":2,"text":"$foo"}]}}"###
);

Ok(())
}
}
23 changes: 12 additions & 11 deletions query-engine/connectors/mongodb-query-connector/src/filter.rs
View file
Open in desktop
Expand Up @@ -157,10 +157,10 @@ impl MongoFilterVisitor {

let filter_doc = match condition {
ScalarCondition::Equals(val) => {
doc! { "$eq": [&field_name, self.as_bson_coerce_count(field, val)?] }
doc! { "$eq": [&field_name, self.coerce_to_bson_for_filter(field, val)?] }
}
ScalarCondition::NotEquals(val) => {
doc! { "$ne": [&field_name, self.as_bson_coerce_count(field, val)?] }
doc! { "$ne": [&field_name, self.coerce_to_bson_for_filter(field, val)?] }
}
ScalarCondition::Contains(val) => self.regex_match(&field_name, field, ".*", val, ".*", false)?,
ScalarCondition::NotContains(val) => {
Expand All @@ -175,16 +175,16 @@ impl MongoFilterVisitor {
doc! { "$not": self.regex_match(&field_name, field, "", val, "$", false)? }
}
ScalarCondition::LessThan(val) => {
doc! { "$lt": [&field_name, self.as_bson_coerce_count(field, val)?] }
doc! { "$lt": [&field_name, self.coerce_to_bson_for_filter(field, val)?] }
}
ScalarCondition::LessThanOrEquals(val) => {
doc! { "$lte": [&field_name, self.as_bson_coerce_count(field, val)?] }
doc! { "$lte": [&field_name, self.coerce_to_bson_for_filter(field, val)?] }
}
ScalarCondition::GreaterThan(val) => {
doc! { "$gt": [&field_name, self.as_bson_coerce_count(field, val)?] }
doc! { "$gt": [&field_name, self.coerce_to_bson_for_filter(field, val)?] }
}
ScalarCondition::GreaterThanOrEquals(val) => {
doc! { "$gte": [&field_name, self.as_bson_coerce_count(field, val)?] }
doc! { "$gte": [&field_name, self.coerce_to_bson_for_filter(field, val)?] }
}
// Todo: The nested list unpack looks like a bug somewhere.
// Likely join code mistakenly repacks a list into a list of PrismaValue somewhere in the core.
Expand All @@ -199,7 +199,7 @@ impl MongoFilterVisitor {
bson_values.extend(
inner
.into_iter()
.map(|val| self.as_bson_coerce_count(field, val))
.map(|val| self.coerce_to_bson_for_filter(field, val))
.collect::<crate::Result<Vec<_>>>()?,
)
}
Expand All @@ -208,7 +208,7 @@ impl MongoFilterVisitor {
doc! { "$in": [&field_name, bson_values] }
}
_ => {
doc! { "$in": [&field_name, self.as_bson_coerce_count(field, PrismaValue::List(vals))?] }
doc! { "$in": [&field_name, self.coerce_to_bson_for_filter(field, PrismaValue::List(vals))?] }
}
},
ConditionListValue::FieldRef(field_ref) => {
Expand All @@ -219,7 +219,7 @@ impl MongoFilterVisitor {
ConditionListValue::List(vals) => {
let bson_values = vals
.into_iter()
.map(|val| self.as_bson_coerce_count(field, val))
.map(|val| self.coerce_to_bson_for_filter(field, val))
.collect::<crate::Result<Vec<_>>>()?;

doc! { "$not": { "$in": [&field_name, bson_values] } }
Expand Down Expand Up @@ -725,13 +725,14 @@ impl MongoFilterVisitor {
///
/// When converting the value of a `_count` aggregation filter for a field that's _not_ numerical,
/// we force the `TypeIdentifier` to be `Int` to prevent panics.
fn as_bson_coerce_count(&self, sf: &ScalarFieldRef, value: impl Into<ConditionValue>) -> crate::Result<Bson> {
fn coerce_to_bson_for_filter(&self, sf: &ScalarFieldRef, value: impl Into<ConditionValue>) -> crate::Result<Bson> {
match value.into() {
ConditionValue::Value(value) => {
if self.parent_is_count_aggregation() && !sf.is_numeric() {
(&TypeIdentifier::Int, value).into_bson()
} else {
(sf, value).into_bson()
let bson_value = (sf, value).into_bson()?;
Ok(Bson::Document(doc! {"$literal": bson_value}))
}
}
ConditionValue::FieldRef(field_ref) => self.prefixed_field_ref(&field_ref),
Expand Down