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

Add 'truncate' option to sequelize.sync. #2671

Closed
wants to merge 2 commits into
base: master
from

Conversation

3 participants
@dkushner

dkushner commented Dec 4, 2014

Fixes #2670.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 5, 2014

@dkushner how is this exactly related to sync? Wouldn't this simply be a standalone truncate method?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 5, 2014

(and i actually believe we have truncate support via truncate: true for deletes)

* @param {Boolean} [options.truncate=false] Similar to force, if truncate is
* true, each DAO will do TRUNCATE TABLE, eliminating all rows within the
* table while maintaining the table structure. Note that force will always
* override this option.

This comment has been minimized.

@mickhansen

mickhansen Dec 5, 2014

Contributor

@janmeier will this doc syntax work?

This comment has been minimized.

@janmeier

This comment has been minimized.

@dkushner

dkushner Dec 10, 2014

@janmeier Adding some more commits to move the functionality out of sync(). What about the syntax breaks the docs? I'll make sure not to do it again.

@dkushner

This comment has been minimized.

dkushner commented Dec 5, 2014

@mickhansen, it definitely can be a stand-alone truncate method. Would that be preferable? I was trying to avoid disrupting the existing API as much as possible. I also have a number of conceptual objections to the use of sync({ force: true }) given that Sequelize models do not represent an authoritative source for a database schema (and should not).

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 5, 2014

@dkushner well in my opinion it doesn't just make a lot of sense in sync since that carries a different meaning.

I neither fully disagree or agree with your objections to sync({force: true}), it's great for its few usecases - but no, it's obviously not the solution you want after you go out of the experimental phase.

@dkushner

This comment has been minimized.

dkushner commented Dec 10, 2014

@mickhansen, I just saw your comment regarding the Model.delete({ truncate: true }) implementation. Since sequelize.sync() essentially just goes through the list of DAOs and calls sync() on each one, would this be an appropriate behaviour for sequelize.truncate()?

For example, it would just be something like:

return Promise.reduce(this.modelManager.daos, function(ag, dao) {
    return dao.delete({}, { truncate: true });
}, null);

Perhaps this is contrived, but a way to simply clear all rows of the database without affecting the schema seems useful to me. Thoughts?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 10, 2014

@dkushner Utility methods are fine to have. sequelize.truncate() would be fine :)
Your proposed solution for implementing a top level truncate method could work, although i'd probably just use Promise.map.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 22, 2014

Needs a rebase @dkushner, didn't see that you moved it to it's own method.

@mickhansen mickhansen self-assigned this Dec 22, 2014

@dkushner

This comment has been minimized.

dkushner commented Dec 30, 2014

@mickhansen, gotcha. My bad on that.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Apr 23, 2015

Ping @dkushner

@janmeier

This comment has been minimized.

Member

janmeier commented May 24, 2015

No response, closing

@janmeier janmeier closed this May 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment