Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Commit

Permalink
fix(graphql-migrations): migration failed for auto increments column
Browse files Browse the repository at this point in the history
Migrations failed if the field was not "id" and the scalar was "ID"
  • Loading branch information
Enda Phelan committed Sep 18, 2020
1 parent 2a1ffb9 commit 83a80cd
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 6 deletions.
3 changes: 2 additions & 1 deletion integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"rimraf": "3.0.2",
"ts-node": "9.0.0",
"tsutils": "3.17.1",
"typescript": "4.0.2"
"typescript": "4.0.2",
"graphql": "15.3.0"
},
"license": "Apache-2.0"
}
47 changes: 47 additions & 0 deletions integration/tests/graphql-migrations-postgres.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import * as Knex from "knex";

import { buildSchema } from 'graphql';
import { migrateDB } from "../../packages/graphql-migrations/src";
import { Operation } from "../../packages/graphql-migrations/src/diff/Operation";


const dbConfig: Knex.Config = {
client: "pg",
connection: {
user: "postgresql",
password: "postgres",
database: "users",
host: "localhost",
port: parseInt(process.env.POSTGRES_PORT, 10) || 5432
}
}

const db = Knex(dbConfig);

beforeEach(async () => {
await db.raw(`DROP SCHEMA public CASCADE;
CREATE SCHEMA public;
GRANT ALL ON SCHEMA public TO postgresql;
GRANT ALL ON SCHEMA public TO public;`);
})

afterAll(() => db.destroy());

test('consecutive migrations', async () => {
const model = buildSchema(`
"""@model"""
type User {
"""@id"""
_id: ID
name: String
}
`)

let ops = await migrateDB(dbConfig, model)
expect(ops.results).toHaveLength(3);
const opsResults = ops.results.map((op: Operation) => op.type);
expect(opsResults).toEqual(['table.create', 'table.primary.set', 'column.create'])

ops = await migrateDB(dbConfig, model)
expect(ops.results).toHaveLength(0);
})
9 changes: 5 additions & 4 deletions packages/graphql-migrations/src/connector/read.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import getTypeAlias from '../util/getTypeAlias'
import getUniques from '../util/getUniques'
import listTables from '../util/listTables'
import transformDefaultValue from '../util/transformDefaultValue'
import { isAutoIncrementableColumn } from '../util/isAutoIncrementableColumn'

/**
* @param {Config} config Knex configuration
Expand Down Expand Up @@ -92,13 +93,13 @@ class Reader {
const columnName = this.getColumnName(key)
if (!columnName) { continue }
const info = columnInfo[key];
const {args, type} = getTypeAlias(info.type, info.maxLength);
const { args, type } = getTypeAlias(info.type, info.maxLength);
const foreign = foreignKeys.find((k: any) => k.column === key);
const isPrimaryKey = primaries.some((primary: {column: string, indexName: string }) => primary.column === columnName);
const isPrimaryKey = primaries.some((primary: { column: string, indexName: string }) => primary.column === columnName);
const column: TableColumn = {
name: columnName,
isPrimaryKey,
autoIncrementable: type === "increments" || type === "bigIncrements", // TODO
autoIncrementable: isPrimaryKey && isAutoIncrementableColumn(this.config.client.toString(), info),
comment: this.getComment(columnComments, key),
annotations: {},
args,
Expand Down Expand Up @@ -175,7 +176,7 @@ class Reader {
return undefined
}

private getColumnNames(names: string[]): string [] {
private getColumnNames(names: string[]): string[] {
// @ts-ignore
return names.map((name: string) => this.getColumnName(name)).filter((n: string) => !!n)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-migrations/src/diff/computeDiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class Differ {

// Compare columns
for (const { fromCol, toCol } of sameColumnQueue) {
if (toCol.name === 'id') {
if (toCol.autoIncrementable || fromCol.autoIncrementable) {
const fromPrimaryKeyType = getKnexColumnType(fromCol.type);
const toPrimaryKeyType = getKnexColumnType(toCol.type);

Expand Down
21 changes: 21 additions & 0 deletions packages/graphql-migrations/src/util/isAutoIncrementableColumn.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Knex from 'knex';

/**
* Detect if the column is incrementable based on column information
*
* @param {string} client - database client
* @param {Knex.ColumnInfo} columnInfo - metadata for column
*/
export function isAutoIncrementableColumn (client: string, columnInfo: Knex.ColumnInfo): boolean {
switch (client) {
case 'pg':
return columnInfo.defaultValue?.toString().startsWith('nextval(');
case 'sqlite3':
// sqlite3 primary keys not supported
return false;
default:
console.warn(`${client} increments column detection not supported`)

return false;
}
}

0 comments on commit 83a80cd

Please sign in to comment.