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.

Sign up for GitHub

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

Jump to bottom

Lift up dialect specific methods in the CreateTableBuilder #3532

Merged
merged 3 commits into from
Nov 21, 2019
Merged

Lift up dialect specific methods in the CreateTableBuilder #3532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
954c170
Lift up dialect specific methods in the CreateTableBuilder
lorefnon Nov 10, 2019
53441fa
Fix tests
lorefnon Nov 12, 2019
4615ef3
Fix accidental change affecting oracle tests
lorefnon Nov 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions lib/schema/tablebuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,12 @@ each(specialMethods, function(methods, dialect) {
each(methods, function(method) {
TableBuilder.prototype[method] = function(value) {
if (this.client.dialect !== dialect) {
this.client.logger.warn(
`Knex only supports ${method} statement with ${dialect}.`
);
throw new Error(`Knex only supports ${method} statement with ${dialect}.`);
}
if (this._method === 'alter') {
this.client.logger.warn(
throw new Error(
`Knex does not support altering the ${method} outside of create ` +
`table, please use knex.raw statement.`
`table, please use knex.raw statement.`
);
}
this._single[method] = value;
Expand Down
25 changes: 15 additions & 10 deletions test/integration/schema/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ module.exports = function(knex) {
it('accepts the table name, and a "container" function', function() {
return knex.schema
.createTable('test_table_one', function(table) {
table.engine('InnoDB');
if (/mysql/i.test(knex.client.driverName)) table.engine('InnoDB');
table.comment('A table comment.');
table.bigIncrements('id');
table.string('first_name').index();
Expand Down Expand Up @@ -438,7 +438,9 @@ module.exports = function(knex) {
it('is possible to set the db engine with the table.engine', function() {
return knex.schema
.createTable('test_table_two', function(table) {
table.engine('InnoDB');
if (/mysql/i.test(knex.client.driverName)) {
table.engine('InnoDB');
}
table.increments();
table.integer('account_id');
if (knex.client.driverName === 'oracledb') {
Expand All @@ -454,9 +456,6 @@ module.exports = function(knex) {
tester('mysql', [
'create table `test_table_two` (`id` int unsigned not null auto_increment primary key, `account_id` int, `details` text, `status` tinyint) default character set utf8 engine = InnoDB',
]);
tester('mssql', [
'CREATE TABLE [test_table_two] ([id] int identity(1,1) not null primary key, [account_id] int, [details] nvarchar(max), [status] tinyint)',
]);
});
});

Expand All @@ -465,7 +464,9 @@ module.exports = function(knex) {
const defaultDetails = { b: { d: 20 } };
return knex.schema
.createTable('test_table_three', function(table) {
table.engine('InnoDB');
if (/mysql/i.test(knex.client.driverName)) {
table.engine('InnoDB');
}
table
.integer('main')
.notNullable()
Expand Down Expand Up @@ -533,7 +534,9 @@ module.exports = function(knex) {
it('handles numeric length correctly', function() {
return knex.schema
.createTable('test_table_numerics', function(table) {
table.engine('InnoDB');
if (/mysql/i.test(knex.client.driverName)) {
table.engine('InnoDB');
}
table.integer('integer_column', 5);
table.tinyint('tinyint_column', 5);
table.smallint('smallint_column', 5);
Expand Down Expand Up @@ -737,9 +740,11 @@ module.exports = function(knex) {
it('is possible to set the table collation with table.charset and table.collate', function() {
return knex.schema
.createTable('charset_collate_test', function(table) {
table.charset('latin1');
table.collate('latin1_general_ci');
table.engine('InnoDB');
if (/mysql/i.test(knex.client.driverName)) {
table.charset('latin1');
table.collate('latin1_general_ci');
table.engine('InnoDB');
}
table.increments();
table.integer('account_id');
table.text('details');
Expand Down
26 changes: 10 additions & 16 deletions test/unit/schema/mssql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,18 @@ describe('MSSQL SchemaBuilder', function() {
let tableSql;
const equal = require('assert').equal;

it('test basic create table with charset and collate', function() {
tableSql = client.schemaBuilder().createTable('users', function(table) {
table.increments('id');
table.string('email');
table.charset('utf8');
table.collate('utf8_unicode_ci');
});

equal(1, tableSql.toSQL().length);
expect(tableSql.toSQL()[0].sql).to.equal(
'CREATE TABLE [users] ([id] int identity(1,1) not null primary key, [email] nvarchar(255))'
);
expect(tableSql.toQuery()).to.equal(
'CREATE TABLE [users] ([id] int identity(1,1) not null primary key, [email] nvarchar(255))'
);
it('throws when charset and collate are specified', function() {
expect(() => {
tableSql = client.schemaBuilder().createTable('users', function(table) {
table.increments('id');
table.string('email');
table.charset('utf8');
table.collate('utf8_unicode_ci');
}).toSQL();
}).to.throw('Knex only supports charset statement with mysql');
});

it('basic create table without charset or collate', function() {
it('basic create table', function() {
tableSql = client
.schemaBuilder()
.table('users', function() {
Expand Down
19 changes: 9 additions & 10 deletions test/unit/schema/postgres.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1318,16 +1318,15 @@ describe('PostgreSQL SchemaBuilder', function() {
});

it('should warn on disallowed method', function() {
tableSql = client
.schemaBuilder()
.createTable('users', function(t) {
t.string('username');
t.engine('myISAM');
})
.toSQL();
expect(tableSql[0].sql).to.equal(
'create table "users" ("username" varchar(255))'
);
expect(() => {
tableSql = client
.schemaBuilder()
.createTable('users', function(t) {
t.string('username');
t.engine('myISAM');
})
.toSQL();
}).to.throw('Knex only supports engine statement with mysql');
});

it('#1430 - .primary & .dropPrimary takes columns and constraintName', function() {
Expand Down
46 changes: 22 additions & 24 deletions test/unit/schema/redshift.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,30 +739,28 @@ describe('Redshift SchemaBuilder', function() {
expect(sql[0].sql).to.equal('drop extension if exists "test"');
});

it('table inherits another table', function() {
tableSql = client
.schemaBuilder()
.createTable('inheriteeTable', function(t) {
t.string('username');
t.inherits('inheritedTable');
})
.toSQL();
expect(tableSql[0].sql).to.equal(
'create table "inheriteeTable" ("username" varchar(255)) like ("inheritedTable")'
);
});

it('should warn on disallowed method', function() {
tableSql = client
.schemaBuilder()
.createTable('users', function(t) {
t.string('username');
t.engine('myISAM');
})
.toSQL();
expect(tableSql[0].sql).to.equal(
'create table "users" ("username" varchar(255))'
);
it('does not support table inheritance', function() {
expect(() => {
client
.schemaBuilder()
.createTable('inheriteeTable', function(t) {
t.string('username');
t.inherits('inheritedTable');
})
.toSQL();
}).to.throw('Knex only supports inherits statement with postgresql');
});

it('should throw on usage of disallowed method', function() {
expect(() => {
client
.schemaBuilder()
.createTable('users', function(t) {
t.string('username');
t.engine('myISAM');
})
.toSQL();
}).to.throw('Knex only supports engine statement with mysql');
});

it('#1430 - .primary & .dropPrimary takes columns and constraintName', function() {
Expand Down
11 changes: 1 addition & 10 deletions types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1554,24 +1554,15 @@ declare namespace Knex {
queryContext(context: any): TableBuilder;
}

interface CreateTableBuilder extends TableBuilder {}

interface MySqlTableBuilder extends CreateTableBuilder {
interface CreateTableBuilder extends TableBuilder {
engine(val: string): CreateTableBuilder;
charset(val: string): CreateTableBuilder;
collate(val: string): CreateTableBuilder;
}

interface PostgreSqlTableBuilder extends CreateTableBuilder {
inherits(val: string): CreateTableBuilder;
}

interface AlterTableBuilder extends TableBuilder {}

interface MySqlAlterTableBuilder extends AlterTableBuilder {}

interface PostgreSqlAlterTableBuilder extends AlterTableBuilder {}

interface ColumnBuilder {
index(indexName?: string): ColumnBuilder;
primary(constraintName?: string): ColumnBuilder;
Expand Down