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

Enable the override of the default attributes #635

Merged
merged 1 commit into from May 22, 2013

Conversation

3 participants
@sevastos
Contributor

sevastos commented May 21, 2013

From what I understood the DAO factory adds some default attributes. In that process it adds the id column with options suited to a traditional PK along with the Integer data type.

I wanted my id to have a different datatype but when I do the following:

sequelize.define('picture', {
      id : Sequelize.BIGINT
})

it does nothing, due to the way the addition of the default attributes. So I made some changes to support that.

Let me know what you think.
Cheers

@sevastos

This comment has been minimized.

Contributor

sevastos commented May 21, 2013

Both postgres and mysql pass, but apparently sqlite is blocking this.

Error: SQLITE_ERROR: AUTOINCREMENT is only allowed on an INTEGER PRIMARY KEY
--> in Database#all('CREATE TABLE IF NOT EXISTS `pictures` (`id` BIGINT PRIMARY KEY AUTOINCREMENT, `createdAt` DATETIME NOT NULL, `updatedAt` DATETIME NOT NULL);', [Function])

A way to proceed would be to check for the combo of BIGINT and autoincrement and throw an exception just for sqlite, but I am not sure how I feel about having different behavior per driver.

Another better way imo would be just to change the test options to have { autoincrement: false} and let the developer bump to the sqlite restriction when it happens.

Feedback welcome.

self.rawAttributes[attr] = value
if (Utils._.isUndefined(self.rawAttributes[attr])) {
self.rawAttributes[attr] = value
}

This comment has been minimized.

@sevastos

sevastos May 21, 2013

Contributor

There is also Utils._.merge that could be used here and make it more flexible, but it could be more risky as well.

@jkarsrud

This comment has been minimized.

jkarsrud commented May 21, 2013

This is a much hidden feature that I bet most people aren't aware of exists. Allowing people do override defaults like id is something that should be in, as there are use cases where the id isn't always a number (they can be GUID's for instance).

@sevastos

This comment has been minimized.

Contributor

sevastos commented May 21, 2013

@sdepold you're right, all int-variants are just aliases to INTEGER, magic handles the sizes1.

So I added on sqlite's query-generator a check for when autoincrement is present to replace BIGINT with INTEGER .

All green, let me know if you want something different.

@sdepold

This comment has been minimized.

Member

sdepold commented May 22, 2013

Thanks a lot.

sdepold added a commit that referenced this pull request May 22, 2013

Merge pull request #635 from sevastos/override-default-attributes
Enable the override of the default attributes

@sdepold sdepold merged commit a7436d3 into sequelize:master May 22, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment