From 12fad4795eef0c21ed444646215f274961d99cf9 Mon Sep 17 00:00:00 2001 From: Serhii Tatarintsev Date: Thu, 28 Mar 2024 02:22:34 +0100 Subject: [PATCH] driver-adapters: Serialize i32 arguments as `number` (#4797) * driver-adapters: Serialize i32 arguments as `number` Before this PR, we serialized all integers as bigint. This was not a problem for any of the adapters that supported bigint natively, however, it turned into a problem for D1. When we are doing order by aggregations, we inserd `ORDER BY COALESCE(count, 0)` into the query, where `0` is passed as i32 argument. As mentioned earlier, i32 argument got converted to `bigint` before this PR. In D1 adapter, we convert all bigints to strings. Which means, that above SQL query would become `ORDER BY COALESCE(count, '0')` now and produce different order for the rows where `count = NULL`. Since i32 bounds are below `Number.MAX_SAFE_INTEGER`, it is safe to convert it to `number` instead of `bigint`. `0` in above query is hardcoded to always be `i32`, so this fixes the issue. Close prisma/team-orm#1049 * fix(adapter-d1): uncomment remaining tests that now work --------- Co-authored-by: jkomyno --- .../order_by_aggregation.rs | 57 +++---------------- .../driver-adapters/src/conversion/js_arg.rs | 1 + .../driver-adapters/src/conversion/mysql.rs | 1 + .../src/conversion/postgres.rs | 1 + .../driver-adapters/src/conversion/sqlite.rs | 1 + .../driver-adapters/src/napi/conversion.rs | 1 + .../driver-adapters/src/wasm/conversion.rs | 1 + 7 files changed, 13 insertions(+), 50 deletions(-) diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs index 744d26e7b565..3c94dd50d30c 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs @@ -33,13 +33,7 @@ mod order_by_aggr { schema.to_owned() } - #[connector_test(exclude(Sqlite("cfd1")))] - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyUser":[{"id":3,"posts":[]},{"id":1,"posts":[{"title":"alice_post_1"}]},{"id":2,"posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]}]}} - // + {"data":{"findManyUser":[{"id":1,"posts":[{"title":"alice_post_1"}]},{"id":2,"posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]},{"id":3,"posts":[]}]}} - // ``` + #[connector_test] async fn one2m_count_asc(runner: Runner) -> TestResult<()> { create_test_data(&runner).await?; @@ -58,13 +52,7 @@ mod order_by_aggr { Ok(()) } - #[connector_test(exclude(Sqlite("cfd1")))] - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyUser":[{"id":2,"posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]},{"id":1,"posts":[{"title":"alice_post_1"}]},{"id":3,"posts":[]}]}} - // + {"data":{"findManyUser":[{"id":3,"posts":[]},{"id":2,"posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]},{"id":1,"posts":[{"title":"alice_post_1"}]}]}} - // ``` + #[connector_test] async fn one2m_count_desc(runner: Runner) -> TestResult<()> { create_test_data(&runner).await?; @@ -121,13 +109,7 @@ mod order_by_aggr { Ok(()) } - #[connector_test(exclude(Sqlite("cfd1")))] - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyUser":[{"id":3,"name":"Motongo","posts":[]},{"id":1,"name":"Alice","posts":[{"title":"alice_post_1"}]},{"id":2,"name":"Bob","posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]}]}} - // + {"data":{"findManyUser":[{"id":1,"name":"Alice","posts":[{"title":"alice_post_1"}]},{"id":2,"name":"Bob","posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]},{"id":3,"name":"Motongo","posts":[]}]}} - // ``` + #[connector_test] async fn one2m_count_asc_field_asc(runner: Runner) -> TestResult<()> { create_test_data(&runner).await?; @@ -456,13 +438,7 @@ mod order_by_aggr { // With pagination tests // "[Cursor] Ordering by one2m count asc" should "work" - #[connector_test(exclude(Sqlite("cfd1")))] - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyUser":[{"id":1,"posts":[{"id":1}]},{"id":2,"posts":[{"id":2},{"id":3}]}]}} - // + {"data":{"findManyUser":[{"id":1,"posts":[{"id":1}]},{"id":2,"posts":[{"id":2},{"id":3}]},{"id":3,"posts":[]}]}} - // ``` + #[connector_test] async fn cursor_one2m_count_asc(runner: Runner) -> TestResult<()> { create_test_data(&runner).await?; @@ -482,13 +458,7 @@ mod order_by_aggr { } // "[Cursor] Ordering by one2m count desc" should "work" - #[connector_test(exclude(Sqlite("cfd1")))] - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyUser":[{"id":1,"posts":[{"id":1}]},{"id":2,"posts":[{"id":2},{"id":3}]}]}} - // + {"data":{"findManyUser":[{"id":1,"posts":[{"id":1}]},{"id":2,"posts":[{"id":2},{"id":3}]},{"id":3,"posts":[]}]}} - // ``` + #[connector_test] async fn cursor_one2m_count_desc(runner: Runner) -> TestResult<()> { create_test_data(&runner).await?; @@ -550,13 +520,7 @@ mod order_by_aggr { } // "[Cursor][Combo] Ordering by one2m count asc + field asc" - #[connector_test(exclude(Sqlite("cfd1")))] - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyUser":[{"id":2,"name":"Bob","posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]}]}} - // + {"data":{"findManyUser":[{"id":2,"name":"Bob","posts":[{"title":"bob_post_1"},{"title":"bob_post_2"}]},{"id":3,"name":"Motongo","posts":[]}]}} - // ``` + #[connector_test] async fn cursor_one2m_count_asc_field_asc(runner: Runner) -> TestResult<()> { create_test_data(&runner).await?; @@ -805,14 +769,7 @@ mod order_by_aggr { schema.to_owned() } - #[connector_test(schema(schema_regression_8036), exclude(Sqlite("cfd1")))] - // Regression test for: // https://github.com/prisma/prisma/issues/8036 - // On D1, this fails with: - // - // ```diff - // - {"data":{"findManyPost":[{"id":2,"title":"Second","_count":{"LikedPeople":0}},{"id":3,"title":"Third","_count":{"LikedPeople":0}},{"id":4,"title":"Fourth","_count":{"LikedPeople":0}},{"id":5,"title":"Fifth","_count":{"LikedPeople":0}}]}} - // + {"data":{"findManyPost":[]}} - // ``` + #[connector_test(schema(schema_regression_8036))] async fn count_m2m_records_not_connected(runner: Runner) -> TestResult<()> { run_query!( runner, diff --git a/query-engine/driver-adapters/src/conversion/js_arg.rs b/query-engine/driver-adapters/src/conversion/js_arg.rs index d6f67ed7716d..6521829bd274 100644 --- a/query-engine/driver-adapters/src/conversion/js_arg.rs +++ b/query-engine/driver-adapters/src/conversion/js_arg.rs @@ -2,6 +2,7 @@ use serde_json::value::Value as JsonValue; #[derive(Debug, PartialEq)] pub enum JSArg { + SafeInt(i32), Value(serde_json::Value), Buffer(Vec), Array(Vec), diff --git a/query-engine/driver-adapters/src/conversion/mysql.rs b/query-engine/driver-adapters/src/conversion/mysql.rs index bd59d3b94ed0..08704b06bccf 100644 --- a/query-engine/driver-adapters/src/conversion/mysql.rs +++ b/query-engine/driver-adapters/src/conversion/mysql.rs @@ -13,6 +13,7 @@ pub fn value_to_js_arg(value: &quaint::Value) -> serde_json::Result { quaint::ValueType::Bytes(Some(bytes)) => JSArg::Buffer(bytes.to_vec()), quaint::ValueType::Date(Some(d)) => JSArg::Value(JsonValue::String(d.format(DATE_FORMAT).to_string())), quaint::ValueType::DateTime(Some(dt)) => JSArg::Value(JsonValue::String(dt.format(DATETIME_FORMAT).to_string())), + quaint::ValueType::Int32(Some(value)) => JSArg::SafeInt(*value), quaint::ValueType::Time(Some(t)) => JSArg::Value(JsonValue::String(t.format(TIME_FORMAT).to_string())), quaint::ValueType::Array(Some(ref items)) => JSArg::Array( items diff --git a/query-engine/driver-adapters/src/conversion/postgres.rs b/query-engine/driver-adapters/src/conversion/postgres.rs index 949cc17e9eba..524834111bce 100644 --- a/query-engine/driver-adapters/src/conversion/postgres.rs +++ b/query-engine/driver-adapters/src/conversion/postgres.rs @@ -14,6 +14,7 @@ pub fn value_to_js_arg(value: &quaint::Value) -> serde_json::Result { (quaint::ValueType::DateTime(Some(dt)), _) => JSArg::Value(JsonValue::String(dt.naive_utc().to_string())), (quaint::ValueType::Json(Some(s)), _) => JSArg::Value(JsonValue::String(serde_json::to_string(s)?)), (quaint::ValueType::Bytes(Some(bytes)), _) => JSArg::Buffer(bytes.to_vec()), + (quaint::ValueType::Int32(Some(value)), _) => JSArg::SafeInt(*value), (quaint::ValueType::Numeric(Some(bd)), _) => JSArg::Value(JsonValue::String(bd.to_string())), (quaint::ValueType::Array(Some(items)), _) => JSArg::Array( items diff --git a/query-engine/driver-adapters/src/conversion/sqlite.rs b/query-engine/driver-adapters/src/conversion/sqlite.rs index b11acdca0d7f..af070ec0b2cd 100644 --- a/query-engine/driver-adapters/src/conversion/sqlite.rs +++ b/query-engine/driver-adapters/src/conversion/sqlite.rs @@ -9,6 +9,7 @@ pub fn value_to_js_arg(value: &quaint::Value) -> serde_json::Result { }, quaint::ValueType::Json(Some(s)) => JSArg::Value(s.to_owned()), quaint::ValueType::Bytes(Some(bytes)) => JSArg::Buffer(bytes.to_vec()), + quaint::ValueType::Int32(Some(value)) => JSArg::SafeInt(*value), quaint::ValueType::Array(Some(ref items)) => JSArg::Array( items .iter() diff --git a/query-engine/driver-adapters/src/napi/conversion.rs b/query-engine/driver-adapters/src/napi/conversion.rs index 6cfe445925e3..2fab5a28bb73 100644 --- a/query-engine/driver-adapters/src/napi/conversion.rs +++ b/query-engine/driver-adapters/src/napi/conversion.rs @@ -16,6 +16,7 @@ impl FromNapiValue for JSArg { impl ToNapiValue for JSArg { unsafe fn to_napi_value(env: napi::sys::napi_env, value: Self) -> napi::Result { match value { + JSArg::SafeInt(v) => ToNapiValue::to_napi_value(env, v), JSArg::Value(v) => ToNapiValue::to_napi_value(env, v), JSArg::Buffer(bytes) => { let env = napi::Env::from_raw(env); diff --git a/query-engine/driver-adapters/src/wasm/conversion.rs b/query-engine/driver-adapters/src/wasm/conversion.rs index 73e6a7c30331..d2039210a626 100644 --- a/query-engine/driver-adapters/src/wasm/conversion.rs +++ b/query-engine/driver-adapters/src/wasm/conversion.rs @@ -24,6 +24,7 @@ impl ToJsValue for Query { impl ToJsValue for JSArg { fn to_js_value(&self) -> Result { match self { + JSArg::SafeInt(num) => Ok(JsValue::from(*num)), JSArg::Value(value) => serde_serialize(value), JSArg::Buffer(buf) => { let array = Uint8Array::from(buf.as_slice());