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

Fix collation when renaming column in MySQL dialect #2666

Merged
merged 4 commits into from
Feb 1, 2022
Merged

Fix collation when renaming column in MySQL dialect #2666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad56ff0
Fix collation when renaming column in MySQL dialect
straub Jun 20, 2018
f3fd0cf
Fix mysql .renameColumn() test
straub Jun 20, 2018
777d4c7
Merge remote-tracking branch 'upstream/master' into pr/2666
OlivierCavadenti Feb 1, 2022
093a802
rebase + rework rename col test
OlivierCavadenti Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/dialects/mysql/schema/mysql-tablecompiler.js
View file
Open in desktop
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
Open in desktop
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