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 efab1ea.

* 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

@@ -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]);
           }
         }
 

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 ';

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']]
+      });
+    });
   });
 }

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];`
       });