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

Remove logic for dropping relations when they are part of the ID #578

Merged
merged 9 commits into from Mar 18, 2020
Merged

Remove logic for dropping relations when they are part of the ID #578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
65f29cf
remove logic for dropping relations
do4gr Mar 16, 2020
0016d9b
accept models that only have a 1:1 relation as id
do4gr Mar 16, 2020
d4e6f10
notes
do4gr Mar 16, 2020
e54c6b4
notes on commenting out logic
do4gr Mar 17, 2020
51f69cb
recognize relations as @id if primary is set on the fk fields
do4gr Mar 17, 2020
df68607
test fixes
do4gr Mar 17, 2020
c41aee9
adjust commenting out tests for models without uniques or 1:1 relations
do4gr Mar 17, 2020
ccbef27
cleanups
do4gr Mar 18, 2020
f3b1c4c
Merge branch 'master' into RemoveGuardrailsForIdEqualsRelation
do4gr Mar 18, 2020
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
57 changes: 32 additions & 25 deletions introspection-engine/connectors/sql-introspection-connector/src/calculate_datamodel.rs
View file
Open in desktop
Expand Up @@ -30,8 +30,12 @@ pub fn calculate_model(schema: &SqlSchema) -> SqlIntrospectionResult<Datamodel>
}

for foreign_key in &table.foreign_keys {
let mut fields = calculate_relation_field(schema, table, foreign_key, &table.foreign_keys);
model.add_fields(&mut fields);
model.add_field(calculate_relation_field(
schema,
table,
foreign_key,
&table.foreign_keys,
));
}

for index in &table.indices {
Expand All @@ -40,24 +44,14 @@ pub fn calculate_model(schema: &SqlSchema) -> SqlIntrospectionResult<Datamodel>
.iter()
.find(|fk| columns_match(&fk.columns, &index.columns));

// This is part of the temporary guardrail that makes us ignore relations that overlap
// with the primary key.
let fk_on_index_overlaps_with_pk: bool = table
.primary_key
.as_ref()
.and_then(|pk| fk_on_index.as_ref().map(|fk| (pk, fk)))
.map(|(pk, fk)| {
pk.columns
.iter()
.any(|pk_col| fk.columns.iter().any(|fk_col| pk_col == fk_col))
})
.unwrap_or(false);

let compound_field_name = || {
let compound_name = || {
model
.fields
.iter()
.find(|f| !f.database_names.is_empty() && columns_match(&f.database_names, &index.columns))
.find(|f| {
!f.database_names.is_empty()
&& columns_match(&f.database_names, &index.columns)
})
.expect("Error finding field matching a compound index.")
.name
.clone()
Expand All @@ -66,10 +60,7 @@ pub fn calculate_model(schema: &SqlSchema) -> SqlIntrospectionResult<Datamodel>
let index_to_add = match (fk_on_index, index.columns.len(), index.is_unique()) {
(Some(_), _, true) => None, // just make the relation 1:1 and dont print the unique index
(Some(_), 1, false) => Some(calculate_index(index)),
(Some(_), _, false) if !fk_on_index_overlaps_with_pk => {
Some(calculate_compound_index(index, compound_field_name()))
}
(Some(_), _, false) => Some(calculate_index(index)),
(Some(_), _, false) => Some(calculate_compound_index(index, compound_name())),
(None, 1, true) => None, // this is expressed by the @unique already
(None, _, true) => Some(calculate_index(index)),
(None, _, false) => Some(calculate_index(index)),
Expand All @@ -88,7 +79,11 @@ pub fn calculate_model(schema: &SqlSchema) -> SqlIntrospectionResult<Datamodel>
for e in schema.enums.iter() {
data_model.add_enum(dml::Enum {
name: e.name.clone(),
values: e.values.iter().map(|v| dml::EnumValue::new(v, None)).collect(),
values: e
.values
.iter()
.map(|v| dml::EnumValue::new(v, None))
.collect(),
database_name: None,
documentation: None,
});
Expand All @@ -110,7 +105,13 @@ pub fn calculate_model(schema: &SqlSchema) -> SqlIntrospectionResult<Datamodel>
.is_none()
{
let other_model = data_model.find_model(relation_info.to.as_str()).unwrap();
let field = calculate_backrelation_field(schema, model, other_model, relation_field, relation_info);
let field = calculate_backrelation_field(
schema,
model,
other_model,
relation_field,
relation_info,
);

fields_to_be_added.push((other_model.name.clone(), field));
}
Expand All @@ -119,7 +120,11 @@ pub fn calculate_model(schema: &SqlSchema) -> SqlIntrospectionResult<Datamodel>
}

// add prisma many to many relation fields
for table in schema.tables.iter().filter(|table| is_prisma_join_table(&table)) {
for table in schema
.tables
.iter()
.filter(|table| is_prisma_join_table(&table))
{
if let (Some(f), Some(s)) = (table.foreign_keys.get(0), table.foreign_keys.get(1)) {
let is_self_relation = f.referenced_table == s.referenced_table;

Expand Down Expand Up @@ -154,5 +159,7 @@ fn columns_match(a_cols: &[String], b_cols: &[String]) -> bool {
return false;
}

a_cols.iter().all(|a_col| b_cols.iter().any(|b_col| a_col == b_col))
a_cols
.iter()
.all(|a_col| b_cols.iter().any(|b_col| a_col == b_col))
}
50 changes: 49 additions & 1 deletion introspection-engine/connectors/sql-introspection-connector/src/commenting_out_guardrails.rs
View file
Open in desktop
@@ -1,13 +1,61 @@
use datamodel::Datamodel;
use datamodel::{Datamodel, FieldArity, FieldType, RelationInfo};

pub fn commenting_out_guardrails(datamodel: &mut Datamodel) {
let mut commented_model_names = vec![];
let mut models_with_one_to_one_relation = vec![];

for model in &datamodel.models {
if model
.fields
.iter()
.any(|f| match (&f.arity, &f.field_type) {
(FieldArity::List, _) => false,
(
_,
FieldType::Relation(RelationInfo {
to,
to_fields: _,
name: relation_name,
..
}),
) => {
let other_model = datamodel.find_model(to).unwrap();
let other_field = other_model
.fields
.iter()
.find(|f| match &f.field_type {
FieldType::Relation(RelationInfo {
to: other_to,
to_fields: _,
name: other_relation_name,
..
}) if other_to == &model.name
&& relation_name == other_relation_name =>
{
true
}
_ => false,
})
.unwrap();

match other_field.arity {
FieldArity::Optional | FieldArity::Required => true,
FieldArity::List => false,
}
}
(_, _) => false,
})
{
models_with_one_to_one_relation.push(model.name.clone())
}
}

//models without uniques / ids
for model in &mut datamodel.models {
if model.id_fields.is_empty()
&& !model.fields.iter().any(|f| f.is_id || f.is_unique)
&& !model.indices.iter().any(|i| i.is_unique())
&& !models_with_one_to_one_relation.contains(&model.name)
{
commented_model_names.push(model.name.clone());
model.is_commented_out = true;
Expand Down
140 changes: 62 additions & 78 deletions introspection-engine/connectors/sql-introspection-connector/src/misc_helpers.rs
View file
Open in desktop
Expand Up @@ -147,92 +147,68 @@ pub(crate) fn calculate_relation_field(
table: &Table,
foreign_key: &ForeignKey,
foreign_keys: &Vec<ForeignKey>,
) -> Vec<Field> {
) -> Field {
debug!("Handling compound foreign key {:?}", foreign_key);
//if at least one primary key column is contained in the foreign key drop the relation.
if table.primary_key.is_some()
&& table
.primary_key
.as_ref()
.unwrap()
.columns
.iter()
.any(|c| foreign_key.columns.contains(c))
{
foreign_key
.columns
.iter()
.map(|fk_column| {
calculate_scalar_field(
schema,
table,
table.columns.iter().find(|c| c.name == *fk_column).unwrap(),
Some(format!(
"This used to be part of a relation to {}",
foreign_key.referenced_table.clone()
)),
)
})
.collect()
} else {
let field_type = FieldType::Relation(RelationInfo {
name: calculate_relation_name(schema, foreign_key, table),
to: foreign_key.referenced_table.clone(),
to_fields: foreign_key.referenced_columns.clone(),
on_delete: OnDeleteStrategy::None,
});

let columns: Vec<&Column> = foreign_key
.columns
.iter()
.map(|c| table.columns.iter().find(|tc| tc.name == *c).unwrap())
.collect();
let field_type = FieldType::Relation(RelationInfo {
name: calculate_relation_name(schema, foreign_key, table),
to: foreign_key.referenced_table.clone(),
to_fields: foreign_key.referenced_columns.clone(),
on_delete: OnDeleteStrategy::None,
});

let arity = match !columns.iter().any(|c| c.is_required()) {
true => FieldArity::Optional,
false => FieldArity::Required,
};
let columns: Vec<&Column> = foreign_key
.columns
.iter()
.map(|c| table.columns.iter().find(|tc| tc.name == *c).unwrap())
.collect();

let more_then_one_compound_to_same_table = || {
foreign_keys
.iter()
.filter(|fk| {
fk.referenced_table == foreign_key.referenced_table && fk.columns.len() > 1
})
.count()
> 1
};
let arity = match !columns.iter().any(|c| c.is_required()) {
true => FieldArity::Optional,
false => FieldArity::Required,
};

let (name, database_name) = match columns.len() {
1 => (columns[0].name.clone(), vec![]),
_ if more_then_one_compound_to_same_table() => (
format!(
"{}_{}",
foreign_key.referenced_table.clone().camel_case(),
columns[0].name.clone()
),
columns.iter().map(|c| c.name.clone()).collect(),
),
_ => (
let more_then_one_compound_to_same_table = || {
foreign_keys
.iter()
.filter(|fk| {
fk.referenced_table == foreign_key.referenced_table && fk.columns.len() > 1
})
.count()
> 1
};

let (name, database_name) = match columns.len() {
1 => (columns[0].name.clone(), vec![]),
_ if more_then_one_compound_to_same_table() => (
format!(
"{}_{}",
foreign_key.referenced_table.clone().camel_case(),
columns.iter().map(|c| c.name.clone()).collect(),
columns[0].name.clone()
),
};
columns.iter().map(|c| c.name.clone()).collect(),
),
_ => (
foreign_key.referenced_table.clone().camel_case(),
columns.iter().map(|c| c.name.clone()).collect(),
),
};

let is_id = is_relation_and_id(columns, &table);

vec![Field {
name,
arity,
field_type,
database_names: database_name,
default_value: None,
is_unique: false,
is_id: false,
documentation: None,
is_generated: false,
is_updated_at: false,
data_source_fields: vec![],
is_commented_out: false,
}]
Field {
name,
arity,
field_type,
database_names: database_name,
default_value: None,
is_unique: false,
is_id,
documentation: None,
is_generated: false,
is_updated_at: false,
data_source_fields: vec![],
is_commented_out: false,
}
}

Expand Down Expand Up @@ -342,6 +318,14 @@ pub(crate) fn is_id(column: &Column, table: &Table) -> bool {
.unwrap_or(false)
}

pub(crate) fn is_relation_and_id(columns: Vec<&Column>, table: &Table) -> bool {
table
.primary_key
.as_ref()
.map(|pk| pk.matches_foreign_key(columns))
.unwrap_or(false)
}

pub(crate) fn is_part_of_id(column: &Column, table: &Table) -> bool {
table
.primary_key
Expand Down
12 changes: 7 additions & 5 deletions ...tors/sql-introspection-connector/tests/db_specific_introspection/mysql/relations_mysql.rs
View file
Open in desktop
Expand Up @@ -64,7 +64,9 @@ async fn introspecting_two_one_to_one_relations_between_the_same_models_should_w
.execute_with_schema(
|migration| {
migration.change_table("User", |t| {
t.inject_custom("ADD CONSTRAINT `post_fk` FOREIGN KEY(`post_id`) REFERENCES `Post`(`id`)");
t.inject_custom(
"ADD CONSTRAINT `post_fk` FOREIGN KEY(`post_id`) REFERENCES `Post`(`id`)",
);
});
},
api.db_name(),
Expand Down Expand Up @@ -502,7 +504,7 @@ async fn introspecting_cascading_delete_behaviour_should_work(api: &TestApi) {
}

#[test_each_connector(tags("mysql"))]
async fn introspecting_id_fields_with_foreign_key_should_ignore_the_relation(api: &TestApi) {
async fn introspecting_id_fields_with_foreign_key_should_work(api: &TestApi) {
let barrel = api.barrel();
let _setup_schema = barrel
.execute(|migration| {
Expand All @@ -520,12 +522,12 @@ async fn introspecting_id_fields_with_foreign_key_should_ignore_the_relation(api
let dm = r#"
model Post {
test String
/// This used to be part of a relation to User
user_id Int @id
user_id User @id
}

model User {
id Int @id @default(autoincrement())
id Int @id @default(autoincrement())
post Post[]
}
"#;
let result = dbg!(api.introspect().await);
Expand Down
4 changes: 2 additions & 2 deletions ...nectors/sql-introspection-connector/tests/db_specific_introspection/mysql/tables_mysql.rs
View file
Open in desktop
Expand Up @@ -274,14 +274,14 @@ async fn introspecting_a_table_without_uniques_should_comment_it_out(api: &TestA
migration.create_table("Post", |t| {
t.add_column("id", types::integer());
t.inject_custom(
"user_id INTEGER NOT NULL UNIQUE,
"user_id INTEGER NOT NULL,
FOREIGN KEY (`user_id`) REFERENCES `User`(`id`)",
)
});
})
.await;

let dm = "/// The underlying table does not contain a unique identifier and can therefore currently not be handled.\n// model Post {\n // id Int\n // user_id User\n// }\n\nmodel User {\n id Int @default(autoincrement()) @id\n}";
let dm = "/// The underlying table does not contain a unique identifier and can therefore currently not be handled.\n// model Post {\n // id Int\n // user_id User\n\n // @@index([user_id], name: \"user_id\")\n// }\n\nmodel User {\n id Int @default(autoincrement()) @id\n}";

let result = dbg!(api.introspect().await);
assert_eq!(&result, dm);
Expand Down