Skip to content

Commit e96eae7

Browse files
authored
fix: implement a workaround for JSON list equality (#5804)
[TML-2177](https://linear.app/prisma-company/issue/TML-2177/fix-list-filtersjson-listsequality) Fixes the only remaining test that was broken due to the parameterisation changes. The issue boils down to two problems present in the JSON list equality test, one is that we get a `Json[]` versus `Json` type mismatch when parsing and second is that we would currently insert an invalid `::jsonb` cast in `visit_equals`. I've tried a number of different fixes, my main idea was to tag the `Json` array parameters as `List(Json)` instead, which seems the cleanest option, but that causes a huge fallout breaking lots of other tests and I couldn't find an easy way to fix them without implementing multiple other workarounds due to the `Json` type already receiving special treatment in multiple places, so I ended up patching `parse_parameterized_list` and `visit_equals` to handle this case specifically.
1 parent 8c28700 commit e96eae7

5 files changed

Lines changed: 107 additions & 59 deletions

File tree

libs/prisma-value/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ impl Placeholder {
140140
r#type,
141141
}
142142
}
143+
144+
pub fn with_type(self, r#type: PrismaValueType) -> Self {
145+
Self {
146+
name: self.name,
147+
r#type,
148+
}
149+
}
143150
}
144151

145152
/// Stringify a date to the following format

quaint/src/ast/expression.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,14 @@ impl<'a> Expression<'a> {
179179
}
180180
}
181181

182+
#[allow(dead_code)]
183+
pub(crate) fn as_column(&self) -> Option<&Column<'a>> {
184+
match &self.kind {
185+
ExpressionKind::Column(column) => Some(column),
186+
_ => None,
187+
}
188+
}
189+
182190
#[allow(dead_code)]
183191
pub(crate) fn into_column(self) -> Option<Column<'a>> {
184192
match self.kind {

quaint/src/visitor/postgres.rs

Lines changed: 50 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -415,47 +415,23 @@ impl<'a> Visitor<'a> for Postgres<'a> {
415415
}
416416

417417
fn visit_equals(&mut self, left: Expression<'a>, right: Expression<'a>) -> visitor::Result {
418-
// LHS must be cast to json/xml-text if the right is a json/xml-text value and vice versa.
419-
let right_cast = match left {
420-
_ if left.is_json_value() => "::jsonb",
421-
_ if left.is_xml_value() => "::text",
422-
_ => "",
423-
};
424-
425-
let left_cast = match right {
426-
_ if right.is_json_value() => "::jsonb",
427-
_ if right.is_xml_value() => "::text",
428-
_ => "",
429-
};
418+
let lhs_conv = EqualsOperandConversion::infer(&left, &right);
419+
let rhs_conv = EqualsOperandConversion::infer(&right, &left);
430420

431-
self.visit_expression(left)?;
432-
self.write(left_cast)?;
421+
lhs_conv.write(left, self)?;
433422
self.write(" = ")?;
434-
self.visit_expression(right)?;
435-
self.write(right_cast)?;
423+
rhs_conv.write(right, self)?;
436424

437425
Ok(())
438426
}
439427

440428
fn visit_not_equals(&mut self, left: Expression<'a>, right: Expression<'a>) -> visitor::Result {
441-
// LHS must be cast to json/xml-text if the right is a json/xml-text value and vice versa.
442-
let right_cast = match left {
443-
_ if left.is_json_value() => "::jsonb",
444-
_ if left.is_xml_value() => "::text",
445-
_ => "",
446-
};
429+
let lhs_conv = EqualsOperandConversion::infer(&left, &right);
430+
let rhs_conv = EqualsOperandConversion::infer(&right, &left);
447431

448-
let left_cast = match right {
449-
_ if right.is_json_value() => "::jsonb",
450-
_ if right.is_xml_value() => "::text",
451-
_ => "",
452-
};
453-
454-
self.visit_expression(left)?;
455-
self.write(left_cast)?;
432+
lhs_conv.write(left, self)?;
456433
self.write(" <> ")?;
457-
self.visit_expression(right)?;
458-
self.write(right_cast)?;
434+
rhs_conv.write(right, self)?;
459435

460436
Ok(())
461437
}
@@ -819,6 +795,48 @@ fn get_column_cast_target(column: &Column<'_>) -> Option<&'static str> {
819795
}
820796
}
821797

