Skip to content

Commit

Permalink
bug(postgres/truncate) Fix truncate with schema in postgres. Closes #…
Browse files Browse the repository at this point in the history 
…4306
  • Loading branch information
@janmeier
janmeier committed Feb 17, 2016
1 parent 2802191 commit 35671ec
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 150 deletions.
1 change: 1 addition & 0 deletions changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- [FIXED] `updatedAt` and `createdAt` values are now set before validation [#5367](https://github.com/sequelize/sequelize/pull/5367)
- [FIXED] `describeTable` maintains proper enum casing in mysql [#5321](https://github.com/sequelize/sequelize/pull/5321)
- [FIXED] Parsing of dates in MySQL, when a named timezone is used [#4208](https://github.com/sequelize/sequelize/issues/4208)
- [FIXED] Truncating in Postgres, when table has a schema [#4306](https://github.com/sequelize/sequelize/issues/4306)

# 3.19.0
- [ADDED] Geography support for postgres
Expand Down
50 changes: 16 additions & 34 deletions lib/dialects/postgres/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ var Utils = require('../../utils')
, util = require('util')
, DataTypes = require('../../data-types')
, AbstractQueryGenerator = require('../abstract/query-generator')
, primaryKeys = {}
, semver = require('semver')
, _ = require('lodash');

Expand Down Expand Up @@ -42,8 +41,6 @@ var QueryGenerator = {
options = Utils._.extend({
}, options || {});

primaryKeys[tableName] = [];

var databaseVersion = Utils._.get(self, 'sequelize.options.databaseVersion', 0);
//Postgres 9.0 does not support CREATE TABLE IF NOT EXISTS, 9.1 and above do
var query = 'CREATE TABLE ' +
Expand All @@ -64,7 +61,7 @@ var QueryGenerator = {
attributes[attr] = attributes[attr].substring(0, i);
}

var dataType = this.pgDataTypeMapping(tableName, attr, attributes[attr]);
var dataType = this.dataTypeMapping(tableName, attr, attributes[attr]);
attrStr.push(this.quoteIdentifier(attr) + ' ' + dataType);
}

Expand All @@ -82,9 +79,12 @@ var QueryGenerator = {
});
}

var pks = primaryKeys[tableName].map(function(pk) {
return this.quoteIdentifier(pk);
}.bind(this)).join(',');
var pks = _.reduce(attributes, function (acc, attribute, key) {
if (_.includes(attribute, 'PRIMARY KEY')) {
acc.push(this.quoteIdentifier(key));
}
return acc;
}.bind(this), []).join(',');

if (pks.length > 0) {
values.attributes += ', PRIMARY KEY (' + pks + ')';
Expand Down Expand Up @@ -189,7 +189,7 @@ var QueryGenerator = {

attribute = Utils._.template('<%= key %> <%= definition %>')({
key: this.quoteIdentifier(key),
definition: this.pgDataTypeMapping(table, key, dbDataType)
definition: this.dataTypeMapping(table, key, dbDataType)
});

return Utils._.template(query)({
Expand All @@ -211,7 +211,7 @@ var QueryGenerator = {
, sql = [];

for (var attributeName in attributes) {
var definition = this.pgDataTypeMapping(tableName, attributeName, attributes[attributeName]);
var definition = this.dataTypeMapping(tableName, attributeName, attributes[attributeName]);
var attrSql = '';

if (definition.indexOf('NOT NULL') > 0) {
Expand Down Expand Up @@ -342,10 +342,10 @@ var QueryGenerator = {

options = options || {};

tableName = Utils.removeTicks(this.quoteTable(tableName), '"');
tableName = this.quoteTable(tableName);

if (options.truncate === true) {
query = 'TRUNCATE ' + QueryGenerator.quoteIdentifier(tableName);
query = 'TRUNCATE ' + tableName;

if (options.cascade) {
query += ' CASCADE';
Expand All @@ -358,34 +358,21 @@ var QueryGenerator = {
options.limit = 1;
}

primaryKeys[tableName] = primaryKeys[tableName] || [];

if (!!model && primaryKeys[tableName].length < 1) {
primaryKeys[tableName] = Utils._.map(Object.keys(model.primaryKeys), function(k){
return model.primaryKeys[k].field;
});
}

if (options.limit) {
query = 'DELETE FROM <%= table %> WHERE <%= primaryKeys %> IN (SELECT <%= primaryKeysSelection %> FROM <%= table %><%= where %><%= limit %>)';
} else {
query = 'DELETE FROM <%= table %><%= where %>';
}

var pks;
if (primaryKeys[tableName] && primaryKeys[tableName].length > 0) {
pks = primaryKeys[tableName].map(function(pk) {
return this.quoteIdentifier(pk);
}.bind(this)).join(',');
} else {
pks = this.quoteIdentifier('id');
}
var pks = _.map(_.values(model.primaryKeys), function (pk) {
return this.quoteIdentifier((pk.field));
}.bind(this)).join(',');

var replacements = {
table: this.quoteIdentifiers(tableName),
table: tableName,
where: this.getWhereConditions(where, null, model, options),
limit: !!options.limit ? ' LIMIT ' + this.escape(options.limit) : '',
primaryKeys: primaryKeys[tableName].length > 1 ? '(' + pks + ')' : pks,
primaryKeys: model.primaryKeyAttributes.length > 1 ? '(' + pks + ')' : pks,
primaryKeysSelection: pks
};

Expand Down Expand Up @@ -818,13 +805,8 @@ var QueryGenerator = {
return (i < 10) ? '0' + i.toString() : i.toString();
},

pgDataTypeMapping: function(tableName, attr, dataType) {
return this.dataTypeMapping(tableName, attr, dataType);
},

dataTypeMapping: function(tableName, attr, dataType) {
if (Utils._.includes(dataType, 'PRIMARY KEY')) {
primaryKeys[tableName].push(attr);
dataType = dataType.replace(/PRIMARY KEY/, '');
}

Expand Down
25 changes: 0 additions & 25 deletions test/unit/dialects/mysql/query-generator.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,31 +509,6 @@ if (Support.dialectIsMySQL()) {
}
],

deleteQuery: [
{
arguments: ['myTable', {name: 'foo'}],
expectation: "DELETE FROM `myTable` WHERE `name` = 'foo' LIMIT 1"
}, {
arguments: ['myTable', 1],
expectation: 'DELETE FROM `myTable` WHERE `id` = 1 LIMIT 1'
},{
arguments: ['myTable', undefined, {truncate: true}],
expectation: 'TRUNCATE `myTable`'
},{
arguments: ['myTable', 1, {limit: 10, truncate: true}],
expectation: 'TRUNCATE `myTable`'
}, {
arguments: ['myTable', 1, {limit: 10}],
expectation: 'DELETE FROM `myTable` WHERE `id` = 1 LIMIT 10'
}, {
arguments: ['myTable', {name: "foo';DROP TABLE myTable;"}, {limit: 10}],
expectation: "DELETE FROM `myTable` WHERE `name` = 'foo\\';DROP TABLE myTable;' LIMIT 10"
}, {
arguments: ['myTable', {name: 'foo'}, {limit: null}],
expectation: "DELETE FROM `myTable` WHERE `name` = 'foo'"
}
],

showIndexesQuery: [
{
arguments: ['User'],
Expand Down
65 changes: 0 additions & 65 deletions test/unit/dialects/postgres/query-generator.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -826,71 +826,6 @@ if (dialect.match(/^postgres/)) {
}
],

deleteQuery: [
{
arguments: ['myTable', {name: 'foo'}],
expectation: "DELETE FROM \"myTable\" WHERE \"id\" IN (SELECT \"id\" FROM \"myTable\" WHERE \"name\" = 'foo' LIMIT 1)"
}, {
arguments: ['myTable', 1],
expectation: 'DELETE FROM \"myTable\" WHERE \"id\" IN (SELECT \"id\" FROM \"myTable\" WHERE \"id\" = 1 LIMIT 1)'
}, {
arguments: ['myTable', undefined, {truncate: true}],
expectation: 'TRUNCATE \"myTable\"'
}, {
arguments: ['myTable', undefined, {truncate: true, cascade: true}],
expectation: 'TRUNCATE \"myTable\" CASCADE'
}, {
arguments: ['myTable', 1, {limit: 10, truncate: true}],
expectation: 'TRUNCATE \"myTable\"'
}, {
arguments: ['myTable', 1, {limit: 10}],
expectation: 'DELETE FROM \"myTable\" WHERE \"id\" IN (SELECT \"id\" FROM \"myTable\" WHERE \"id\" = 1 LIMIT 10)'
}, {
arguments: ['myTable', {name: "foo';DROP TABLE myTable;"}, {limit: 10}],
expectation: "DELETE FROM \"myTable\" WHERE \"id\" IN (SELECT \"id\" FROM \"myTable\" WHERE \"name\" = 'foo'';DROP TABLE myTable;' LIMIT 10)"
}, {
arguments: ['mySchema.myTable', {name: 'foo'}],
expectation: "DELETE FROM \"mySchema\".\"myTable\" WHERE \"id\" IN (SELECT \"id\" FROM \"mySchema\".\"myTable\" WHERE \"name\" = 'foo' LIMIT 1)"
}, {
arguments: ['mySchema.myTable', {name: "foo';DROP TABLE mySchema.myTable;"}, {limit: 10}],
expectation: "DELETE FROM \"mySchema\".\"myTable\" WHERE \"id\" IN (SELECT \"id\" FROM \"mySchema\".\"myTable\" WHERE \"name\" = 'foo'';DROP TABLE mySchema.myTable;' LIMIT 10)"
}, {
arguments: ['myTable', {name: 'foo'}, {limit: null}],
expectation: "DELETE FROM \"myTable\" WHERE \"name\" = 'foo'"
},

// Variants when quoteIdentifiers is false
{
arguments: ['myTable', {name: 'foo'}],
expectation: "DELETE FROM myTable WHERE id IN (SELECT id FROM myTable WHERE name = 'foo' LIMIT 1)",
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['myTable', 1],
expectation: 'DELETE FROM myTable WHERE id IN (SELECT id FROM myTable WHERE id = 1 LIMIT 1)',
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['myTable', 1, {limit: 10}],
expectation: 'DELETE FROM myTable WHERE id IN (SELECT id FROM myTable WHERE id = 1 LIMIT 10)',
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['myTable', {name: "foo';DROP TABLE myTable;"}, {limit: 10}],
expectation: "DELETE FROM myTable WHERE id IN (SELECT id FROM myTable WHERE name = 'foo'';DROP TABLE myTable;' LIMIT 10)",
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['mySchema.myTable', {name: 'foo'}],
expectation: "DELETE FROM mySchema.myTable WHERE id IN (SELECT id FROM mySchema.myTable WHERE name = 'foo' LIMIT 1)",
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['mySchema.myTable', {name: "foo';DROP TABLE mySchema.myTable;"}, {limit: 10}],
expectation: "DELETE FROM mySchema.myTable WHERE id IN (SELECT id FROM mySchema.myTable WHERE name = 'foo'';DROP TABLE mySchema.myTable;' LIMIT 10)",
context: {options: {quoteIdentifiers: false}}
}, {
arguments: ['myTable', {name: 'foo'}, {limit: null}],
expectation: "DELETE FROM myTable WHERE name = 'foo'",
context: {options: {quoteIdentifiers: false}}
}
],

removeIndexQuery: [
{
arguments: ['User', 'user_foo_bar'],
Expand Down
22 changes: 0 additions & 22 deletions test/unit/dialects/sqlite/query-generator.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,28 +497,6 @@ if (dialect === 'sqlite') {
expectation: "UPDATE `myTable` SET `bar`=`foo` WHERE `name` = 'foo'",
needsSequelize: true
}
],

deleteQuery: [
{
arguments: ['myTable', {name: 'foo'}],
expectation: "DELETE FROM `myTable` WHERE `name` = 'foo'"
}, {
arguments: ['myTable', 1],
expectation: 'DELETE FROM `myTable` WHERE `id` = 1'
}, {
arguments: ['myTable', 1, {truncate: true}],
expectation: 'DELETE FROM `myTable` WHERE `id` = 1'
}, {
arguments: ['myTable', 1, {limit: 10}],
expectation: 'DELETE FROM `myTable` WHERE `id` = 1'
}, {
arguments: ['myTable', {name: "foo';DROP TABLE myTable;"}, {limit: 10}],
expectation: "DELETE FROM `myTable` WHERE `name` = 'foo'';DROP TABLE myTable;'"
}, {
arguments: ['myTable', {name: 'foo'}, {limit: null}],
expectation: "DELETE FROM `myTable` WHERE `name` = 'foo'"
}
]
};

Expand Down
86 changes: 82 additions & 4 deletions test/unit/sql/delete.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,95 @@ var Support = require(__dirname + '/../support')

suite(Support.getTestDialectTeaser('SQL'), function() {
suite('delete', function () {
var User = current.define('test_user', {}, {
timestamps:false,
schema: 'public'
});

suite('delete when the primary key has a different field name', function () {
suite('truncate #4306', function () {
var options = {
table: User.getTableName(),
where: {},
truncate: true,
cascade: true,
limit: 10
};

test(util.inspect(options, {depth: 2}), function () {
return expectsql(
sql.deleteQuery(
options.table,
options.where,
options,
User
), {
postgres: 'TRUNCATE "public"."test_users" CASCADE',
mssql: "TRUNCATE TABLE [public].[test_users]",
mysql: 'TRUNCATE `public.test_users`',
sqlite: 'DELETE FROM `public.test_users`'
}
);
});
});

suite('delete without limit', function () {
var options = {
table: User.getTableName(),
where: {name: 'foo' },
limit: null
};

test(util.inspect(options, {depth: 2}), function () {
return expectsql(
sql.deleteQuery(
options.table,
options.where,
options,
User
), {
default: "DELETE FROM [public.test_users] WHERE `name` = 'foo'",
postgres: 'DELETE FROM "public"."test_users" WHERE "name" = \'foo\'',
mssql: "DELETE FROM [public].[test_users] WHERE [name] = N'foo'; SELECT @@ROWCOUNT AS AFFECTEDROWS;"
}
);
});
});

suite('delete with limit', function () {
var options = {
table: User.getTableName(),
where: {name: "foo';DROP TABLE mySchema.myTable;"},
limit: 10
};

test(util.inspect(options, {depth: 2}), function () {
return expectsql(
sql.deleteQuery(
options.table,
options.where,
options,
User
), {
postgres: 'DELETE FROM "public"."test_users" WHERE "id" IN (SELECT "id" FROM "public"."test_users" WHERE "name" = \'foo\'\';DROP TABLE mySchema.myTable;\' LIMIT 10)',
sqlite: "DELETE FROM `public.test_users` WHERE `name` = 'foo'';DROP TABLE mySchema.myTable;'",
mssql: "DELETE TOP(10) FROM [public].[test_users] WHERE [name] = N'foo'';DROP TABLE mySchema.myTable;'; SELECT @@ROWCOUNT AS AFFECTEDROWS;",
default: "DELETE FROM [public.test_users] WHERE `name` = 'foo\\';DROP TABLE mySchema.myTable;' LIMIT 10"
}
);
});
});

suite('delete when the primary key has a different field name', function () {
var User = current.define('test_user', {
id: {
type: Sequelize.INTEGER,
primaryKey: true,
field: "test_user_id",
field: "test_user_id"
}
}, { freezeTableName: true, timestamps:false });
}, {
timestamps:false,
schema: 'public'
});

var options = {
table: 'test_user',
Expand All @@ -43,7 +122,6 @@ suite(Support.getTestDialectTeaser('SQL'), function() {
}
);
});

});
});
});

0 comments on commit 35671ec

Please sign in to comment.