Skip to content

Commit

Permalink
driver-adapters: Serialize i32 arguments as number (#4797)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
Serhii Tatarintsev and jkomyno authored Mar 28, 2024
1 parent 80622dd commit 12fad47
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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?;

Expand All @@ -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?;

Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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?;

Expand All @@ -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?;

Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions query-engine/driver-adapters/src/conversion/js_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8>),
Array(Vec<JSArg>),
Expand Down
1 change: 1 addition & 0 deletions query-engine/driver-adapters/src/conversion/mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub fn value_to_js_arg(value: &quaint::Value) -> serde_json::Result<JSArg> {
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
Expand Down
1 change: 1 addition & 0 deletions query-engine/driver-adapters/src/conversion/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub fn value_to_js_arg(value: &quaint::Value) -> serde_json::Result<JSArg> {
(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
Expand Down
1 change: 1 addition & 0 deletions query-engine/driver-adapters/src/conversion/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub fn value_to_js_arg(value: &quaint::Value) -> serde_json::Result<JSArg> {
},
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()
Expand Down
1 change: 1 addition & 0 deletions query-engine/driver-adapters/src/napi/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<napi::sys::napi_value> {
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);
Expand Down
1 change: 1 addition & 0 deletions query-engine/driver-adapters/src/wasm/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ impl ToJsValue for Query {
impl ToJsValue for JSArg {
fn to_js_value(&self) -> Result<wasm_bindgen::prelude::JsValue, wasm_bindgen::prelude::JsValue> {
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());
Expand Down

0 comments on commit 12fad47

Please sign in to comment.