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 for pg dialect #5793

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
12 changes: 7 additions & 5 deletions lib/dialects/postgres/query/pg-querycompiler.js
Expand Up @@ -19,15 +19,17 @@ class QueryCompiler_PG extends QueryCompiler {

// Compiles a truncate query.
truncate() {
return `truncate ${this.tableName} restart identity`;
let sql = `${this.comments()} truncate ${this.tableName} restart identity`;
return sql.trimStart();
}

// is used if the an array with multiple empty values supplied

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use `` here as well

sql = sql.trimStart;
if (sql === '') return sql;

const { returning, onConflict, ignore, merge, insert } = this.single;
Expand All @@ -54,7 +56,7 @@ class QueryCompiler_PG extends QueryCompiler {
return {
sql:
withSQL +
`update ${this.single.only ? 'only ' : ''}${this.tableName} ` +
(`${this.comments()} update ${this.single.only ? 'only ' : ''}${this.tableName} `).trimStart() +
`set ${updateData.join(', ')}` +
this._updateFrom(updateFrom) +
(wheres ? ` ${wheres}` : '') +
Expand Down Expand Up @@ -124,12 +126,12 @@ class QueryCompiler_PG extends QueryCompiler {
// With 'using' syntax, no tablename between DELETE and FROM.
const sql =
withSQL +
`delete from ${this.single.only ? 'only ' : ''}${tableName}` +
`${this.comments()} delete from ${this.single.only ? 'only ' : ''}${tableName}` +
(using ? ` ${using}` : '') +
(wheres ? ` ${wheres}` : '');
const { returning } = this.single;
return {
sql: sql + this._returning(returning),
sql: sql.trimStart() + this._returning(returning),
returning,
};
}
Expand Down
13 changes: 13 additions & 0 deletions sourcesknex/seeds/20240123154439_somename.js
@@ -0,0 +1,13 @@
/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.seed = async function(knex) {
// Deletes ALL existing entries
await knex('table_name').del()
await knex('table_name').insert([
{id: 1, colName: 'rowValue1'},
{id: 2, colName: 'rowValue2'},
{id: 3, colName: 'rowValue3'}
]);
};
6 changes: 6 additions & 0 deletions test/integration/query/deletes.js
Expand Up @@ -98,6 +98,12 @@ module.exports = function (knex) {
[1],
0
);
tester(
'pg',
'/* removing acccount */ delete from "accounts" where "id" = ?',
[1],
0
);
});
});

Expand Down
13 changes: 10 additions & 3 deletions test/integration2/query/insert/inserts.spec.js
Expand Up @@ -925,6 +925,13 @@ describe('Inserts', function () {
[],
[2]
);
tester(
'pg',
'/* insert into test_default_table */ insert into "test_default_table" () values ()',
'insert into "test_default_table" default values returning "id"',
[],
[{ id: 1 }]
);
});
});

Expand Down Expand Up @@ -2012,8 +2019,8 @@ describe('Inserts', function () {
});
await knex.raw(
'create unique index email_type1 ' +
'on upsert_tests(email) ' +
"where type = 'type1'"
'on upsert_tests(email) ' +
"where type = 'type1'"
);

await knex('upsert_tests').insert([
Expand Down Expand Up @@ -2056,7 +2063,7 @@ describe('Inserts', function () {
tester(
'sqlite3',
'insert into `upsert_tests` (`email`, `name`, `type`) select ? as `email`, ? as `name`, ? as `type` union all select ? as `email`, ? as `name`, ? as `type` union all select ? as `email`, ? as `name`, ? as `type` where true ' +
"on conflict (email) where type = 'type1' do update set `email` = excluded.`email`, `name` = excluded.`name`, `type` = excluded.`type`",
"on conflict (email) where type = 'type1' do update set `email` = excluded.`email`, `name` = excluded.`name`, `type` = excluded.`type`",
[
'one@example.com',
'AFTER',
Expand Down
6 changes: 6 additions & 0 deletions test/integration2/query/update/updates.spec.js
Expand Up @@ -100,6 +100,12 @@ describe('Updates', function () {
['User', 'Test', 'test100@example.com', 1],
1
);
tester(
'pg',
'/* update in account */ update "accounts" set "first_name" = ?, "last_name" = ?, "email" = ? where "id" = ?',
['User', 'Test', 'test100@example.com', 1],
1
);
});
});

Expand Down
20 changes: 16 additions & 4 deletions test/unit/query/builder.js
Expand Up @@ -9521,6 +9521,10 @@ describe('QueryBuilder', () => {
sql: '/* Added comment 1 */ /* Added comment 2 */ insert into `users` (`email`) values (?)',
bindings: ['foo'],
},
pg: {
sql: '/* Added comment 1 */ /* Added comment 2 */ insert into "users" ("email") values (?)',
bindings: ['foo'],
},
}
);
});
Expand All @@ -9537,6 +9541,10 @@ describe('QueryBuilder', () => {
sql: '/* Added comment 1 */ /* Added comment 2 */ update `users` set `email` = ?',
bindings: ['foo'],
},
pg: {
sql: '/* Added comment 1 */ /* Added comment 2 */ update "users" set "email" = ?',
bindings: ['foo'],
},
}
);
});
Expand All @@ -9553,6 +9561,10 @@ describe('QueryBuilder', () => {
sql: '/* Added comment 1 */ /* Added comment 2 */ delete from `users`',
bindings: [],
},
pg: {
sql: '/* Added comment 1 */ /* Added comment 2 */ delete from "users"',
bindings: [],
},
}
);
});
Expand Down Expand Up @@ -9835,8 +9847,8 @@ describe('QueryBuilder', () => {
} catch (error) {
expect(error.message).to.contain(
'Undefined binding(s) detected when compiling ' +
builder._method.toUpperCase() +
`. Undefined column(s): [${undefinedColumns.join(', ')}] query:`
builder._method.toUpperCase() +
`. Undefined column(s): [${undefinedColumns.join(', ')}] query:`
); //This test is not for asserting correct queries
}
});
Expand All @@ -9856,8 +9868,8 @@ describe('QueryBuilder', () => {
} else {
expect(error.message).to.contain(
'Undefined binding(s) detected when compiling ' +
builder._method.toUpperCase() +
`. Undefined column(s): [${undefinedColumns.join(', ')}] query:`
builder._method.toUpperCase() +
`. Undefined column(s): [${undefinedColumns.join(', ')}] query:`
); //This test is not for asserting correct queries
}
}
Expand Down