Skip to content

Commit

Permalink
Fix collation when renaming column in MySQL dialect (#2666)
Browse files Browse the repository at this point in the history 
Co-authored-by: Olivier Cavadenti <olivier.cavadenti@gmail.com>
  • Loading branch information
@straub @OlivierCavadenti
straub and OlivierCavadenti committed Feb 1, 2022
1 parent ffb6019 commit 81d6ffa
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 55 deletions.
7 changes: 5 additions & 2 deletions lib/dialects/mysql/schema/mysql-tablecompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class TableCompiler_MySQL extends TableCompiler {

this.pushQuery({
sql:
`show fields from ${table} where field = ` +
`show full fields from ${table} where field = ` +
this.client.parameter(from, this.tableBuilder, this.bindingsHolder),
output(resp) {
const column = resp[0];
Expand Down Expand Up @@ -101,7 +101,10 @@ class TableCompiler_MySQL extends TableCompiler {
if (column.Default !== void 0 && column.Default !== null) {
sql += ` DEFAULT '${column.Default}'`;
}
// Add back the auto increment if the column had it, fix issue #2767
if (column.Collation !== void 0 && column.Collation !== null) {
sql += ` COLLATE '${column.Collation}'`;
}
// Add back the auto increment if the column it, fix issue #2767
if (column.Extra == 'auto_increment') {
sql += ` AUTO_INCREMENT`;
}
Expand Down
118 changes: 65 additions & 53 deletions test/integration2/query/misc/additional.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ describe('Additional', function () {
});
}

it('should allow renaming a column', function () {
it('should allow renaming a column', async function () {
let countColumn;
if (isOracle(knex)) {
countColumn = 'COUNT(*)';
Expand All @@ -542,7 +542,10 @@ describe('Additional', function () {
countColumn = 'count(*)';
}

let count;
await dropTables(knex);
await createAccounts(knex, false, false);
await createTestTableTwo(knex);

const inserts = [];
_.times(40, function (i) {
inserts.push({
Expand All @@ -551,59 +554,68 @@ describe('Additional', function () {
last_name: 'Data',
});
});
return knex('accounts')
.insert(inserts)
.then(function () {
return knex.count('*').from('accounts');
})
.then(function (resp) {
count = resp[0][countColumn];
return knex.schema
.table('accounts', function (t) {
t.renameColumn('about', 'about_col');
})
.testSql(function (tester) {
tester('mysql', [
'show fields from `accounts` where field = ?',
]);
tester('pg', [
'alter table "accounts" rename "about" to "about_col"',
]);
tester('pgnative', [
'alter table "accounts" rename "about" to "about_col"',
]);
tester('pg-redshift', [
'alter table "accounts" rename "about" to "about_col"',
]);
tester('sqlite3', [
'alter table `accounts` rename `about` to `about_col`',
]);
tester('oracledb', [
'DECLARE PK_NAME VARCHAR(200); IS_AUTOINC NUMBER := 0; BEGIN EXECUTE IMMEDIATE (\'ALTER TABLE "accounts" RENAME COLUMN "about" TO "about_col"\'); SELECT COUNT(*) INTO IS_AUTOINC from "USER_TRIGGERS" where trigger_name = \'accounts_autoinc_trg\'; IF (IS_AUTOINC > 0) THEN SELECT cols.column_name INTO PK_NAME FROM all_constraints cons, all_cons_columns cols WHERE cons.constraint_type = \'P\' AND cons.constraint_name = cols.constraint_name AND cons.owner = cols.owner AND cols.table_name = \'accounts\'; IF (\'about_col\' = PK_NAME) THEN EXECUTE IMMEDIATE (\'DROP TRIGGER "accounts_autoinc_trg"\'); EXECUTE IMMEDIATE (\'create or replace trigger "accounts_autoinc_trg" BEFORE INSERT on "accounts" for each row declare checking number := 1; begin if (:new."about_col" is null) then while checking >= 1 loop select "accounts_seq".nextval into :new."about_col" from dual; select count("about_col") into checking from "accounts" where "about_col" = :new."about_col"; end loop; end if; end;\'); end if; end if;END;',
]);
tester('mssql', ["exec sp_rename ?, ?, 'COLUMN'"]);
});
})
.then(function () {
return knex.count('*').from('accounts');
})
.then(function (resp) {
expect(resp[0][countColumn]).to.equal(count);
})
.then(function () {
return knex('accounts').select('about_col');
})
.then(function () {
return knex.schema.table('accounts', function (t) {
t.renameColumn('about_col', 'about');
});
})
.then(function () {
return knex.count('*').from('accounts');
await knex('accounts').insert(inserts);

let aboutCol;
if (isMysql(knex)) {
const metadata = await knex.raw('SHOW FULL COLUMNS FROM accounts');
// Store the column metadata
aboutCol = metadata[0].filter((t) => t.Field === 'about')[0];
delete aboutCol.Field;
}

const count = (await knex.count('*').from('accounts'))[0][
countColumn
];

await knex.schema
.table('accounts', function (t) {
t.renameColumn('about', 'about_col');
})
.then(function (resp) {
expect(resp[0][countColumn]).to.equal(count);
.testSql(function (tester) {
tester('mysql', [
'show full fields from `accounts` where field = ?',
]);
tester('pg', [
'alter table "accounts" rename "about" to "about_col"',
]);
tester('pgnative', [
'alter table "accounts" rename "about" to "about_col"',
]);
tester('pg-redshift', [
'alter table "accounts" rename "about" to "about_col"',
]);
tester('sqlite3', [
'alter table `accounts` rename `about` to `about_col`',
]);
tester('oracledb', [
'DECLARE PK_NAME VARCHAR(200); IS_AUTOINC NUMBER := 0; BEGIN EXECUTE IMMEDIATE (\'ALTER TABLE "accounts" RENAME COLUMN "about" TO "about_col"\'); SELECT COUNT(*) INTO IS_AUTOINC from "USER_TRIGGERS" where trigger_name = \'accounts_autoinc_trg\'; IF (IS_AUTOINC > 0) THEN SELECT cols.column_name INTO PK_NAME FROM all_constraints cons, all_cons_columns cols WHERE cons.constraint_type = \'P\' AND cons.constraint_name = cols.constraint_name AND cons.owner = cols.owner AND cols.table_name = \'accounts\'; IF (\'about_col\' = PK_NAME) THEN EXECUTE IMMEDIATE (\'DROP TRIGGER "accounts_autoinc_trg"\'); EXECUTE IMMEDIATE (\'create or replace trigger "accounts_autoinc_trg" BEFORE INSERT on "accounts" for each row declare checking number := 1; begin if (:new."about_col" is null) then while checking >= 1 loop select "accounts_seq".nextval into :new."about_col" from dual; select count("about_col") into checking from "accounts" where "about_col" = :new."about_col"; end loop; end if; end;\'); end if; end if;END;',
]);
tester('mssql', ["exec sp_rename ?, ?, 'COLUMN'"]);
});

if (isMysql(knex)) {
const values = await knex.raw('SHOW FULL COLUMNS FROM accounts');
const newAboutCol = values[0].filter(
(t) => t.Field === 'about_col'
)[0];
// Check if all metadata excepted the Field name (DEFAULT, COLLATION, EXTRA, etc.) are preserved after rename.
delete newAboutCol.Field;
expect(aboutCol).to.eql(newAboutCol);
}

const countAfterRename = (await knex.count('*').from('accounts'))[0][
countColumn
];
expect(countAfterRename).to.equal(count);

await knex.schema.table('accounts', function (t) {
t.renameColumn('about_col', 'about');
});
const countOrigin = (await knex.count('*').from('accounts'))[0][
countColumn
];
expect(countOrigin).to.equal(count);
});

it('should allow dropping a column', async function () {
Expand Down

0 comments on commit 81d6ffa

Please sign in to comment.