From 5a257bc93c7e760f6b0158f55b3cb48878698450 Mon Sep 17 00:00:00 2001 From: Sascha Depold Date: Thu, 18 Aug 2022 13:00:03 +0200 Subject: [PATCH] fix: minified aliases are now properly referenced in subqueries (v6) (#14852) * fix: minified aliases are now properly referenced in subqueries Backport of #14804 * fix: linting mistakes * change syntax * Revert "fix: linting mistakes" This reverts commit efab1ea8ebdeb29ca2bc2d46992f24337dbeb9c4. * fix: respect aliases without subQuery enabled * fix: unit tests * fix: tmp * fix: should we disable this for mssql hm * fix: adjust conditions for manual mssql ordering * fix: don't add manual pk order if already exist * fix: field name extraction * remove unused file --- src/dialects/abstract/query-generator.js | 22 ++++-- src/dialects/mssql/query-generator.js | 33 ++++++++- .../dialects/postgres/query.test.js | 73 +++++++++++++++++++ test/unit/sql/select.test.js | 8 +- 4 files changed, 122 insertions(+), 14 deletions(-) diff --git a/src/dialects/abstract/query-generator.js b/src/dialects/abstract/query-generator.js index afff3dd68ed5..c6d63ba314c7 100644 --- a/src/dialects/abstract/query-generator.js +++ b/src/dialects/abstract/query-generator.js @@ -2083,17 +2083,25 @@ class QueryGenerator { && !(typeof order[0].model === 'function' && order[0].model.prototype instanceof Model) && !(typeof order[0] === 'string' && model && model.associations !== undefined && model.associations[order[0]]) ) { - subQueryOrder.push(this.quote(order, model, '->')); + const field = model.rawAttributes[order[0]] ? model.rawAttributes[order[0]].field : order[0]; + const subQueryAlias = this._getAliasForField(this.quoteIdentifier(model.name), field, options); + + subQueryOrder.push(this.quote(subQueryAlias === null ? order : subQueryAlias, model, '->')); } - if (subQuery) { - // Handle case where sub-query renames attribute we want to order by, - // see https://github.com/sequelize/sequelize/issues/8739 - const subQueryAttribute = options.attributes.find(a => Array.isArray(a) && a[0] === order[0] && a[1]); - if (subQueryAttribute) { + // Handle case where renamed attributes are used to order by, + // see https://github.com/sequelize/sequelize/issues/8739 + // need to check if either of the attribute options match the order + if (options.attributes && model) { + const aliasedAttribute = options.attributes.find(attr => Array.isArray(attr) + && attr[1] + && (attr[0] === order[0] || attr[1] === order[0])); + + if (aliasedAttribute) { const modelName = this.quoteIdentifier(model.name); + const alias = this._getAliasForField(modelName, aliasedAttribute[1], options); - order[0] = new Utils.Col(this._getAliasForField(modelName, subQueryAttribute[1], options) || subQueryAttribute[1]); + order[0] = new Utils.Col(alias || aliasedAttribute[1]); } } diff --git a/src/dialects/mssql/query-generator.js b/src/dialects/mssql/query-generator.js index 6fce1947c9d1..ddf8ec45b7eb 100644 --- a/src/dialects/mssql/query-generator.js +++ b/src/dialects/mssql/query-generator.js @@ -976,12 +976,39 @@ class MSSQLQueryGenerator extends AbstractQueryGenerator { // TODO: document why this is adding the primary key of the model in ORDER BY // if options.include is set if (!options.order || options.order.length === 0 || options.include && orders.subQueryOrder.length === 0) { - const tablePkFragment = `${this.quoteTable(options.tableAs || model.name)}.${this.quoteIdentifier(model.primaryKeyField)}`; + let primaryKey = model.primaryKeyField; + + const tablePkFragment = `${this.quoteTable(options.tableAs || model.name)}.${this.quoteIdentifier(primaryKey)}`; + const aliasedAttribute = (options.attributes || []).find(attr => Array.isArray(attr) + && attr[1] + && (attr[0] === primaryKey || attr[1] === primaryKey)); + + if (aliasedAttribute) { + const modelName = this.quoteIdentifier(options.tableAs || model.name); + const alias = this._getAliasForField(modelName, aliasedAttribute[1], options); + + primaryKey = new Utils.Col(alias || aliasedAttribute[1]); + } + if (!options.order || !options.order.length) { fragment += ` ORDER BY ${tablePkFragment}`; } else { - const orderFieldNames = _.map(options.order, order => order[0]); - const primaryKeyFieldAlreadyPresent = _.includes(orderFieldNames, model.primaryKeyField); + const orderFieldNames = (options.order || []).map(order => { + const value = Array.isArray(order) ? order[0] : order; + + if (value instanceof Utils.Col) { + return value.col; + } + + if (value instanceof Utils.Literal) { + return value.val; + } + + return value; + }); + const primaryKeyFieldAlreadyPresent = orderFieldNames.some( + fieldName => fieldName === (primaryKey.col || primaryKey) + ); if (!primaryKeyFieldAlreadyPresent) { fragment += options.order && !isSubQuery ? ', ' : ' ORDER BY '; diff --git a/test/integration/dialects/postgres/query.test.js b/test/integration/dialects/postgres/query.test.js index 55085a6ccf6a..f4888d20fecf 100644 --- a/test/integration/dialects/postgres/query.test.js +++ b/test/integration/dialects/postgres/query.test.js @@ -102,5 +102,78 @@ if (dialect.match(/^postgres/)) { } }); }); + + it('orders by a literal when subquery and minifyAliases are enabled', async () => { + const sequelizeMinifyAliases = Support.createSequelizeInstance({ + logQueryParameters: true, + benchmark: true, + minifyAliases: true, + define: { + timestamps: false + } + }); + + const Foo = sequelizeMinifyAliases.define('Foo', { + name: { + field: 'my_name', + type: DataTypes.TEXT + } + }, { timestamps: false }); + + await sequelizeMinifyAliases.sync({ force: true }); + await Foo.create({ name: 'record1' }); + await Foo.create({ name: 'record2' }); + + const thisWorks = (await Foo.findAll({ + subQuery: false, + order: sequelizeMinifyAliases.literal('"Foo".my_name') + })).map(f => f.name); + expect(thisWorks[0]).to.equal('record1'); + + const thisShouldAlsoWork = (await Foo.findAll({ + attributes: { + include: [ + [sequelizeMinifyAliases.literal('"Foo".my_name'), 'customAttribute'] + ] + }, + subQuery: true, + order: ['customAttribute'] + })).map(f => f.name); + expect(thisShouldAlsoWork[0]).to.equal('record1'); + }); + + it('returns the minified aliased attributes', async () => { + const sequelizeMinifyAliases = Support.createSequelizeInstance({ + logQueryParameters: true, + benchmark: true, + minifyAliases: true, + define: { + timestamps: false + } + }); + + const Foo = sequelizeMinifyAliases.define( + 'Foo', + { + name: { + field: 'my_name', + type: DataTypes.TEXT + } + }, + { timestamps: false } + ); + + await sequelizeMinifyAliases.sync({ force: true }); + + await Foo.findAll({ + subQuery: false, + attributes: { + include: [ + [sequelizeMinifyAliases.literal('"Foo".my_name'), 'order_0'] + ] + }, + order: [['order_0', 'DESC']] + }); + }); }); } diff --git a/test/unit/sql/select.test.js b/test/unit/sql/select.test.js index 544d8d5cf6be..5fd61069a7a2 100644 --- a/test/unit/sql/select.test.js +++ b/test/unit/sql/select.test.js @@ -273,8 +273,8 @@ describe(Support.getTestDialectTeaser('SQL'), () => { }, { default: `SELECT [user].*, [POSTS].[id] AS [POSTS.id], [POSTS].[title] AS [POSTS.title] FROM (${ [ - `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub`, - `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub` + `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub`, + `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub` ].join(current.dialect.supports['UNION ALL'] ? ' UNION ALL ' : ' UNION ') }) AS [user] LEFT OUTER JOIN [post] AS [POSTS] ON [user].[id] = [POSTS].[user_id];` }); @@ -363,8 +363,8 @@ describe(Support.getTestDialectTeaser('SQL'), () => { }, { default: `SELECT [user].*, [POSTS].[id] AS [POSTS.id], [POSTS].[title] AS [POSTS.title], [POSTS->COMMENTS].[id] AS [POSTS.COMMENTS.id], [POSTS->COMMENTS].[title] AS [POSTS.COMMENTS.title] FROM (${ [ - `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub`, - `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub` + `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub`, + `SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub` ].join(current.dialect.supports['UNION ALL'] ? ' UNION ALL ' : ' UNION ') }) AS [user] LEFT OUTER JOIN [post] AS [POSTS] ON [user].[id] = [POSTS].[user_id] LEFT OUTER JOIN [comment] AS [POSTS->COMMENTS] ON [POSTS].[id] = [POSTS->COMMENTS].[post_id];` });