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

Cyclic dependency checker should consider different 'as' parameter #1717

Closed
mlegenhausen opened this issue May 7, 2014 · 26 comments
Closed
Labels
released status: understood For issues. Applied when the issue is understood / reproducible. type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@mlegenhausen
Copy link
Contributor

mlegenhausen commented May 7, 2014

I have following example:

var Sequelize = require('sequelize');
var sequelize = new Sequelize('sequelize-test', 'root', '');
var Content = sequelize.define('Content', {});
var Version = sequelize.define('Version', {});

Content.hasMany(Version);
Version.belongsTo(Content);

Content.belongsTo(Version, {
    as: 'ReleaseVersion',
    foreignKey: 'ReleaseVersionId'
});

Version.hasOne(Content, {
    as: 'ReleaseVersion',
    foreignKey: 'ReleaseVersionId'
});

sequelize.sync({ force: true });

Which produces following error:

Error: Cyclic dependency found. 'Contents' is dependent of itself.
Dependency Chain: Contents -> Versions => Contents

I want to associate the active release of a content by the Content model, for this I added a new association that links directly to my Version model. The key for this id (ReleaseVersionId) should be in the Content Table. This key can be null if no version was released. Since Version 2.0.0-dev11 I get an error when constructing something like this. Do I miss something or can I model something like this in a sequelize way?

@janmeier
Copy link
Member

janmeier commented May 7, 2014

This is because dev11 introduces automatic foreign keys on your tables. This means that contentId on version will reference content.id, and content.realeaseVersionId will reference version.id. Since both tables now depend on each other, we cannot create any of them (content must be created before version, and version must be created before content.

To fix this problem, you should add constraints: false to the options of one or more of your associations to disable the foreign keys for these. If you want the security of foreign keys, you could add the keys manually after .sync has finished

@mlegenhausen
Copy link
Contributor Author

Thanks for this super fast response time. That solved the problem.

@asimsohail
Copy link

@janmeier.. can you show how to manually add the keys after .sync is complete?

@janmeier
Copy link
Member

janmeier commented Jul 2, 2015

@asimsohail Add them manually either using phpmyadmin/pgadmin whatever or by writing the query

@bottleofwater
Copy link

@janmeier Couldn't Sequelize first create the tables without the foreign keys, then add all the foreign keys right after?

@mickhansen
Copy link
Contributor

@bottleofwater that could work

@Znarkus
Copy link
Contributor

Znarkus commented Mar 14, 2016

Is constraints: false and adding keys after .sync the canonical way of doing cyclic database structures?

Table structure:

template
- id
- category_id

category
- id
- default_template_id

@ralusek
Copy link

ralusek commented Dec 15, 2016

@mickhansen Is this currently in development?

@marc-ferrer
Copy link

I know this is an old issue, but why does not allowNull: true on the foreign key parameter make this work?

@jasonbodily
Copy link

jasonbodily commented Mar 8, 2017

Yes old issue, but it doesn't appear to work. With above, is this currently available?

EDIT: This does kind of work. You just have to do 3 things:

  1. Add 'constraints: false' on one of the associations
  2. Remove the relation information from the model attributes altogether
  3. Write the custom migration that adds the fkey at the appropriate time. An example of that custom query is
queryInterface.sequelize.query('ALTER TABLE public."<source_table>"  ADD CONSTRAINT 
"<source_table>_<local_fkey>_fkey" FOREIGN KEY (<local_fkey>)  
REFERENCES public."<TargetTable>" (id) MATCH SIMPLE 
ON UPDATE NO ACTION ON DELETE NO ACTION')

@gzurbach
Copy link

@jasonbodily it's constraints: false (with an s)... spent a few minutes figuring that out :)

@jasonbodily
Copy link

@gzurbach - Sorry about that! Updated my answer!

@mentos1386
Copy link

@jasonbodily You have a typo, constrainst: false should be constraints: false (ts at the end`)

@petersg83
Copy link

Same problem here.
More than 2 years this issue has been opened :)
I'll add contraints: false but it hurts my heart :'(

@JQuezada0
Copy link

Was this ever resolved?

@xaiyeon
Copy link

xaiyeon commented Apr 12, 2018

Aw man and here I thought this was solved... Just ran into this issue, but I have over 15 tables with hasMany relationships :( I will try them without and query right after migration then, but how do I call a migration file to be executed after .sync().then((function{}))?

@semy
Copy link

semy commented Aug 12, 2018

Any news on this? :-(

@yocontra
Copy link
Contributor

Can we get this issue reopened so somebody can potentially send a PR for this?

@sushantdhiman sushantdhiman added the type: feature For issues and PRs. For new features. Never breaking changes. label Oct 18, 2018
@sushantdhiman sushantdhiman reopened this Oct 18, 2018
@yocontra
Copy link
Contributor

@sushantdhiman Thanks!

@petrovi4
Copy link

petrovi4 commented Jan 8, 2019

@sushantdhiman Is this in development now?

@stale

This comment has been minimized.

@stale stale bot added the stale label Jul 23, 2019
@papb papb removed the stale label Jul 24, 2019
@LFabre
Copy link

LFabre commented Jan 15, 2020

Any news?

@papb
Copy link
Member

papb commented Jan 16, 2020

@LFabre and others - none that I know of. PR is welcome.

@papb papb added the status: understood For issues. Applied when the issue is understood / reproducible. label Jan 16, 2020
@esyeng
Copy link

esyeng commented Sep 11, 2020

Any news? :-(

@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 6.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ephys
Copy link
Member

ephys commented May 23, 2022

I'll close. Starting with Sequelize 6.20.0, if there is a cyclic foreign key, sync will first create the tables without foreign key constrants, then add them.
This change should resolve this issue.

@ephys ephys closed this as completed May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released status: understood For issues. Applied when the issue is understood / reproducible. type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests