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.

Sign up for GitHub

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

Jump to bottom

fix: regression on 1-1 nested connect in create #3514

Merged
merged 8 commits into from
Dec 20, 2022
Merged

fix: regression on 1-1 nested connect in create #3514

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
70bb416
fix: regression on 1-1 nested connect in create
Weakky Dec 15, 2022
46a5aee
fix mssql
Weakky Dec 15, 2022
6cdae11
add inlined 1-1 on parent for full mode
Weakky Dec 15, 2022
7be4980
print datamodel & query during tests
Weakky Dec 15, 2022
be5934c
[skip ci] improve comment
Weakky Dec 15, 2022
6136f51
exclude disconnect test
Weakky Dec 16, 2022
fa1ba00
exclude test for mongodb
Weakky Dec 16, 2022
0503baa
remove test project
Weakky Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions ...ine-tests/tests/writes/nested_mutations/already_converted/nested_connect_inside_create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ mod connect_inside_create {
)?;

assert_error!(
runner,
format!(
r#"mutation {{
runner,
format!(
r#"mutation {{
createOneParent(data:{{
p: "p2"
p_1: "p2_1"
Expand All @@ -155,11 +155,11 @@ mod connect_inside_create {
}}
}}
}}"#,
child = child
),
2025,
"An operation failed because it depends on one or more records that were required but not found. No 'Child' record to connect was found was found for a nested connect on one-to-one relation 'ChildToParent'."
);
child = child
),
2025,
"An operation failed because it depends on one or more records that were required but not found."
);

Ok(())
}
Expand Down
5 changes: 3 additions & 2 deletions ...-tests/tests/writes/nested_mutations/already_converted/nested_disconnect_inside_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ mod disconnect_inside_update {
}

// "a P1 to C1 relation " should "be disconnectable through a nested mutation by id"
#[relation_link_test(on_parent = "ToOneOpt", on_child = "ToOneOpt")]
// TODO: MongoDB doesn't support joins on top-level updates. It should be un-excluded once we fix that.
#[relation_link_test(on_parent = "ToOneOpt", on_child = "ToOneOpt", exclude(MongoDb))]
async fn p1_c1_by_fails_if_filters_no_match(runner: &Runner, t: &DatamodelWithParams) -> TestResult<()> {
let parent = t.parent().parse(
run_query_json!(
Expand Down Expand Up @@ -132,7 +133,7 @@ mod disconnect_inside_update {
where: {parent}
data:{{
p: {{ set: "p2" }}
childOpt: {{disconnect: {{ non_unique: "1" }} }}
childOpt: {{ disconnect: {{ non_unique: "1" }} }}
}}){{
childOpt {{
c
Expand Down
3 changes: 2 additions & 1 deletion ...-tests/tests/writes/nested_mutations/already_converted/nested_disconnect_inside_upsert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ mod disconnect_inside_upsert {
}

// "a P1 to C1 relation " should "be disconnectable through a nested mutation by id"
#[relation_link_test(on_parent = "ToOneOpt", on_child = "ToOneOpt")]
// TODO: MongoDB doesn't support joins on top-level updates. It should be un-excluded once we fix that.
#[relation_link_test(on_parent = "ToOneOpt", on_child = "ToOneOpt", exclude(MongoDb))]
async fn p1_c1_by_fails_if_filter_no_match(runner: &Runner, t: &DatamodelWithParams) -> TestResult<()> {
let res = run_query_json!(
runner,
Expand Down
2 changes: 1 addition & 1 deletion query-engine/connector-test-kit-rs/query-tests-setup/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ fn run_relation_link_test_impl(

run_with_tokio(
async move {
tracing::debug!("Used datamodel:\n {}", datamodel.clone().yellow());
println!("Used datamodel:\n {}", datamodel.clone().yellow());
setup_project(&datamodel, Default::default()).await.unwrap();

let runner = Runner::load(config.runner(), datamodel.clone(), connector, metrics, log_capture)
Expand Down
3 changes: 3 additions & 0 deletions query-engine/connector-test-kit-rs/query-tests-setup/src/runner/direct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{ConnectorTag, RunnerInterface, TestResult, TxResult};
use colored::Colorize;
use query_core::{executor, schema::QuerySchemaRef, schema_builder, QueryExecutor, TxId};
use query_engine_metrics::MetricRegistry;
use request_handlers::{GraphQlBody, GraphQlHandler, MultiQuery};
Expand Down Expand Up @@ -36,6 +37,8 @@ impl RunnerInterface for DirectRunner {
}

async fn query(&self, query: String) -> TestResult<crate::QueryResult> {
println!("{}", query.bright_green());

let handler = GraphQlHandler::new(&*self.executor, &self.query_schema);
let query = GraphQlBody::Single(query.into());

Expand Down
7 changes: 7 additions & 0 deletions query-engine/connector-test-kit-rs/query-tests-setup/src/schema_gen/references.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ pub fn simple_child_references<'a>(
) -> Vec<RelationReference<'a>> {
match *parent_id {
_ if on_child.is_list() && !on_parent.is_list() => vec![(RelationReference::NoRef)],
Identifier::Simple if on_child.is_to_one_opt() && on_parent.is_to_one_opt() => {
vec![RelationReference::SimpleParentId(on_child), RelationReference::NoRef]
}
Comment on lines +137 to +139
Copy link
Member Author

@Weakky Weakky Dec 15, 2022 •
edited

Choose a reason for hiding this comment

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

Makes sure we generate optional 1-1 relation inlined on both sides (child and parent).

It's a bit hard to follow what this does without the full context. In a nutshell, RelationReference::NoRef means some of the generated datamodels will not be inlined on the child side. Therefore, the logic for the parent side will inline the relation when it's not already inlined in the child.

This means we can expect generated datamodels of these (rough) shapes:

(inlined child-side)

model Parent {
  id Int @id
  childOpt Child?
}

model Child {
  id Int @id
  parentId Int? @unique
  parentOpt? @relation(fields: [parentId], references: [id])
}

(inlined parent-side)

model Parent {
  id Int @id

  childId Int? @unique
  childOpt Child? @relation(fields: [childId], references: [id])
}

model Child {
  id Int @id
  parentOpt?
}

Identifier::Simple => vec![RelationReference::SimpleParentId(on_child)],
Identifier::Compound => vec![RelationReference::CompoundParentId(on_child)],
Identifier::None => vec![RelationReference::ParentReference(on_child)],
Expand All @@ -150,6 +153,10 @@ pub fn full_child_references<'a>(
if !is_m2m {
match *parent_id {
_ if on_child.is_list() && !on_parent.is_list() => vec![RelationReference::NoRef],
Identifier::Simple if on_child.is_to_one_opt() && on_parent.is_to_one_opt() => {
vec![RelationReference::SimpleParentId(on_child), RelationReference::NoRef]
.clone_append(&mut common_parent_references(on_child))
}
Identifier::Simple => {
vec![RelationReference::SimpleParentId(on_child)].clone_append(&mut common_parent_references(on_child))
}
Expand Down
4 changes: 4 additions & 0 deletions query-engine/connector-test-kit-rs/query-tests-setup/src/schema_gen/relation_field.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ impl RelationField {
}
}

pub fn is_to_one_opt(&self) -> bool {
matches!(self, Self::ToOneOpt { .. })
}

pub fn field_name(&self) -> String {
match self {
RelationField::ToOneOpt { child } => match child {
Expand Down
22 changes: 19 additions & 3 deletions query-engine/connector-test-kit-rs/query-tests-setup/src/schema_gen/schema_with_relation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ fn render_relation_fields(

(rendered_parent, rendered_child)
} else {
let rendered_parent = format!(
let mut rendered_parent = format!(
"{} {} {}",
parent.field_name(),
parent.type_name(),
Expand All @@ -244,7 +244,20 @@ fn render_relation_fields(
);

if !child.is_list() && !parent.is_list() {
let unique_attribute = match child_ref_to_parent {
let child_unique = match child_ref_to_parent {
RelationReference::SimpleChildId(_) => r#"@@unique([childId])"#,
RelationReference::SimpleParentId(_) => r#"@@unique([parentId])"#,
RelationReference::CompoundParentId(_) => r#"@@unique([parent_id_1, parent_id_2])"#,
RelationReference::CompoundChildId(_) => r#"@@unique([child_id_1, child_id_2])"#,
RelationReference::ParentReference(_) => r#"@@unique([parentRef])"#,
RelationReference::CompoundParentReference(_) => r#"@@unique([parent_p_1, parent_p_2])"#,
RelationReference::ChildReference(_) => r#"@@unique([parent_c])"#,
RelationReference::CompoundChildReference(_) => r#"@@unique([child_c_1, child_c_2])"#,
RelationReference::IdReference => "",
RelationReference::NoRef => "",
};

let parent_unique = match parent_ref_to_child {
RelationReference::SimpleChildId(_) => r#"@@unique([childId])"#,
RelationReference::SimpleParentId(_) => r#"@@unique([parentId])"#,
RelationReference::CompoundParentId(_) => r#"@@unique([parent_id_1, parent_id_2])"#,
Expand All @@ -258,7 +271,10 @@ fn render_relation_fields(
};

rendered_child.push('\n');
rendered_child.push_str(unique_attribute);
rendered_child.push_str(child_unique);

rendered_parent.push('\n');
rendered_parent.push_str(parent_unique);
}

(rendered_parent, rendered_child)
Expand Down
9 changes: 9 additions & 0 deletions query-engine/connector-test-kit-rs/relation_link/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "relation_link"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
setup = { path = "../query-tests-setup", package = "query-tests-setup" }
Weakky marked this conversation as resolved.
Show resolved Hide resolved