From 09cb2c4b7f5ee2525bb7512bf48fc31167eebdf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Tue, 27 Feb 2024 07:55:03 +0100 Subject: [PATCH] bugfix(qe): Push to scalar list unless the field is an enum in CDB (#4750) * Push except if the field is an enum in cockroachDB --- .../src/datamodel_connector/capabilities.rs | 2 +- .../writes/data_types/scalar_list/base.rs | 47 ++++++++++++++----- .../fields/data_input_mapper/update.rs | 6 ++- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/psl/psl-core/src/datamodel_connector/capabilities.rs b/psl/psl-core/src/datamodel_connector/capabilities.rs index 9b3fe025d8ca..912a67cb2755 100644 --- a/psl/psl-core/src/datamodel_connector/capabilities.rs +++ b/psl/psl-core/src/datamodel_connector/capabilities.rs @@ -70,7 +70,7 @@ capabilities!( MultipleFullTextAttributesPerModel, ClusteringSetting, // Start of query-engine-only Capabilities - EnumArrayPush, + EnumArrayPush, // implies the ScalarList capability. Necessary, as CockroachDB supports pushing to a list of scalars, but not to the particular case of an enum list. See https://github.com/cockroachdb/cockroach/issues/71388 InsensitiveFilters, CreateMany, CreateManyWriteableAutoIncId, diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/writes/data_types/scalar_list/base.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/writes/data_types/scalar_list/base.rs index 9a5e74dd8547..0be9a67d2283 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/writes/data_types/scalar_list/base.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/writes/data_types/scalar_list/base.rs @@ -252,9 +252,38 @@ mod basic_types { Ok(()) } - // "An Update Mutation that pushes to some empty scalar lists" should "work" // Skipped for CockroachDB as enum array concatenation is not supported (https://github.com/cockroachdb/cockroach/issues/71388). #[connector_test(exclude(CockroachDb))] + async fn update_mut_push_empty_enum_array(runner: Runner) -> TestResult<()> { + create_row(&runner, r#"{ id: 1 }"#).await?; + create_row(&runner, r#"{ id: 2 }"#).await?; + + insta::assert_snapshot!( + run_query!(&runner, r#"mutation { + updateOneScalarModel(where: { id: 1 }, data: { + enums: { push: A } + }) { + enums + } + }"#), + @r###"{"data":{"updateOneScalarModel":{"enums":["A"]}}}"### + ); + + insta::assert_snapshot!( + run_query!(&runner, r#"mutation { + updateOneScalarModel(where: { id: 2 }, data: { + enums: { push: [A, B] } + }) { + enums + } + }"#), + @r###"{"data":{"updateOneScalarModel":{"enums":["A","B"]}}}"### + ); + + Ok(()) + } + + #[connector_test] async fn update_mut_push_empty_scalar_list(runner: Runner) -> TestResult<()> { create_row(&runner, r#"{ id: 1 }"#).await?; create_row(&runner, r#"{ id: 2 }"#).await?; @@ -265,21 +294,19 @@ mod basic_types { strings: { push: "future" } ints: { push: 15 } floats: { push: 2 } - booleans: { push: true } - enums: { push: A } + booleans: { push: true } dateTimes: { push: "2019-07-31T23:59:01.000Z" } bytes: { push: "dGVzdA==" } }) { strings ints floats - booleans - enums + booleans dateTimes bytes } }"#), - @r###"{"data":{"updateOneScalarModel":{"strings":["future"],"ints":[15],"floats":[2.0],"booleans":[true],"enums":["A"],"dateTimes":["2019-07-31T23:59:01.000Z"],"bytes":["dGVzdA=="]}}}"### + @r###"{"data":{"updateOneScalarModel":{"strings":["future"],"ints":[15],"floats":[2.0],"booleans":[true],"dateTimes":["2019-07-31T23:59:01.000Z"],"bytes":["dGVzdA=="]}}}"### ); insta::assert_snapshot!( @@ -288,21 +315,19 @@ mod basic_types { strings: { push: ["present", "future"] } ints: { push: [14, 15] } floats: { push: [1, 2] } - booleans: { push: [false, true] } - enums: { push: [A, B] } + booleans: { push: [false, true] } dateTimes: { push: ["2019-07-31T23:59:01.000Z", "2019-07-31T23:59:02.000Z"] } bytes: { push: ["dGVzdA==", "dGVzdA=="] } }) { strings ints floats - booleans - enums + booleans dateTimes bytes } }"#), - @r###"{"data":{"updateOneScalarModel":{"strings":["present","future"],"ints":[14,15],"floats":[1.0,2.0],"booleans":[false,true],"enums":["A","B"],"dateTimes":["2019-07-31T23:59:01.000Z","2019-07-31T23:59:02.000Z"],"bytes":["dGVzdA==","dGVzdA=="]}}}"### + @r###"{"data":{"updateOneScalarModel":{"strings":["present","future"],"ints":[14,15],"floats":[1.0,2.0],"booleans":[false,true],"dateTimes":["2019-07-31T23:59:01.000Z","2019-07-31T23:59:02.000Z"],"bytes":["dGVzdA==","dGVzdA=="]}}}"### ); Ok(()) diff --git a/query-engine/schema/src/build/input_types/fields/data_input_mapper/update.rs b/query-engine/schema/src/build/input_types/fields/data_input_mapper/update.rs index e6f051b70586..3ba74be76f2d 100644 --- a/query-engine/schema/src/build/input_types/fields/data_input_mapper/update.rs +++ b/query-engine/schema/src/build/input_types/fields/data_input_mapper/update.rs @@ -75,13 +75,15 @@ impl DataInputFieldMapper for UpdateDataInputFieldMapper { let mut input_object = input_object_type(ident, move || { let mut object_fields = vec![simple_input_field(operations::SET, list_input_type.clone(), None).optional()]; - // Todo this capability looks wrong to me. - if ctx.has_capability(ConnectorCapability::EnumArrayPush) { + if ctx.has_capability(ConnectorCapability::ScalarLists) + && (ctx.has_capability(ConnectorCapability::EnumArrayPush) || !type_identifier.is_enum()) + { let map_scalar_type = map_scalar_input_type(ctx, type_identifier, false); object_fields.push( input_field(operations::PUSH, vec![map_scalar_type, list_input_type.clone()], None).optional(), ) } + object_fields }); input_object.require_exactly_one_field();