knex: createTableIfNotExists generates alter table output which fails

I’m using knex 0.10.0, Postgres.

createTableIfNotExists does not create table but adding primary index fails (because it already exist).

How to avoid alter table request if table already exist?

  db.schema.createTableIfNotExists('users', function(t) {
    t.uuid('id').notNullable().primary();
    t.string('__type', 16).defaultTo('User');
    t.string('email', 100);
    t.string('pass_salt', 32);
    t.string('password_salted', 32);
    t.boolean('verified').defaultTo(false);
    t.json('details');
  })
  .then(() => {});

Generates

[ { sql: 'create table if not exists "users" ("id" uuid not null, "__type" varchar(16) default \'User\', "email" varchar(100), "pass_salt" varchar(32), "password_salted" varchar(32), "verified" boolean default \'0\', "details" json)',
    bindings: [] },
  { sql: 'alter table "users" add primary key ("id")',
    bindings: [] } ]
Unhandled rejection error: alter table "users" add primary key ("id") - multiple primary keys for table "users" are not allowed
    at Connection.parseE (/usr/src/letsplay/node_modules/pg/lib/connection.js:539:11)
    at Connection.parseMessage (/usr/src/letsplay/node_modules/pg/lib/connection.js:366:17)
    at Socket.<anonymous> (/usr/src/letsplay/node_modules/pg/lib/connection.js:105:22)
    at emitOne (events.js:90:13)
    at Socket.emit (events.js:182:7)
    at readableAddChunk (_stream_readable.js:153:18)
    at Socket.Readable.push (_stream_readable.js:111:10)
    at TCP.onread (net.js:529:20)

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 8
  • Comments: 28 (17 by maintainers)

Commits related to this issue

Most upvoted comments

@diversario The workaround I’m using is to check if the table exists (try knex.schema.hasTable(tableName)), and then if it doesn’t exist run the table and index creation code.

As per my earlier comments I still stand behind deprecating the whole createTableIfNotExists method since it causes only problems.

Using hasTable + adding all the initially necessary code of table creating inside that block is a lot more powerful and pretty much as easy as having that createTableIfNotExists for that specific single use case.

maybe createTableIfNotExists should just be a wrapper of hasTable and createTable

would be two queries, but more intuitive

@asadakbar it is not worth of breaking old migrations backwards compatibility when doing that change.

I wouldn’t say no to a new similar method, which is async, uses hasTable + createTable combo, but then again it is common use case too that inside the same .hasTable check you also want to run some more raw queries to make some specific index etc.

So you would again need again drop down to .hasTable + .createTable + .raw. For me creating raw queries in migrations is more common case than needing to check if table exist before trying to create it (I know when my DB has some table and when it does not).

That being said I’ve been thinking a lot about adding completely new more consistent migration API which doesn’t try to go too far with abstracting types (like json -> string etc. conversions that schema api is doing a lot) and which would have more reasonable default values etc. but I will post API proposal before starting to work on it (which might not happen even this year or never anyways).

@elhigu I agree that .hasTable plus the table creation code is fairly easy to write, but its not as readable and adds more code that needs to be repeated every time I am creating a table. The use case feels common enough that its worth making .createTableIfExists use .hasTable and .createTable under the hood.

@wubzz yep, probably most of the other DBs can also do something similar, but honestly I don’t think that the benefit is worth the trouble.

So documentation and discouraging use of the current implementation would be easiest solution to prevent people hurting themselves.

This new warning issued when createTableIfNotExists is called is also being shown when creating the migration tables during an initial migration - https://github.com/tgriesser/knex/blob/master/src/migrate/index.js#L156-L169

This is confusing for end users who may see this warning and not be able to find any usage of createTableIfNotExists within their client code.

Is there currently work underway to update Knex itself to not use createTableIfNotExists?

I agree. Otherwise, createTableIfNotExists can work with table without any index, which is useless.

Not sure how other dialects can deal with this problem, but speaking on behalf of postgres there is a possible solution which would work for both query-building and query-running purposes by combining the existing logic with the proposed solution of knex.schema.hasTable:

console.log(db.schema.createTableIfNotExists('users', function(t) {
	t.uuid('id').notNullable().primary();
	t.string('__type', 16).defaultTo('User');
	t.string('email', 100);
	t.string('pass_salt', 32);
	t.string('password_salted', 32);
	t.boolean('verified').defaultTo(false);
	t.json('details');
})
	.toString());
// DO
// $do$
// BEGIN
// IF NOT EXISTS (select * from information_schema.tables where table_name = 'users' and table_schema = current_schema) THEN
// create table "users" ("id" uuid not null, "__type" varchar(16) default 'User', "email" varchar(100), "pass_salt" varchar(32), "password_salted" varchar(32), "verified" boolean default '0', "details" json);
// ALTER TABLE "users" add primary key ("id");
// END IF;
// END
// $do$

Not a very neat solution, but still…

I just realized that my migration (seeding) is failing because of this. I’m trying to accomplish a pretty simple task: create a table if there isn’t one and insert rows, ignoring duplicates. I never get to the insert part because migration fails on createTableIfNotExists as it’s trying to add an index on an existing table.

I agree with discussions here and in other threads that callback shouldn’t be called in createTableIfNotExists – or, at least something should be done about errors resulting from calling that callback in such scenario.

Anyway, is there some workaround for this right now?