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

fix(quaint): extract constraint name from CockroachDB foreign key constraint violation error #5002

Merged
merged 15 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
6 changes: 5 additions & 1 deletion .github/workflows/test-query-engine-template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ on:
version:
type: string
required: true
ubuntu:
type: string
required: true
single_threaded:
type: boolean
default: false
Expand All @@ -32,6 +35,7 @@ jobs:
partition: ["1/4", "2/4", "3/4", "4/4"]

env:
CI: "true"
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
LOG_LEVEL: "info"
LOG_QUERIES: "y"
RUST_LOG_FORMAT: "devel"
Expand All @@ -46,7 +50,7 @@ jobs:
PRISMA_ENGINE_PROTOCOL: ${{ matrix.engine_protocol }}
PRISMA_RELATION_LOAD_STRATEGY: ${{ matrix.relation_load_strategy }}

runs-on: ubuntu-latest
runs-on: ubuntu-${{ inputs.ubuntu }}
steps:
- uses: actions/checkout@v4
- uses: actions-rust-lang/setup-rust-toolchain@v1
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/test-query-engine.yml
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,13 @@ jobs:
database:
- name: "mssql_2022"
version: "2022"
ubuntu: "latest"
- name: "mssql_2019"
version: "2019"
ubuntu: "latest"
- name: "mssql_2017"
version: "2017"
ubuntu: "20.04"
uses: ./.github/workflows/test-query-engine-template.yml
name: mssql ${{ matrix.database.version }}
with:
Expand Down
7 changes: 4 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ napi = { version = "2.15.1", default-features = false, features = [
napi-derive = "2.15.0"
js-sys = { version = "0.3" }
rand = { version = "0.8" }
regex = { version = "1", features = ["std"] }
serde_repr = { version = "0.1.17" }
serde-wasm-bindgen = { version = "0.5" }
tracing = { version = "0.1" }
Expand Down
2 changes: 1 addition & 1 deletion libs/prisma-value/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ version = "0.1.0"
base64 = "0.13"
chrono.workspace = true
once_cell = "1.3"
regex = "1.2"
regex.workspace = true
bigdecimal = "0.3"
serde.workspace = true
serde_json.workspace = true
Expand Down
5 changes: 1 addition & 4 deletions libs/user-facing-errors/src/query_engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ pub struct UniqueKeyViolation {
}

#[derive(Debug, UserFacingError, Serialize)]
#[user_facing(
code = "P2003",
message = "Foreign key constraint failed on the field: `{field_name}`"
)]
#[user_facing(code = "P2003", message = "Foreign key constraint violated: `{field_name}`")]
pub struct ForeignKeyViolation {
/// Field name from one model from Prisma schema
pub field_name: String,
Expand Down
2 changes: 1 addition & 1 deletion psl/psl-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ chrono = { workspace = true }
connection-string.workspace = true
itertools.workspace = true
once_cell = "1.3.1"
regex = "1.3.7"
regex.workspace = true
serde.workspace = true
serde_json.workspace = true
enumflags2.workspace = true
Expand Down
1 change: 1 addition & 0 deletions quaint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ futures = "0.3"
url.workspace = true
hex = "0.4"
itertools.workspace = true
regex.workspace = true

either = { version = "1.6" }
base64 = { version = "0.12.3" }
Expand Down
18 changes: 12 additions & 6 deletions quaint/src/connector/postgres/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use regex::Regex;
use std::fmt::{Display, Formatter};

use crate::error::{DatabaseConstraint, Error, ErrorKind, Name};
Expand Down Expand Up @@ -28,8 +29,17 @@ impl Display for PostgresError {
}
}

