Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(schema-engine): disabled implicit transaction for batch statements starting from CockroachDB v22.2 #4632

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,44 @@ impl Connection {

let version = quaint.version().await.map_err(quaint_err(&url))?;

if version.map(|v| v.starts_with("CockroachDB CCL v22.2")).unwrap_or(false) {
// first config issue: https://github.com/prisma/prisma/issues/16909
// second config value: Currently at least version 22.2.5, enums are
// not case-sensitive without this.
quaint
.raw_cmd(indoc! {r#"
SET enable_implicit_transaction_for_batch_statements=false;
SET use_declarative_schema_changer=off
"#})
.await
.map_err(quaint_err(&url))?;
if let Some(version) = version {
let cockroach_version_prefix = "CockroachDB CCL v";

let semver: Option<(u8, u8)> = version.strip_prefix(cockroach_version_prefix).and_then(|v| {
let semver_unparsed: String = v.chars().take_while(|&c| c.is_ascii_digit() || c == '.').collect();

// we only consider the major and minor version, as the patch version is not interesting for us
semver_unparsed.split_once('.').and_then(|(major, minor_and_patch)| {
let major = major.parse::<u8>().ok();

let minor = minor_and_patch
.chars()
.take_while(|&c| c != '.')
.collect::<String>()
.parse::<u8>()
.ok();

major.zip(minor)
})
});

match semver {
Some((major, minor)) if (major == 22 && minor >= 2) || major >= 23 => {
// we're on 22.2+ or 23+
//
// first config issue: https://github.com/prisma/prisma/issues/16909
// second config value: Currently at least version 22.2.5, enums are
// not case-sensitive without this.
quaint
.raw_cmd(indoc! {r#"
SET enable_implicit_transaction_for_batch_statements=false;
SET use_declarative_schema_changer=off
"#})
.await
.map_err(quaint_err(&url))?;
}
None | Some(_) => (),
};
}

Ok(Connection(quaint))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,72 @@ fn connecting_to_a_cockroachdb_database_with_the_postgresql_connector_fails(_api
expected_error.assert_eq(&err);
}

// This test follows https://github.com/prisma/prisma-engines/pull/4632.
#[test_connector(tags(CockroachDb))]
fn decimal_to_boolean_migrations_work(api: TestApi) {
let dir = api.create_migrations_directory();

let dm1 = r#"
datasource db {
provider = "cockroachdb"
url = env("TEST_DATABASE_URL")
}

model Cat {
id BigInt @id @default(autoincrement())
tag Decimal
}
"#;

api.create_migration("create-cats-decimal", &dm1, &dir)
.send_sync()
.assert_migration_directories_count(1)
.assert_migration("create-cats-decimal", move |migration| {
let expected_script = expect![[r#"
-- CreateTable
CREATE TABLE "Cat" (
"id" INT8 NOT NULL DEFAULT unique_rowid(),
"tag" DECIMAL(65,30) NOT NULL,

CONSTRAINT "Cat_pkey" PRIMARY KEY ("id")
);
"#]];

migration.expect_contents(expected_script)
});

let dm2 = r#"
datasource db {
provider = "cockroachdb"
url = env("TEST_DATABASE_URL")
}

model Cat {
id BigInt @id @default(autoincrement())
tag Boolean
}
"#;

api.create_migration("migrate-cats-boolean", &dm2, &dir)
.send_sync()
.assert_migration_directories_count(2)
.assert_migration("migrate-cats-boolean", move |migration| {
let expected_script = expect![[r#"
/*
Warnings:

- Changed the type of `tag` on the `Cat` table. No cast exists, the column would be dropped and recreated, which cannot be done if there is data, since the column is required.

*/
-- AlterTable
ALTER TABLE "Cat" DROP COLUMN "tag";
ALTER TABLE "Cat" ADD COLUMN "tag" BOOL NOT NULL;
"#]];

migration.expect_contents(expected_script)
});
}
Comment on lines +454 to +518
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to the reviewer: although I expected this test to fail against main when run on CockroachDB 23.1, it doesn't :/

This implies that somewhere in here we're not testing the "bad case for CockroachDB 22.2+" properly.


#[test_connector(tags(CockroachDb))]
fn int_to_string_conversions_work(api: TestApi) {
let dm1 = r#"
Expand Down