798+
/// Utility type for equality and inequality expressions to determine if and how operands should
799+
/// be converted before comparison.
800+
#[derive(Debug, Clone, Copy)]
801+
enum EqualsOperandConversion {
802+
JsonbCast,
803+
TextCast,
804+
ToJsonb,
805+
Identity,
806+
}
807+
808+
impl EqualsOperandConversion {
809+
fn infer(this: &Expression<'_>, other: &Expression<'_>) -> Self {
810+
// If we reference a JSON list column, we have to convert it to a JSON array
811+
// (rather than a PostgreSQL list) before comparing it to a JSON value,
812+
// otherwise the comparison would fail with a type error.
813+
if other.is_json_value() && this.as_column().is_some_and(|c| c.is_list) {
814+
Self::ToJsonb
815+
} else if other.is_json_value() {
816+
Self::JsonbCast
817+
} else if other.is_xml_value() {
818+
Self::TextCast
819+
} else {
820+
Self::Identity
821+
}
822+
}
823+
824+
fn write<'a>(self, expr: Expression<'a>, v: &mut Postgres<'a>) -> visitor::Result {
825+
match self {
826+
Self::JsonbCast => {
827+
v.visit_expression(expr)?;
828+
v.write("::jsonb")
829+
}
830+
Self::TextCast => {
831+
v.visit_expression(expr)?;
832+
v.write("::text")
833+
}
834+
Self::ToJsonb => v.surround_with("TO_JSONB(", ")", |v| v.visit_expression(expr)),
835+
Self::Identity => v.visit_expression(expr),
836+
}
837+
}
838+
}
839+
822840
#[cfg(test)]
823841
mod tests {
824842
use crate::visitor::*;

query-compiler/core/src/query_document/parser.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,22 @@ impl QueryDocumentParser {
717717
)
718718
};
719719

720+
if placeholder.r#type == PrismaValueType::Json
721+
&& matches!(element_input_type, InputType::Scalar(ScalarType::Json))
722+
{
723+
// Allow handling a JSON placeholder as a list. This is needed to support the equality
724+
// operator for JSON arrays.
725+
// JSON array arguments are treated as scalars rather than arrays by default, since the
726+
// code that infers their type cannot differentiate between a value that's meant to be
727+
// a database-level array versus a JSON-level array. This would normally lead to a type
728+
// mismatch, hence this workaround is needed.
729+
return Ok(ParsedInputValue::Single(
730+
placeholder
731+
.with_type(PrismaValueType::List(PrismaValueType::Json.into()))
732+
.into(),
733+
));
734+
}
735+
720736
let PrismaValueType::List(inner_type) = &placeholder.r#type else {
721737
return Err(error());
722738
};

query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/filters/list_filters.rs

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -900,33 +900,32 @@ mod json_lists {
900900
schema.to_owned()
901901
}
902902

903-
// Uncomment this test when TML-2177 is fixed
904-
// #[connector_test]
905-
// async fn equality(runner: Runner) -> TestResult<()> {
906-
// test_data(&runner).await?;
907-
908-
// // equals json
909-
// insta::assert_snapshot!(
910-
// list_query(&runner, "json", "equals", r#"["{}", "{\"int\":5}", "[1, 2, 3]"]"#).await?,
911-
// @r###"{"data":{"findManyTestModel":[{"id":1}]}}"###
912-
// );
913-
// insta::assert_snapshot!(
914-
// list_query(&runner, "json", "equals", r#"["null", "\"test\""]"#).await?,
915-
// @r###"{"data":{"findManyTestModel":[{"id":3}]}}"###
916-
// );
917-
918-
// // NOT equals json
919-
// insta::assert_snapshot!(
920-
// not_list_query(&runner, "json", "equals", r#"["{}", "{\"int\":5}", "[1, 2, 3]"]"#).await?,
921-
// @r###"{"data":{"findManyTestModel":[{"id":2},{"id":3}]}}"###
922-
// );
923-
// insta::assert_snapshot!(
924-
// not_list_query(&runner, "json", "equals", r#"["null", "\"test\""]"#).await?,
925-
// @r###"{"data":{"findManyTestModel":[{"id":1},{"id":2}]}}"###
926-
// );
927-
928-
// Ok(())
929-
// }
903+
#[connector_test]
904+
async fn equality(runner: Runner) -> TestResult<()> {
905+
test_data(&runner).await?;
906+
907+
// equals json
908+
insta::assert_snapshot!(
909+
list_query(&runner, "json", "equals", r#"["{}", "{\"int\":5}", "[1, 2, 3]"]"#).await?,
910+
@r###"{"data":{"findManyTestModel":[{"id":1}]}}"###
911+
);
912+
insta::assert_snapshot!(
913+
list_query(&runner, "json", "equals", r#"["null", "\"test\""]"#).await?,
914+
@r###"{"data":{"findManyTestModel":[{"id":3}]}}"###
915+
);
916+
917+
// NOT equals json
918+
insta::assert_snapshot!(
919+
not_list_query(&runner, "json", "equals", r#"["{}", "{\"int\":5}", "[1, 2, 3]"]"#).await?,
920+
@r###"{"data":{"findManyTestModel":[{"id":2},{"id":3}]}}"###
921+
);
922+
insta::assert_snapshot!(
923+
not_list_query(&runner, "json", "equals", r#"["null", "\"test\""]"#).await?,
924+
@r###"{"data":{"findManyTestModel":[{"id":1},{"id":2}]}}"###
925+
);
926+
927+
Ok(())
928+
}
930929

931930
#[connector_test]
932931
async fn has(runner: Runner) -> TestResult<()> {

0 commit comments

Comments
 (0)