fn extract_fk_constraint_name(message: &str) -> Option<String> {
let re = Regex::new(r#"foreign key constraint "([^"]+)""#).unwrap();
re.captures(message)
.and_then(|caps| caps.get(1).map(|m| m.as_str().to_string()))
}

impl From<PostgresError> for Error {
fn from(value: PostgresError) -> Self {
// println!("PostgresError: {:?}", value);
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
// panic!("PostgresError.code: {:?}", value.code.as_str());

match value.code.as_str() {
"22001" => {
let mut builder = Error::builder(ErrorKind::LengthMismatch {
Expand Down Expand Up @@ -89,12 +99,8 @@ impl From<PostgresError> for Error {
builder.build()
}
None => {
let constraint = value
.message
.split_whitespace()
.nth(10)
.and_then(|s| s.split('"').nth(1))
.map(ToString::to_string)
// `value.message` looks like `update on table "Child" violates foreign key constraint "Child_parent_id_fkey"`
let constraint = extract_fk_constraint_name(value.message.as_str())
.map(DatabaseConstraint::Index)
.unwrap_or(DatabaseConstraint::CannotParse);

Expand Down
2 changes: 1 addition & 1 deletion query-engine/black-box-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ user-facing-errors.workspace = true
insta = "1.7.1"
enumflags2.workspace = true
query-engine-metrics = {path = "../metrics"}
regex = "1.9.3"
regex.workspace = true
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,28 @@ macro_rules! match_connector_result {
};
}

#[macro_export]
macro_rules! assert_connector_error {
($runner:expr, $q:expr, $code:expr, $( $($matcher:pat_param)|+ $( if $pred:expr )? => $msg:expr ),*) => {
use query_tests_setup::*;
use query_tests_setup::ConnectorVersion::*;

let connector = $runner.connector_version();

let mut results = match &connector {
$(
$( $matcher )|+ $( if $pred )? => $msg.to_string()
),*
};

if results.len() == 0 {
panic!("No assertion failure defined for connector {connector}.");
}

$runner.query($q).await?.assert_failure($code, Some(results));
};
}

#[macro_export]
macro_rules! is_one_of {
($result:expr, $potential_results:expr) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ mod one2one_req {
runner,
"mutation { deleteOneParent(where: { id: 1 }) { id }}",
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down Expand Up @@ -187,7 +187,7 @@ mod one2one_opt {
runner,
"mutation { deleteOneParent(where: { id: 1 }) { id }}",
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down Expand Up @@ -293,7 +293,7 @@ mod one2many_req {
runner,
"mutation { deleteOneParent(where: { id: 1 }) { id }}",
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down Expand Up @@ -397,7 +397,7 @@ mod one2many_opt {
runner,
"mutation { deleteOneParent(where: { id: 1 }) { id }}",
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ mod one2one_req {
&runner,
r#"mutation { updateOneParent(where: { id: 1 }, data: { uniq: "u1" }) { id }}"#,
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down Expand Up @@ -182,7 +182,7 @@ mod one2one_opt {
&runner,
r#"mutation { updateOneParent(where: { id: 1 } data: { uniq: "u1" }) { id }}"#,
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down Expand Up @@ -287,7 +287,7 @@ mod one2many_req {
&runner,
r#"mutation { updateOneParent(where: { id: 1 }, data: { uniq: "u1" }) { id }}"#,
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down Expand Up @@ -390,7 +390,7 @@ mod one2many_opt {
&runner,
r#"mutation { updateOneParent(where: { id: 1 }, data: { uniq: "u1" }) { id }}"#,
2003,
"Foreign key constraint failed on the field"
"Foreign key constraint violated"
);

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ mod prisma_21901;
mod prisma_22007;
mod prisma_22298;
mod prisma_22971;
mod prisma_24072;
mod prisma_5952;
mod prisma_6173;
mod prisma_7010;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use indoc::indoc;
use query_engine_tests::*;

#[test_suite(schema(schema))]
mod prisma_24072 {
fn schema() -> String {
let schema = indoc! {
r#"model Parent {
#id(id, Int, @id)
child Child?
}

model Child {
#id(id, Int, @id)
parent_id Int? @default(2) @unique
parent Parent? @relation(fields: [parent_id], references: [id], onDelete: SetDefault)
}"#
};

schema.to_owned()
}

// Deleting the parent without cascading to the child should fail with an explicitly named constraint violation,
// without any "(not available)" names.
#[connector_test]
async fn test_24072(runner: Runner) -> TestResult<()> {
insta::assert_snapshot!(
run_query!(&runner, r#"mutation { createOneParent(data: { id: 1, child: { create: { id: 1 }}}) { id }}"#),
@r###"{"data":{"createOneParent":{"id":1}}}"###
);

assert_connector_error!(
&runner,
"mutation { deleteOneParent(where: { id: 1 }) { id }}",
2003,
CockroachDb(_) | Postgres(_) | SqlServer(_) | Vitess(_) => "Foreign key constraint violated: `Child_parent_id_fkey (index)`",
MySql(_) => "Foreign key constraint violated: `parent_id`",
Sqlite(_) => "Foreign key constraint violated: `foreign key`",
_ => "Foreign key constraint violated"
);

Ok(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ thiserror = "1.0"
async-trait.workspace = true
nom = "7.1"
itertools.workspace = true
regex = "1"
regex.workspace = true
serde.workspace = true
tracing.workspace = true
tracing-futures = "0.2"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ itertools.workspace = true
mongodb.workspace = true
bson.workspace = true
rand.workspace = true
regex = "1"
regex.workspace = true
serde_json.workspace = true
thiserror = "1.0"
tokio.workspace = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ tokio.workspace = true
tracing.workspace = true
convert_case = "0.6.0"
once_cell = "1.8.0"
regex = "1.7.3"
regex.workspace = true
indoc.workspace = true

[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion schema-engine/connectors/sql-schema-connector/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ chrono.workspace = true
connection-string.workspace = true
enumflags2.workspace = true
once_cell = "1.3"
regex = "1"
regex.workspace = true
serde_json.workspace = true
tracing.workspace = true
tracing-futures = "0.2"
Expand Down
2 changes: 1 addition & 1 deletion schema-engine/datamodel-renderer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ edition = "2021"
[dependencies]
once_cell = "1.15.0"
psl.workspace = true
regex = "1.6.0"
regex.workspace = true
base64 = "0.13.1"

[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion schema-engine/sql-schema-describer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ enumflags2.workspace = true
indexmap.workspace = true
indoc.workspace = true
once_cell = "1.3"
regex = "1.2"
regex.workspace = true
serde.workspace = true
tracing.workspace = true
tracing-error = "0.2"
Expand Down
Loading