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

Support smallserial and bigserial data type. #4

Open
murmur76 opened this issue Jan 14, 2016 · 10 comments
Open

Support smallserial and bigserial data type. #4

murmur76 opened this issue Jan 14, 2016 · 10 comments

Comments

@murmur76
Copy link

murmur76 commented Jan 14, 2016

Unlike mysql's auto_increment constraint, postgresql has 3 different types for autoincrement field smallserial, serial, bigserial.

For example,

db.createTable('user', {
    id: { type: type.BIG_INTEGER, primaryKey: true, notNull: true, autoIncrement: true }
})

I tried this code but it creates serial type which is unexpected behavior. From quick look of source code, I think it would be same for smallserial type as well. It could be easily fixed like below.

    createColumnConstraint: function(spec, options, tableName, columnName) {
        var constraint = [],
            cb;

        if (spec.primaryKey && options.emitPrimaryKey) {
            if (spec.autoIncrement) {
                switch (spec.type) {
                    case type.SMALL_INTEGER: constraint.push('SMALLSERIAL');
                    case type.BIG_INTEGER: constraint.push('BIGSERIAL');
                    default: constraint.push('SERIAL');
                }
            }
            constraint.push('PRIMARY KEY');
        }
     ...


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@wzrdtales
Copy link
Member

As there is also bigint in postgre, this would rather be named BIG_SERIAL instead of BIG_INTEGER. If you wont mind you could also open a PR and we work on getting this into this driver.

@murmur76
Copy link
Author

Do you mean we're going to add additional types such as BIG_SERIAL and SMALL_SERIAL to this file only for PostgreSQL support?
If it is, I'm afraid that end users using other databases could get confused. So I prefer my initial suggestion. How about your opinion?

By the way, I'm willing to make a PR for this issue :)

@wzrdtales
Copy link
Member

To support db specifics better drivers are now not a part of the project anymore and every driver will and should contain its own documentation outlining the differences.

But to discuss this, may be it makes sense anyway to additionally add what you suggested. Using the proper datatypes for postgre if autoincrement is getting set. This would add both: The support of the datatypes itself and the handling of the generic datatypes that are available overall.

@wzrdtales
Copy link
Member

With not the project I meant, they are not a part of db-migrate, the main module, anymore, but are standalone now. While db-migrate enforces some opinions there is still the freedom for drivers to support some specifics. But that would also ment that they need to maintain those. For all drivers officially maintained this is the same. And there are plans to extend the drivers to get the most out of the different systems.

For example there is a new driver planned, the MariaDB driver which is supporting specifics like virtual columns, dynamic columns and so on and so forth.

I wanted to write a bit about how I imagine how drivers are related to db-migrate and what a good migration looks like for me, but really haven't had the time for it. I need to set me a date for this to finally do it :)

@murmur76
Copy link
Author

Yeah, I agree on what you think of. Supporting all databases only with generic data types has a clear limit.
So basically you're going to extend main db-migrate's data types to this repos and manage postgres specific data types here, right? It seems that it will take some time until you finish base work for that.

I'll keep this issue until it's ready, then I could make a PR for it.

@wzrdtales
Copy link
Member

Yes there are some things that need some time, there is also a big chunk that is upcoming for db-migrate. I'm currently redesigning some things such as the seeder handling and changes to the testing. Tests are currently only on db-migrates level but they need to be migrated to the drivers itself.

Please feel free to suggest, ask and also help whenever you want, I will try to answer and react as fast as possible :)

@murmur76
Copy link
Author

That sounds nice. Testing drivers on their own repo is exactly what i was thinking. If you can make some TODOs, maybe I can help you on free time.

@wzrdtales
Copy link
Member

I think I will transfer my tasks from asana into issues and take some time to describe them all. I have marked me the sunday to start with this :)

@worldspawn
Copy link
Contributor

#13

Before outputting the primary key constraint just check the specified data type and output the preferred SERIAL version. SERIAL as fallback.

@yosbelms
Copy link

yosbelms commented May 1, 2021

Any progress on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants