Regression in 5.18.x - "Unknown structure passed to order / group" when grouping by include (MySQL)

[geoffreak]

Issue Description

Something broke with the 5.18.x releases that caused a regression in the group by behavior. I am now getting Unknown structure passed to order / group errors while trying to get some aggregates while grouping by an included model.

What are you doing?

I have boiled down a working code example that demonstrates this issue.

CREATE TABLE `first` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `pid` int(10) unsigned NOT NULL DEFAULT '0',
  `created_at` datetime NOT NULL,
  `updated_at` datetime NOT NULL,
  `deleted_at` datetime NOT NULL,
  PRIMARY KEY (`id`)
);

CREATE TABLE `second` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `a` int(10) unsigned NOT NULL DEFAULT '0',
  `b` int(10) unsigned NOT NULL DEFAULT '0',
  `c` int(10) unsigned NOT NULL DEFAULT '0',
  `d` int(10) unsigned NOT NULL DEFAULT '0',
  `created_at` datetime NOT NULL,
  `updated_at` datetime NOT NULL,
  `deleted_at` datetime NOT NULL,
  `first_id` int(10) unsigned DEFAULT NULL,
  PRIMARY KEY (`id`),
  CONSTRAINT `second_ibfk_1` FOREIGN KEY (`first_id`) REFERENCES `first` (`id`) ON DELETE CASCADE ON UPDATE CASCADE
);

/* eslint-disable no-console, no-process-exit */
const Sequelize = require('sequelize');

const sequelize = new Sequelize('test', 'root', null, {
  host: '127.0.0.1',
  dialect: 'mysql',
  logging: console.log,
});


const FirstModel = sequelize.define('first', {
  id: {
    primaryKey: true,
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
    autoIncrement: true,
  },

  pid: {
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
  },
}, {
  underscored: true,
  paranoid: true,
  tableName: 'first',
});

// Create the model
const SecondModel = sequelize.define('second', {
  id: {
    primaryKey: true,
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
    autoIncrement: true,
  },

  a: {
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
    allowNull: false,
    defaultValue: 0,
  },

  b: {
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
    allowNull: false,
    defaultValue: 0,
  },

  c: {
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
    allowNull: false,
    defaultValue: 0,
  },

  d: {
    type: Sequelize.DataTypes.INTEGER.UNSIGNED,
    allowNull: false,
    defaultValue: 0,
  },
}, {
  underscored: true,
  paranoid: false,
  tableName: 'second',
});

// Setup associations
SecondModel.belongsTo(FirstModel);
FirstModel.hasMany(SecondModel);


/**
 * Run the test logic.
 * @returns {void}
 */
const run = async function() {
  // Get the sums of all activity for all time
  const aggregatedSums = await SecondModel.findOne({
    attributes: [
      [sequelize.fn('sum', sequelize.col('a')), 'a'],
      [sequelize.fn('sum', sequelize.col('b')), 'b'],
      [sequelize.fn('sum', sequelize.col('c')), 'c'],
      [sequelize.fn('sum', sequelize.col('d')), 'd'],
    ],
    include: [{
      attributes: [],
      model: FirstModel,
      where: {
        pid: 1,
      },
    }],
    group: [[FirstModel, 'pid']],
  });
  console.log(aggregatedSums);
};

run().then(() => {
  console.log('Done');
  process.exit(0);
}).catch((err) => {
  console.error(err);
  process.exit(1);
});

What do you expect to happen?

In 5.17.2 and below, the following is output:

Executing (default): SELECT sum(`a`) AS `a`, sum(`b`) AS `b`, sum(`c`) AS `c`, sum(`d`) AS `d` FROM `second` AS `second` INNER JOIN `first` AS `first` ON `second`.`first_id` = `first`.`id` AND (`first`.`deleted_at` IS NULL AND `first`.`pid` = 1) GROUP BY `first`.`pid` LIMIT 1;
null
Done

What is actually happening?

In 5.18.0 through 5.18.4 the following is output instead:

Error: Unknown structure passed to order / group: first
    at MySQLQueryGenerator.quote (/path/node_modules/sequelize/lib/dialects/abstract/query-generator.js:879:11)
    at MySQLQueryGenerator.aliasGrouping (/path/node_modules/sequelize/lib/dialects/abstract/query-generator.js:1407:17)
    at options.group.Array.isArray.options.group.map.t (/path/node_modules/sequelize/lib/dialects/abstract/query-generator.js:1333:82)
    at Array.map (<anonymous>)
    at MySQLQueryGenerator.selectQuery (/path/node_modules/sequelize/lib/dialects/abstract/query-generator.js:1333:68)
    at QueryInterface.select (/path/node_modules/sequelize/lib/query-interface.js:1119:27)
    at Promise.try.then.then.then (/path/node_modules/sequelize/lib/model.js:1756:34)
    at tryCatcher (/path/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/path/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/path/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/path/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/path/node_modules/bluebird/js/release/promise.js:693:18)
    at Async._drainQueue (/path/node_modules/bluebird/js/release/async.js:133:16)
    at Async._drainQueues (/path/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues (/path/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)

Additional context

N/A

Environment

• Sequelize version: 5.18.0 through 5.18.4
• Node.js version: v8.16.0
• Operating System: macOS 10.14.6
• If TypeScript related: TypeScript version: n/a

Issue Template Checklist

How does this problem relate to dialects?

• I think this problem happens regardless of the dialect.
• I think this problem happens only for the following dialect(s):
• I don't know, I was using MySQL, with connector library mysql2 version 1.5.2 and database version 5.6

Would you be willing to resolve this issue by submitting a Pull Request?

• Yes, I have the time and I know how to start.
• Yes, I have the time but I don't know how to start, I would need guidance.
• No, I don't have the time, although I believe I could do it if I had the time...
• No, I don't have the time and I wouldn't even know how to start.

[createthis]

I'm also seeing this when upgrading from Sequelize 5.12.2 to 5.19.1. I can confirm it does seem to appear in 5.18.0.

[createthis]

@geoffreak It seems nested array syntax in group by has been broken/removed. I think one workaround is to change your group from:

group: [[FirstModel, 'pid']],

to:

group: ['first.pid'],

[geoffreak]

I'll just hold off on the upgrade for now until this gets resolved. Good to know there's a workaround in case of a security patch.

[validkeys]

I believe I found the issue https://github.com/sequelize/sequelize/blob/master/lib/dialects/abstract/query-generator.js#L1367

The call to getAliasForGrouping is returning a null, so it's passing the "src" var to quote. If you are grouping by an array of arrays, the "src" var will only have the first item (model) in the array, not the full grouping array item.

Replacing:

return this.quote(this._getAliasForField(tableName, src, options) || src, model);

with

return this.quote(this._getAliasForField(tableName, src, options) || field, model);

resolves the issue in a quick local test. Will try a PR and run tests to see if that causes any further issues

[papb]

@validkeys Nice, thanks, please try the PR :)

[pallatee]

Any news on this? Documentation states that this should work.

[mooniker]

Just encountered this in 5.22.3 with Postgres. Is this still conformed to be not fixed? Removing the nested array syntax addressed the issue.

[stp-engineering-goodgames]

we updated yesterday to 5.22.4 and we are getting.
Unknown structure passed to order / group: Literal { val: ' ASC' }
any info in this regards? updates?

[stp-engineering-goodgames]

after some investigation we figured out the error has nothing to do with Sequelize, rather with Serverless Offline ( which was recently updated and enabled hotmodulereloading. That caching functionality affects somehow Singletons and GlobalVariables and for some reason Sequelize initialization is affected, thus this error ( which was happening ONLY when invoking our endpoints --> sequelize via our react application from the browser on localhost - no issues in production, no errors when testing the endpoint individually via CURL)

[github-actions]

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