Skip to content

Commit

Permalink
Add alterType and update index.d.ts for alter function (#4967)
Browse files Browse the repository at this point in the history
Co-authored-by: intech <ru31337@gmail.com>
  • Loading branch information
OlivierCavadenti and intech committed Jan 27, 2022
1 parent 2467db1 commit 2e1016e
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 17 deletions.
21 changes: 12 additions & 9 deletions lib/dialects/postgres/schema/pg-tablecompiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,25 +91,28 @@ class TableCompiler_PG extends TableCompiler {

// To alter enum columns they must be cast to text first
const isEnum = col.type === 'enu';

const alterNullable = col.columnBuilder.alterNullable;

this.pushQuery({
sql: `alter table ${quotedTableName} alter column ${colName} drop default`,
bindings: [],
});

const alterNullable = col.columnBuilder.alterNullable;
if (alterNullable) {
this.pushQuery({
sql: `alter table ${quotedTableName} alter column ${colName} drop not null`,
bindings: [],
});
}
this.pushQuery({
sql: `alter table ${quotedTableName} alter column ${colName} type ${type} using (${colName}${
isEnum ? '::text::' : '::'
}${type})`,
bindings: [],
});

const alterType = col.columnBuilder.alterType;
if (alterType) {
this.pushQuery({
sql: `alter table ${quotedTableName} alter column ${colName} type ${type} using (${colName}${
isEnum ? '::text::' : '::'
}${type})`,
bindings: [],
});
}

const defaultTo = col.modified['defaultTo'];
if (defaultTo) {
Expand Down
6 changes: 5 additions & 1 deletion lib/schema/columnbuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,13 @@ AlterMethods.alterType = function (type) {
};

// Set column method to alter (default is add).
AlterMethods.alter = function ({ alterNullable = true } = {}) {
AlterMethods.alter = function ({
alterNullable = true,
alterType = true,
} = {}) {
this._method = 'alter';
this.alterNullable = alterNullable;
this.alterType = alterType;

return this;
};
Expand Down
23 changes: 17 additions & 6 deletions test/integration2/schema/set-nullable.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
isMysql,
isBetterSQLite3,
isOracle,
isCockroachDB,
} = require('../../util/db-helpers');
const { getAllDbs, getKnexForDb } = require('../util/knex-instance-provider');

Expand Down Expand Up @@ -76,7 +77,7 @@ describe('Schema', () => {
});

it('should throw error if alter a not nullable column with primary key #4401', async function () {
if (!isPostgreSQL(knex)) {
if (!(isPostgreSQL(knex) || isCockroachDB(knex))) {
this.skip();
}
await knex.schema.dropTableIfExists('primary_table_null');
Expand All @@ -91,15 +92,25 @@ describe('Schema', () => {
} catch (e) {
error = e;
}
expect(error.message).to.eq(
'alter table "primary_table_null" alter column "id_not_nullable_primary" drop not null - column "id_not_nullable_primary" is in a primary key'
);
if (isCockroachDB(knex)) {
// TODO: related comment in issue https://github.com/cockroachdb/cockroach/issues/49329#issuecomment-1022120446
expect(error.message).to.eq(
'alter table "primary_table_null" alter column "id_not_nullable_primary" drop not null - column "id_not_nullable_primary" is in a primary index'
);
} else {
expect(error.message).to.eq(
'alter table "primary_table_null" alter column "id_not_nullable_primary" drop not null - column "id_not_nullable_primary" is in a primary key'
);
}
});

it('should not throw error if alter a not nullable column with primary key with alterNullable is false #4401', async function () {
if (!isPostgreSQL(knex)) {
// TODO: related issue for cockroach https://github.com/cockroachdb/cockroach/issues/47636
if (!(isPostgreSQL(knex) || isCockroachDB(knex))) {
this.skip();
}
// With CoackroachDb we don't alter type (not supported).
const alterType = isPostgreSQL(knex);
await knex.schema.dropTableIfExists('primary_table_null');
await knex.schema.createTable('primary_table_null', (table) => {
table.integer('id_not_nullable_primary').notNullable().primary();
Expand All @@ -108,7 +119,7 @@ describe('Schema', () => {
await knex.schema.table('primary_table_null', (table) => {
table
.integer('id_not_nullable_primary')
.alter({ alterNullable: false });
.alter({ alterNullable: false, alterType: alterType });
});
});
});
Expand Down
20 changes: 20 additions & 0 deletions test/unit/schema-builder/postgres.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,26 @@ describe('PostgreSQL SchemaBuilder', function () {
);
});

it('should not alter type when alterType is false', function () {
tableSql = client
.schemaBuilder()
.table('users', function () {
this.string('foo').default('foo').alter({ alterType: false });
})
.toSQL();

equal(3, tableSql.length);
expect(tableSql[0].sql).to.equal(
'alter table "users" alter column "foo" drop default'
);
expect(tableSql[1].sql).to.equal(
'alter table "users" alter column "foo" drop not null'
);
expect(tableSql[2].sql).to.equal(
'alter table "users" alter column "foo" set default \'foo\''
);
});

it('alter table with schema', function () {
tableSql = client
.schemaBuilder()
Expand Down
2 changes: 1 addition & 1 deletion types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,7 @@ export declare namespace Knex {
notNullable(): ColumnBuilder;
nullable(): ColumnBuilder;
comment(value: string): ColumnBuilder;
alter(): ColumnBuilder;
alter(options: Readonly<{alterNullable?: boolean, alterType?: boolean}>): ColumnBuilder;
queryContext(context: any): ColumnBuilder;
after(columnName: string): ColumnBuilder;
first(): ColumnBuilder;
Expand Down

0 comments on commit 2e1016e

Please sign in to comment.