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

createTable errors out when you specify length and autoincrement in postgres #323

Closed
zilkey opened this issue Nov 6, 2015 · 4 comments
Closed
Assignees

Comments

@zilkey
Copy link

zilkey commented Nov 6, 2015

When using the postgres driver, and using the migration from the docs:

'use strict';

var dbm;
var type;
var seed;

/**
  * We receive the dbmigrate dependency from dbmigrate initially.
  * This enables us to not have to rely on NODE_PATH.
  */
exports.setup = function(options, seedLink) {
  dbm = options.dbmigrate;
  type = dbm.dataType;
  seed = seedLink;
};

exports.up = function(db) {
  db.createTable( 'product_variant',
  {
      id:
      {
        type: 'int',
        unsigned: true,
        notNull: true,
        primaryKey: true,
        autoIncrement: true,
        length: 10
      },
      product_id:
      {
        type: 'int',
        unsigned: true,
        length: 10,
        notNull: true,
        foreignKey: {
          name: 'product_variant_product_id_fk',
          table: 'product',
          rules: {
            onDelete: 'CASCADE',
            onUpdate: 'RESTRICT'
          },
          mapping: 'id'
        }
      },
  });
  return null;
};

exports.down = function(db) {
  return null;
};

And you run it with node node_modules/.bin/db-migrate up you'll get:

[ERROR] error: syntax error at or near "("
    at Connection.parseE (/Users/galvanize/workspace/exercises/distributed-memories-services/node_modules/pg/lib/connection.js:539:11)
    at Connection.parseMessage (/Users/galvanize/workspace/exercises/distributed-memories-services/node_modules/pg/lib/connection.js:366:17)
    at Socket.<anonymous> (/Users/galvanize/workspace/exercises/distributed-memories-services/node_modules/pg/lib/connection.js:105:22)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at readableAddChunk (_stream_readable.js:146:16)
    at Socket.Readable.push (_stream_readable.js:110:10)
    at TCP.onread (net.js:523:20)

When you run it with node node_modules/.bin/db-migrate up --dry-run you'll see:

CREATE TABLE  "product_variant" ("id"  (10) SERIAL PRIMARY KEY NOT NULL, "product_id" INTEGER (10) NOT NULL);

Notice the "id" (10) in there. It's caused by this line:

https://github.com/db-migrate/pg/blob/master/index.js#L43

var type = spec.autoIncrement ? '' : this.mapDataType(spec.type);
var len = spec.length ? util.format('(%s)', spec.length) : '';

If autoincrement is true, type is an empty string. If you leave the limit off, it produces this:

CREATE TABLE  "product_variant" ("id"   SERIAL PRIMARY KEY NOT NULL, "product_id" INTEGER (10) NOT NULL);

Solution 1

Raise an error when both are specified.

There's no real reason to specify a length and autoincrement. Postgres allows you to specify bigserial as a datatype, but does not (to my knowledge) allow you to specify a serial/bigserial table with a length.

Solution 2

Silently ignore / warn users when length + autoincrement are specified in postgres.

Solution 3

Any other ideas?


Regardless, it seems like we should update the docs to not use the combination, since it's a bummer for new users when they copy the example and it blows up.

I'm happy to write the tests / submit the pull request / update the docs, but would like some guidance on how to handle this case.

Thanks!

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/28018706-createtable-errors-out-when-you-specify-length-and-autoincrement-in-postgres?utm_campaign=plugin&utm_content=tracker%2F73887&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F73887&utm_medium=issues&utm_source=github).
@wzrdtales
Copy link
Member

You're using the migrations in a wrong way!

This should be like this instead:

'use strict';

var dbm;
var type;
var seed;

/**
  * We receive the dbmigrate dependency from dbmigrate initially.
  * This enables us to not have to rely on NODE_PATH.
  */
exports.setup = function(options, seedLink) {
  dbm = options.dbmigrate;
  type = dbm.dataType;
  seed = seedLink;
};

exports.up = function(db) {
  return db.createTable( 'product_variant',
  {
      id:
      {
        type: 'int',
        unsigned: true,
        notNull: true,
        primaryKey: true,
        autoIncrement: true,
        length: 10
      },
      product_id:
      {
        type: 'int',
        unsigned: true,
        length: 10,
        notNull: true,
        foreignKey: {
          name: 'product_variant_product_id_fk',
          table: 'product',
          rules: {
            onDelete: 'CASCADE',
            onUpdate: 'RESTRICT'
          },
          mapping: 'id'
        }
      },
  });
};

exports.down = function(db) {
  return null;
};

the createTable method returns a Promise, also to ask: Do you use v0.10.x-beta already, this is not true for v0.9.x though.

And yes you're absolutely right, I'm currently mainly focusing on the docs and think a lot about how to structure them better and make the entrance as easy as possible. Up to the final release of v0.10.x the docs should be up to date though.

@wzrdtales wzrdtales self-assigned this Nov 7, 2015
@wzrdtales
Copy link
Member

For the real issue with this issue, this needs that to be added to the pg driver.

@andris310
Copy link

this bug is still hapening

@stale
Copy link

stale bot commented Nov 23, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 23, 2017
@stale stale bot closed this as completed Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants