From 192cb5a18a6554ad56caf5d8106f0d6c84e1168f Mon Sep 17 00:00:00 2001 From: Flavian Desverne Date: Thu, 28 Mar 2024 15:51:05 +0100 Subject: [PATCH 01/11] fix(d1): allow custom max_bind_values --- Cargo.toml | 2 +- quaint/src/connector/connection_info.rs | 15 +++++++++++- quaint/src/connector/external.rs | 4 +++- .../tests/new/regressions/prisma_15607.rs | 11 ++++++++- .../tests/new/regressions/prisma_7434.rs | 2 +- .../query-tests-setup/src/config.rs | 24 ++++++++++++++++++- .../src/connector_tag/mod.rs | 21 +++------------- .../query-tests-setup/src/lib.rs | 3 ++- .../query-tests-setup/src/runner/mod.rs | 17 +++++++++---- .../test-configs/cloudflare-d1 | 3 ++- .../sql-query-connector/src/context.rs | 5 ++-- query-engine/driver-adapters/src/queryable.rs | 1 + query-engine/driver-adapters/src/types.rs | 15 ++++++++---- 13 files changed, 85 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2a3077fbf0eb..1403197c3e8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,7 +71,7 @@ tracing = { version = "0.1" } tsify = { version = "0.4.5" } wasm-bindgen = { version = "0.2.92" } wasm-bindgen-futures = { version = "0.4" } -wasm-rs-dbg = { version = "0.1.2" } +wasm-rs-dbg = { version = "0.1.2", default-features = false, features = ["console-error"] } wasm-bindgen-test = { version = "0.3.0" } url = { version = "2.5.0" } diff --git a/quaint/src/connector/connection_info.rs b/quaint/src/connector/connection_info.rs index cbf0aaeb5bbe..7dd8a5b58257 100644 --- a/quaint/src/connector/connection_info.rs +++ b/quaint/src/connector/connection_info.rs @@ -190,6 +190,19 @@ impl ConnectionInfo { } } + pub fn max_insert_rows(&self) -> Option { + self.sql_family().max_insert_rows() + } + + pub fn max_bind_values(&self) -> usize { + match self { + #[cfg(not(target_arch = "wasm32"))] + ConnectionInfo::Native(_) => self.sql_family().max_bind_values(), + // Wasm connectors can override the default max bind values. + ConnectionInfo::External(info) => info.max_bind_values.unwrap_or(self.sql_family().max_bind_values()), + } + } + /// The family of databases connected. pub fn sql_family(&self) -> SqlFamily { match self { @@ -316,7 +329,7 @@ impl SqlFamily { } /// Get the default max rows for a batch insert. - pub fn max_insert_rows(&self) -> Option { + pub(crate) fn max_insert_rows(&self) -> Option { match self { #[cfg(feature = "postgresql")] SqlFamily::Postgres => None, diff --git a/quaint/src/connector/external.rs b/quaint/src/connector/external.rs index 51d341d8d9c4..92423363fcc8 100644 --- a/quaint/src/connector/external.rs +++ b/quaint/src/connector/external.rs @@ -6,13 +6,15 @@ use super::{SqlFamily, TransactionCapable}; pub struct ExternalConnectionInfo { pub sql_family: SqlFamily, pub schema_name: String, + pub max_bind_values: Option, } impl ExternalConnectionInfo { - pub fn new(sql_family: SqlFamily, schema_name: String) -> Self { + pub fn new(sql_family: SqlFamily, schema_name: String, max_bind_values: Option) -> Self { ExternalConnectionInfo { sql_family, schema_name, + max_bind_values, } } } diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs index a4b072256ec0..d46fda36abbc 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs @@ -73,7 +73,16 @@ impl Actor { Some("READ COMMITTED"), ); - let mut runner = Runner::load(datamodel, &[], version, tag, setup_metrics(), log_capture).await?; + let mut runner = Runner::load( + datamodel, + &[], + version, + tag, + CONFIG.max_bind_values(), + setup_metrics(), + log_capture, + ) + .await?; tokio::spawn(async move { while let Some(message) = query_receiver.recv().await { diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs index 166e9e1e4a94..61788cd24e40 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs @@ -1,6 +1,6 @@ use query_engine_tests::*; -#[test_suite(schema(autoinc_id), capabilities(CreateMany, AutoIncrement), exclude(CockroachDb))] +#[test_suite(schema(autoinc_id), capabilities(AutoIncrement), exclude(CockroachDb))] mod not_in_chunking { use query_engine_tests::Runner; diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/config.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/config.rs index 5566fe3d717e..f287f3e4782b 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/config.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/config.rs @@ -69,6 +69,9 @@ pub struct TestConfigFromSerde { /// This is the URL to the mobile emulator which will execute the queries against /// the instances of the engine running on the device. pub(crate) mobile_emulator_url: Option, + + /// The maximum number of bind values to use in a query for a driver adapter test runner. + pub(crate) driver_adapter_max_bind_values: Option, } impl TestConfigFromSerde { @@ -156,6 +159,9 @@ pub(crate) struct WithDriverAdapter { /// The driver adapter configuration to forward as a stringified JSON object to the external /// test executor by setting the `DRIVER_ADAPTER_CONFIG` env var when spawning the executor. pub(crate) config: Option, + + /// The maximum number of bind values to use in a query for a driver adapter test runner. + pub(crate) max_bind_values: Option, } impl WithDriverAdapter { @@ -181,6 +187,7 @@ impl From for TestConfig { adapter, test_executor: config.external_test_executor.unwrap(), config: config.driver_adapter_config, + max_bind_values: config.driver_adapter_max_bind_values, }), None => None, }; @@ -295,6 +302,9 @@ impl TestConfig { let driver_adapter_config = std::env::var("DRIVER_ADAPTER_CONFIG") .map(|config| serde_json::from_str::(config.as_str()).ok()) .unwrap_or_default(); + let driver_adapter_max_bind_values = std::env::var("DRIVER_ADAPTER_MAX_BIND_VALUES") + .ok() + .map(|v| v.parse::().unwrap()); let mobile_emulator_url = std::env::var("MOBILE_EMULATOR_URL").ok(); @@ -310,6 +320,7 @@ impl TestConfig { driver_adapter, driver_adapter_config, mobile_emulator_url, + driver_adapter_max_bind_values, }) .map(Self::from) } @@ -387,7 +398,7 @@ impl TestConfig { } pub fn test_connector(&self) -> TestResult<(ConnectorTag, ConnectorVersion)> { - let version = ConnectorVersion::try_from((self.connector(), self.connector_version()))?; + let version = self.parse_connector_version()?; let tag = match version { ConnectorVersion::SqlServer(_) => &SqlServerConnectorTag as ConnectorTag, ConnectorVersion::Postgres(_) => &PostgresConnectorTag, @@ -401,6 +412,17 @@ impl TestConfig { Ok((tag, version)) } + pub fn max_bind_values(&self) -> Option { + let version = self.parse_connector_version().unwrap(); + let local_mbv = self.with_driver_adapter().and_then(|config| config.max_bind_values); + + local_mbv.or_else(|| version.max_bind_values()) + } + + fn parse_connector_version(&self) -> TestResult { + ConnectorVersion::try_from((self.connector(), self.connector_version())) + } + #[rustfmt::skip] pub fn for_external_executor(&self) -> Vec<(String, String)> { let with_driver_adapter = self.with_driver_adapter().unwrap(); diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs index 247a2da60f7f..e18341e495a2 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs @@ -290,19 +290,15 @@ impl ConnectorVersion { /// From the PoV of the test binary, the target architecture is that of where the test runs, /// generally x86_64, or aarch64, etc. /// - /// As a consequence there is an mismatch between the the max_bind_values as seen by the test + /// As a consequence there is a mismatch between the max_bind_values as seen by the test /// binary (overriden by the QUERY_BATCH_SIZE env var) and the max_bind_values as seen by the /// WASM engine being exercised in those tests, through the RunnerExecutor::External test runner. /// - /// What we do in here, is returning the number of max_bind_values hat the connector under test + /// What we do in here, is returning the number of max_bind_values that the connector under test /// will use. i.e. if it's a WASM connector, the default, not overridable one. Otherwise the one /// as seen by the test binary (which will be the same as the engine exercised) pub fn max_bind_values(&self) -> Option { - if self.is_wasm() { - self.sql_family().map(|f| f.default_max_bind_values()) - } else { - self.sql_family().map(|f| f.max_bind_values()) - } + self.sql_family().map(|f| f.max_bind_values()) } /// SQL family for the connector @@ -317,17 +313,6 @@ impl ConnectorVersion { _ => None, } } - - /// Determines if the connector uses a driver adapter implemented in Wasm - fn is_wasm(&self) -> bool { - matches!( - self, - Self::Postgres(Some(PostgresVersion::PgJsWasm)) - | Self::Postgres(Some(PostgresVersion::NeonJsWasm)) - | Self::Vitess(Some(VitessVersion::PlanetscaleJsWasm)) - | Self::Sqlite(Some(SqliteVersion::LibsqlJsWasm)) - ) - } } impl fmt::Display for ConnectorVersion { diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs index de7020ce3ec4..9e3aeb20ade0 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs @@ -168,7 +168,7 @@ fn run_relation_link_test_impl( run_with_tokio( async move { println!("Used datamodel:\n {}", datamodel.yellow()); - let runner = Runner::load(datamodel.clone(), &[], version, connector_tag, metrics, log_capture) + let runner = Runner::load(datamodel.clone(), &[], version, connector_tag, CONFIG.max_bind_values(), metrics, log_capture) .await .unwrap(); @@ -286,6 +286,7 @@ fn run_connector_test_impl( db_schemas, version, connector_tag, + CONFIG.max_bind_values(), metrics, log_capture, ) diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs index a8db6206d070..8b7fb3c38f07 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs @@ -207,6 +207,9 @@ pub struct Runner { metrics: MetricRegistry, protocol: EngineProtocol, log_capture: TestLogCapture, + // This is a local override for the max bind values that can be used in a query. + // It is set in the test config files, specifically for D1 for now, which has a lower limit than the SQLite native connector. + local_max_bind_values: Option, } impl Runner { @@ -222,7 +225,8 @@ impl Runner { } pub fn max_bind_values(&self) -> Option { - self.connector_version().max_bind_values() + self.local_max_bind_values + .or_else(|| self.connector_version().max_bind_values()) } pub async fn load( @@ -230,6 +234,7 @@ impl Runner { db_schemas: &[&str], connector_version: ConnectorVersion, connector_tag: ConnectorTag, + local_max_bind_values: Option, metrics: MetricRegistry, log_capture: TestLogCapture, ) -> TestResult { @@ -241,12 +246,13 @@ impl Runner { let (executor, db_version) = match crate::CONFIG.with_driver_adapter() { Some(with_driver_adapter) => { let external_executor = ExternalExecutor::new(); - let external_initializer: ExternalExecutorInitializer<'_> = - external_executor.init(&datamodel, url.as_str()); - let executor = RunnerExecutor::External(external_executor); + + let external_initializer = external_executor.init(&datamodel, url.as_str()); qe_setup::setup_external(with_driver_adapter.adapter, external_initializer, db_schemas).await?; + let executor = RunnerExecutor::External(external_executor); + let database_version = None; (executor, database_version) } @@ -264,8 +270,8 @@ impl Runner { let connector = query_executor.primary_connector(); let conn = connector.get_connection().await.unwrap(); let database_version = conn.version().await; - let executor = RunnerExecutor::Builtin(query_executor); + (executor, database_version) } }; @@ -284,6 +290,7 @@ impl Runner { metrics, protocol, log_capture, + local_max_bind_values, }) } diff --git a/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 b/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 index 51f9a52edea3..75e3e594c688 100644 --- a/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 +++ b/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 @@ -2,5 +2,6 @@ "connector": "sqlite", "version": "cfd1", "driver_adapter": "d1", - "external_test_executor": "Wasm" + "external_test_executor": "Wasm", + "driver_adapter_max_bind_value": 100 } \ No newline at end of file diff --git a/query-engine/connectors/sql-query-connector/src/context.rs b/query-engine/connectors/sql-query-connector/src/context.rs index fbb9941f51c4..5b2887451ef1 100644 --- a/query-engine/connectors/sql-query-connector/src/context.rs +++ b/query-engine/connectors/sql-query-connector/src/context.rs @@ -13,9 +13,8 @@ pub(super) struct Context<'a> { impl<'a> Context<'a> { pub(crate) fn new(connection_info: &'a ConnectionInfo, trace_id: Option<&'a str>) -> Self { - let sql_family = connection_info.sql_family(); - let max_insert_rows = sql_family.max_insert_rows(); - let max_bind_values = sql_family.max_bind_values(); + let max_insert_rows = connection_info.max_insert_rows(); + let max_bind_values = connection_info.max_bind_values(); Context { connection_info, diff --git a/query-engine/driver-adapters/src/queryable.rs b/query-engine/driver-adapters/src/queryable.rs index 6250d137fbb6..278c4e4f514e 100644 --- a/query-engine/driver-adapters/src/queryable.rs +++ b/query-engine/driver-adapters/src/queryable.rs @@ -236,6 +236,7 @@ impl std::fmt::Debug for JsQueryable { impl ExternalConnector for JsQueryable { async fn get_connection_info(&self) -> quaint::Result { let conn_info = self.driver_proxy.get_connection_info().await?; + Ok(conn_info.into_external_connection_info(&self.inner.provider)) } } diff --git a/query-engine/driver-adapters/src/types.rs b/query-engine/driver-adapters/src/types.rs index 70ae20c10c52..1b4cbe531359 100644 --- a/query-engine/driver-adapters/src/types.rs +++ b/query-engine/driver-adapters/src/types.rs @@ -41,8 +41,8 @@ impl FromStr for AdapterFlavour { } } -impl From for SqlFamily { - fn from(value: AdapterFlavour) -> Self { +impl From<&AdapterFlavour> for SqlFamily { + fn from(value: &AdapterFlavour) -> Self { match value { #[cfg(feature = "mysql")] AdapterFlavour::Mysql => SqlFamily::Mysql, @@ -60,14 +60,21 @@ impl From for SqlFamily { #[derive(Default)] pub(crate) struct JsConnectionInfo { pub schema_name: Option, + pub max_bind_values: Option, } impl JsConnectionInfo { pub fn into_external_connection_info(self, provider: &AdapterFlavour) -> ExternalConnectionInfo { let schema_name = self.get_schema_name(provider); - let sql_family = provider.to_owned().into(); - ExternalConnectionInfo::new(sql_family, schema_name.to_owned()) + let sql_family = SqlFamily::from(provider); + + ExternalConnectionInfo::new( + sql_family, + schema_name.to_owned(), + self.max_bind_values.map(|v| v as usize), + ) } + fn get_schema_name(&self, provider: &AdapterFlavour) -> &str { match self.schema_name.as_ref() { Some(name) => name, From aa8bee642e190455965e39466960d684086a05ac Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:35:49 +0200 Subject: [PATCH 02/11] feat(driver-adapters-executor): allow reading "maxBindValues" from Driver Adapter --- .../driver-adapters/executor/src/testd.ts | 64 ++++++++++--------- .../driver-adapters/executor/src/types/env.ts | 21 ++++-- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/query-engine/driver-adapters/executor/src/testd.ts b/query-engine/driver-adapters/executor/src/testd.ts index a83df43d86d0..3171235c583b 100644 --- a/query-engine/driver-adapters/executor/src/testd.ts +++ b/query-engine/driver-adapters/executor/src/testd.ts @@ -8,7 +8,7 @@ import { import { webcrypto } from "node:crypto"; import type { DriverAdaptersManager } from "./driver-adapters-manager"; -import { jsonRpc, Env, ExternalTestExecutor } from "./types"; +import { jsonRpc, Env } from "./types"; import * as qe from "./qe"; import { PgManager } from "./driver-adapters-manager/pg"; import { NeonWsManager } from "./driver-adapters-manager/neon.ws"; @@ -119,10 +119,9 @@ async function handleRequest( env, migrationScript ); - const engineType = env.EXTERNAL_TEST_EXECUTOR ?? "Napi"; const { engine, adapter } = await initQe({ - engineType, + env, url, driverAdapterManager, schema, @@ -136,7 +135,15 @@ async function handleRequest( adapter, logs, }; - return null; + + if (adapter && adapter.getConnectionInfo) { + const maxBindValuesResult = adapter.getConnectionInfo().map(info => info.maxBindValues) + if (maxBindValuesResult.ok) { + return { maxBindValues: maxBindValuesResult.value } + } + } + + return { maxBindValues: null } } case "query": { debug("Got `query`", params); @@ -235,41 +242,40 @@ function respondOk(requestId: number, payload: unknown) { } type InitQueryEngineParams = { - engineType: ExternalTestExecutor; - driverAdapterManager: DriverAdaptersManager; - url: string; - schema: string; - logCallback: qe.QueryLogCallback; + env: Env + driverAdapterManager: DriverAdaptersManager + url: string + schema: string + logCallback: qe.QueryLogCallback }; async function initQe({ - engineType, + env, driverAdapterManager, url, schema, logCallback, }: InitQueryEngineParams) { - if (process.env.EXTERNAL_TEST_EXECUTOR === "Mobile") { - if (process.env.MOBILE_EMULATOR_URL) { - url = process.env.MOBILE_EMULATOR_URL; - } + if (env.EXTERNAL_TEST_EXECUTOR === 'Mobile') { + url = env.MOBILE_EMULATOR_URL; + const engine = createRNEngineConnector(url, schema, logCallback); return { engine, adapter: null }; - } else { - const adapter = await driverAdapterManager.connect({ url }); - const errorCapturingAdapter = bindAdapter(adapter); - const engineInstance = await qe.initQueryEngine( - engineType, - errorCapturingAdapter, - schema, - logCallback, - debug - ); - - return { - engine: engineInstance, - adapter: errorCapturingAdapter, - }; + } + + const adapter = await driverAdapterManager.connect({ url }) + const errorCapturingAdapter = bindAdapter(adapter) + const engineInstance = await qe.initQueryEngine( + env.EXTERNAL_TEST_EXECUTOR, + errorCapturingAdapter, + schema, + logCallback, + debug + ) + + return { + engine: engineInstance, + adapter: errorCapturingAdapter, } } diff --git a/query-engine/driver-adapters/executor/src/types/env.ts b/query-engine/driver-adapters/executor/src/types/env.ts index e80e1c87e0fc..b15bf092b47d 100644 --- a/query-engine/driver-adapters/executor/src/types/env.ts +++ b/query-engine/driver-adapters/executor/src/types/env.ts @@ -2,7 +2,7 @@ import * as S from '@effect/schema/Schema' const DriverAdapterConfig = S.struct({ proxy_url: S.string.pipe(S.nonEmpty({ - message: () => 'proxy_url must not be empty', + message: () => '`proxy_url` must not be empty', })), }) @@ -23,8 +23,14 @@ const EnvNeonWS = S.struct({ DRIVER_ADAPTER_CONFIG: DriverAdapterConfigFromString, }) +export const MobileAdapterConfig = S.struct({ + EXTERNAL_TEST_EXECUTOR: S.literal('Mobile'), + MOBILE_EMULATOR_URL: S.string.pipe(S.nonEmpty({ + message: () => '`MOBILE_EMULATOR_URL` must not be empty', + })), +}) + export const ExternalTestExecutor = S.literal('Wasm', 'Napi') -export type ExternalTestExecutor = S.Schema.Type export const Env = S.extend( S.union( @@ -34,9 +40,14 @@ export const Env = S.extend( DRIVER_ADAPTER: S.literal('pg', 'libsql', 'd1'), }), ), - S.struct({ - EXTERNAL_TEST_EXECUTOR: S.optional(ExternalTestExecutor), - }), + S.union( + MobileAdapterConfig, + S.struct({ + EXTERNAL_TEST_EXECUTOR: S.optional(ExternalTestExecutor, { + default: () => 'Napi', + }), + }), + ), ) export type Env = S.Schema.Type From 1291dd7ea8cb75b3d80413b5101c564e9c6fa4c4 Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:37:11 +0200 Subject: [PATCH 03/11] feat(connector-test-kit-rs): "ExternalInitializer::init*" methods now return "InitResult" --- .../connector-test-kit-rs/qe-setup/src/lib.rs | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/query-engine/connector-test-kit-rs/qe-setup/src/lib.rs b/query-engine/connector-test-kit-rs/qe-setup/src/lib.rs index 74b2d015f7df..18bcdd036fa3 100644 --- a/query-engine/connector-test-kit-rs/qe-setup/src/lib.rs +++ b/query-engine/connector-test-kit-rs/qe-setup/src/lib.rs @@ -21,15 +21,21 @@ use psl::{builtin_connectors::*, Datasource}; use schema_core::schema_connector::{ConnectorResult, DiffTarget, SchemaConnector}; use std::env; +#[derive(Debug, serde::Deserialize, PartialEq)] +pub struct InitResult { + pub max_bind_values: Option, +} + pub trait ExternalInitializer<'a> where Self: Sized, { #[allow(async_fn_in_trait)] - async fn init_with_migration(&self, script: String) -> Result<(), Box>; + async fn init_with_migration(&self, script: String) + -> Result>; #[allow(async_fn_in_trait)] - async fn init(&self) -> Result<(), Box>; + async fn init(&self) -> Result>; fn url(&self) -> &'a str; fn datamodel(&self) -> &'a str; @@ -63,38 +69,41 @@ pub async fn setup_external<'a, EI>( driver_adapter: DriverAdapter, initializer: EI, db_schemas: &[&str], -) -> ConnectorResult<()> +) -> ConnectorResult where EI: ExternalInitializer<'a> + ?Sized, { let prisma_schema = initializer.datamodel(); let (source, url, _preview_features) = parse_configuration(prisma_schema)?; - if driver_adapter == DriverAdapter::D1 { - // 1. Compute the diff migration script. - std::fs::remove_file(source.url.as_literal().unwrap().trim_start_matches("file:")).ok(); - let mut connector = sql_schema_connector::SqlSchemaConnector::new_sqlite(); - let migration_script = crate::diff(prisma_schema, url, &mut connector).await?; - - // 2. Tell JavaScript to take care of the schema migration. - // This results in a JSON-RPC call to the JS runtime. - // The JSON-RPC machinery is defined in the `[query-tests-setup]` crate, and it - // implements the `ExternalInitializer<'a>` trait. - initializer - .init_with_migration(migration_script) - .await - .map_err(|err| ConnectorError::from_msg(format!("Error migrating with D1 adapter: {}", err)))?; - } else { - setup(prisma_schema, db_schemas).await?; - - // 3. Tell JavaScript to initialize the external test session. - // The schema migration is taken care of by the Schema Engine. - initializer.init().await.map_err(|err| { - ConnectorError::from_msg(format!("Error initializing {} adapter: {}", driver_adapter, err)) - })?; - } + let init_result = match driver_adapter { + DriverAdapter::D1 => { + // 1. Compute the diff migration script. + std::fs::remove_file(source.url.as_literal().unwrap().trim_start_matches("file:")).ok(); + let mut connector = sql_schema_connector::SqlSchemaConnector::new_sqlite(); + let migration_script = crate::diff(prisma_schema, url, &mut connector).await?; + + // 2. Tell JavaScript to take care of the schema migration. + // This results in a JSON-RPC call to the JS runtime. + // The JSON-RPC machinery is defined in the `[query-tests-setup]` crate, and it + // implements the `ExternalInitializer<'a>` trait. + initializer + .init_with_migration(migration_script) + .await + .map_err(|err| ConnectorError::from_msg(format!("Error migrating with D1 adapter: {}", err))) + } + _ => { + setup(prisma_schema, db_schemas).await?; + + // 3. Tell JavaScript to initialize the external test session. + // The schema migration is taken care of by the Schema Engine. + initializer.init().await.map_err(|err| { + ConnectorError::from_msg(format!("Error initializing {} adapter: {}", driver_adapter, err)) + }) + } + }?; - Ok(()) + Ok(init_result) } /// Database setup for connector-test-kit-rs. From 1ccdfb189abd7c63c74e025a1953ee432d6cba6e Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:42:33 +0200 Subject: [PATCH 04/11] feat(connector-test-kit-rs): update "Runner::load" after "ExternalInitializer::init*" changes; add optional "override_local_max_bind_values" argument --- .../query-tests-setup/src/runner/mod.rs | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs index 8b7fb3c38f07..e2e7dd4f2567 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/mod.rs @@ -97,19 +97,19 @@ impl<'a> qe_setup::ExternalInitializer<'a> for ExternalExecutorInitializer<'a> { async fn init_with_migration( &self, migration_script: String, - ) -> Result<(), Box> { + ) -> Result> { let migration_script = Some(migration_script); - executor_process_request("initializeSchema", json!({ "schemaId": self.schema_id, "schema": self.schema, "url": self.url, "migrationScript": migration_script })).await?; - Ok(()) + let init_result = executor_process_request("initializeSchema", json!({ "schemaId": self.schema_id, "schema": self.schema, "url": self.url, "migrationScript": migration_script })).await?; + Ok(init_result) } - async fn init(&self) -> Result<(), Box> { - executor_process_request( + async fn init(&self) -> Result> { + let init_result = executor_process_request( "initializeSchema", json!({ "schemaId": self.schema_id, "schema": self.schema, "url": self.url }), ) .await?; - Ok(()) + Ok(init_result) } fn url(&self) -> &'a str { @@ -207,8 +207,6 @@ pub struct Runner { metrics: MetricRegistry, protocol: EngineProtocol, log_capture: TestLogCapture, - // This is a local override for the max bind values that can be used in a query. - // It is set in the test config files, specifically for D1 for now, which has a lower limit than the SQLite native connector. local_max_bind_values: Option, } @@ -234,7 +232,7 @@ impl Runner { db_schemas: &[&str], connector_version: ConnectorVersion, connector_tag: ConnectorTag, - local_max_bind_values: Option, + override_local_max_bind_values: Option, metrics: MetricRegistry, log_capture: TestLogCapture, ) -> TestResult { @@ -243,18 +241,19 @@ impl Runner { let datasource = schema.configuration.datasources.first().unwrap(); let url = datasource.load_url(|key| env::var(key).ok()).unwrap(); - let (executor, db_version) = match crate::CONFIG.with_driver_adapter() { + let (executor, db_version, init_external_result) = match crate::CONFIG.with_driver_adapter() { Some(with_driver_adapter) => { let external_executor = ExternalExecutor::new(); let external_initializer = external_executor.init(&datamodel, url.as_str()); - qe_setup::setup_external(with_driver_adapter.adapter, external_initializer, db_schemas).await?; + let init_external_result = + qe_setup::setup_external(with_driver_adapter.adapter, external_initializer, db_schemas).await?; + let database_version = None; let executor = RunnerExecutor::External(external_executor); - let database_version = None; - (executor, database_version) + (executor, database_version, Some(init_external_result)) } None => { qe_setup::setup(&datamodel, db_schemas).await?; @@ -272,10 +271,23 @@ impl Runner { let database_version = conn.version().await; let executor = RunnerExecutor::Builtin(query_executor); - (executor, database_version) + (executor, database_version, None) } }; + // If `override_local_max_bind_values` is provided, use that. + // Otherwise, if the external process has provided an `init_result`, use `init_result.max_bind_values`. + // Otherwise, use the connector's (Wasm-aware) default. + // + // Note: Use `override_local_max_bind_values` only for local testing purposes. + // If a feature requires a specific `max_bind_values` value for a Driver Adapter, it should be set in the + // TypeScript Driver Adapter implementation itself. + let local_max_bind_values = match (override_local_max_bind_values, init_external_result) { + (Some(override_max_bind_values), _) => Some(override_max_bind_values), + (_, Some(init_result)) => init_result.max_bind_values, + (_, None) => None, + }; + let query_schema = schema::build(Arc::new(schema), true).with_db_version_supports_join_strategy( relation_load_strategy::db_version_supports_joins_strategy(db_version)?, ); From 925f822e0683670add6d68592efe01df60200a98 Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:43:10 +0200 Subject: [PATCH 05/11] chore(connector-test-kit-rs): set optional "override_local_max_bind_values" argument to None when manually invoking "Runner::load()" --- .../tests/new/regressions/prisma_15607.rs | 11 +---------- .../query-tests-setup/src/lib.rs | 6 ++++-- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs index d46fda36abbc..e026a90016bd 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_15607.rs @@ -73,16 +73,7 @@ impl Actor { Some("READ COMMITTED"), ); - let mut runner = Runner::load( - datamodel, - &[], - version, - tag, - CONFIG.max_bind_values(), - setup_metrics(), - log_capture, - ) - .await?; + let mut runner = Runner::load(datamodel, &[], version, tag, None, setup_metrics(), log_capture).await?; tokio::spawn(async move { while let Some(message) = query_receiver.recv().await { diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs index 9e3aeb20ade0..b151244fafab 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs @@ -168,7 +168,8 @@ fn run_relation_link_test_impl( run_with_tokio( async move { println!("Used datamodel:\n {}", datamodel.yellow()); - let runner = Runner::load(datamodel.clone(), &[], version, connector_tag, CONFIG.max_bind_values(), metrics, log_capture) + let override_local_max_bind_values = None; + let runner = Runner::load(datamodel.clone(), &[], version, connector_tag, override_local_max_bind_values, metrics, log_capture) .await .unwrap(); @@ -281,12 +282,13 @@ fn run_connector_test_impl( crate::run_with_tokio( async { println!("Used datamodel:\n {}", datamodel.yellow()); + let override_local_max_bind_values = None; let runner = Runner::load( datamodel.clone(), db_schemas, version, connector_tag, - CONFIG.max_bind_values(), + override_local_max_bind_values, metrics, log_capture, ) From f8cc9ceb9077e033abc261a72cdeed8e088529eb Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:44:46 +0200 Subject: [PATCH 06/11] chore(connector-test-kit-rs): remove "driver_adapter_max_bind_value" from D1 --- query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 b/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 index 75e3e594c688..51f9a52edea3 100644 --- a/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 +++ b/query-engine/connector-test-kit-rs/test-configs/cloudflare-d1 @@ -2,6 +2,5 @@ "connector": "sqlite", "version": "cfd1", "driver_adapter": "d1", - "external_test_executor": "Wasm", - "driver_adapter_max_bind_value": 100 + "external_test_executor": "Wasm" } \ No newline at end of file From 89e5b8e938c7c00a5a53ac307b1850614d88be21 Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:45:16 +0200 Subject: [PATCH 07/11] fix(connector-test-kit-rs): restore "ConnectorVersion::is_wasm", add D1 to it --- .../src/connector_tag/mod.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs b/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs index e18341e495a2..7888aed0be03 100644 --- a/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs +++ b/query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/mod.rs @@ -298,7 +298,11 @@ impl ConnectorVersion { /// will use. i.e. if it's a WASM connector, the default, not overridable one. Otherwise the one /// as seen by the test binary (which will be the same as the engine exercised) pub fn max_bind_values(&self) -> Option { - self.sql_family().map(|f| f.max_bind_values()) + if self.is_wasm() { + self.sql_family().map(|f| f.default_max_bind_values()) + } else { + self.sql_family().map(|f| f.max_bind_values()) + } } /// SQL family for the connector @@ -313,6 +317,20 @@ impl ConnectorVersion { _ => None, } } + + /// Determines if the connector uses a driver adapter implemented in Wasm. + /// Do not delete! This is used because the `#[cfg(target_arch = "wasm32")]` conditional compilation + /// directive doesn't work in the test runner. + fn is_wasm(&self) -> bool { + matches!( + self, + Self::Postgres(Some(PostgresVersion::PgJsWasm)) + | Self::Postgres(Some(PostgresVersion::NeonJsWasm)) + | Self::Vitess(Some(VitessVersion::PlanetscaleJsWasm)) + | Self::Sqlite(Some(SqliteVersion::LibsqlJsWasm)) + | Self::Sqlite(Some(SqliteVersion::CloudflareD1)) + ) + } } impl fmt::Display for ConnectorVersion { From bd0ec04810840a437b82e4a4b86f5cf2674aa6c2 Mon Sep 17 00:00:00 2001 From: jkomyno Date: Mon, 20 May 2024 17:46:06 +0200 Subject: [PATCH 08/11] DRIVER_ADAPTERS_BRANCH=integration/fix-sqlite-d1-max-bind-values chore: retrigger CI/CD From e6a25176a0b0c7e745fff860618d74f7cc78f81a Mon Sep 17 00:00:00 2001 From: jkomyno Date: Tue, 28 May 2024 11:04:56 +0200 Subject: [PATCH 09/11] DRIVER_ADAPTERS_BRANCH=integration/fix-sqlite-d1-max-bind-values chore: uncomment D1 test --- .../query-engine-tests/tests/new/regressions/prisma_7434.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs index 61788cd24e40..1924008e5e44 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs @@ -4,7 +4,7 @@ use query_engine_tests::*; mod not_in_chunking { use query_engine_tests::Runner; - #[connector_test(exclude(CockroachDb, Sqlite("cfd1")))] + #[connector_test] async fn not_in_batch_filter(runner: Runner) -> TestResult<()> { assert_error!( runner, From bc855636ead8a6e9e139ce99bba7c5c76ecdf70d Mon Sep 17 00:00:00 2001 From: jkomyno Date: Fri, 31 May 2024 11:43:49 +0200 Subject: [PATCH 10/11] DRIVER_ADAPTERS_BRANCH=integration/fix-sqlite-d1-max-bind-values chore: retrigger CI/CD From 150a46211abf82a6d0c5da74006c9337cf8f692b Mon Sep 17 00:00:00 2001 From: jkomyno Date: Fri, 31 May 2024 12:43:36 +0200 Subject: [PATCH 11/11] DRIVER_ADAPTERS_BRANCH=integration/fix-sqlite-d1-max-bind-values chore: retrigger CI/CD