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.

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

Missing comments on delete, update and insert #5738

Merged
merged 7 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions lib/dialects/mysql/query/mysql-querycompiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,21 @@ class QueryCompiler_MySQL extends QueryCompiler {

this._emptyInsertValue = '() values ()';
}
// Compiles an `delete` allowing comments
del() {
const sql = super.del();
if (sql === '') return sql;
const comments = this.comments();
return (comments === '' ? '' : comments + ' ') + sql;
}

// Compiles an `insert` query, allowing for multiple
// inserts using a single query statement.
insert() {
let sql = super.insert();
if (sql === '') return sql;
const comments = this.comments();
sql = (comments === '' ? '' : comments + ' ') + sql;

const { ignore, merge, insert } = this.single;
if (ignore) sql = sql.replace('insert into', 'insert ignore into');
Expand Down Expand Up @@ -86,13 +95,15 @@ class QueryCompiler_MySQL extends QueryCompiler {

// Update method, including joins, wheres, order & limits.
update() {
const comments = this.comments();
const withSQL = this.with();
const join = this.join();
const updates = this._prepUpdate(this.single.update);
const where = this.where();
const order = this.order();
const limit = this.limit();
return (
(comments === '' ? '' : comments + ' ') +
withSQL +
`update ${this.tableName}` +
(join ? ` ${join}` : '') +
Expand Down
15 changes: 15 additions & 0 deletions test/integration/query/deletes.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,21 @@ module.exports = function (knex) {
});
});

it('#5738 should handle deletes with comments', function () {
return knex('accounts')
.where('id', 1)
.del()
.comment('removing acccount')
.testSql(function (tester) {
tester(
'mysql',
'/* removing acccount */ delete from `accounts` where `id` = ?',
[1],
0
);
});
});

describe('Delete with join', function () {
it('should handle basic delete with join', async function () {
const query = knex('test_table_two')
Expand Down
14 changes: 14 additions & 0 deletions test/integration2/query/insert/inserts.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,20 @@ describe('Inserts', function () {
});
});

it('#5738 should handle insert with comments', async function () {
await knex('test_default_table')
.insert({}, 'id')
.comment('insert into test_default_table')
.testSql(function (tester) {
tester(
'mysql',
'/* insert into test_default_table */ insert into `test_default_table` () values ()',
[],
[2]
);
});
});

it('should handle empty arrays inserts', async function () {
await knex.schema.createTable('test_default_table2', function (qb) {
qb.increments().primary();
Expand Down
19 changes: 19 additions & 0 deletions test/integration2/query/update/updates.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,25 @@ describe('Updates', function () {
});
});

it('#5738 should handle update with comments', async function () {
await knex('accounts')
.where('id', 1)
.update({
first_name: 'User',
last_name: 'Test',
email: 'test100@example.com',
})
.comment('update in account')
.testSql(function (tester) {
tester(
'mysql',
'/* update in account */ update `accounts` set `first_name` = ?, `last_name` = ?, `email` = ? where `id` = ?',
['User', 'Test', 'test100@example.com', 1],
1
);
});
});

it('should allow for null updates', async function () {
await knex('accounts')
.where('id', 1000)
Expand Down
49 changes: 49 additions & 0 deletions test/unit/query/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -9508,6 +9508,55 @@ describe('QueryBuilder', () => {
);
});

it('#5738 - should allow query comments in querybuilder for insert statement', () => {
testsql(
qb()
.into('users')
.insert({ email: 'foo' })
.comment('Added comment 1')
.comment('Added comment 2'),

{
mysql: {
sql: '/* Added comment 1 */ /* Added comment 2 */ insert into `users` (`email`) values (?)',
bindings: ['foo'],
},
}
);
});

it('#5738 - should allow query comments in querybuilder for update statement', () => {
testsql(
qb()
.from('users')
.update({ email: 'foo' })
.comment('Added comment 1')
.comment('Added comment 2'),
{
mysql: {
sql: '/* Added comment 1 */ /* Added comment 2 */ update `users` set `email` = ?',
bindings: ['foo'],
},
}
);
});

it('#5738 - should allow query comments in querybuilder for delete statement', () => {
testsql(
qb()
.from('users')
.delete()
.comment('Added comment 1')
.comment('Added comment 2'),
{
mysql: {
sql: '/* Added comment 1 */ /* Added comment 2 */ delete from `users`',
bindings: [],
},
}
);
});

it('#1982 (2) - should throw error on non string', () => {
try {
testsql(qb().from('testtable').comment({ prop: 'val' }), {
Expand Down