Regression (5.17.2 -> 5.18.0): bug in aggregation with alias

[madmoizo]

Issue Description

Before v5.18.0, it was possible to do some aggregation using alias name directly.
(Probably related to 3fcb2d2)

Question: is there an alternative official way to do the following request without using alias explicitly?

What are you doing?

await User.findAll({
    attributes: [
      'id',
      'firstName',
      'lastName',
      'color',
      [
        Sequelize.fn('count', Sequelize.col('missions.clientfileId')),
        'totalMissions'
      ],
      [
        Sequelize.fn('sum', Sequelize.col('missions->clientfile->invoice.totalTaxed')),
        'totalRevenue'
      ]
    ],
    where: {
      officeId: state.decodedToken.officeId
    },
    include: [{
      association: 'missions',
      attributes: [],
      where: {
        startDate: {
          [Op.gte]: start,
          [Op.lte]: end
        }
      },
      include: [{
        association: 'clientfile',
        required: true,
        attributes: [],
        include: [{
          association: 'invoice',
          required: true,
          attributes: []
        }]
      }]
    }],
    order: [
      ['lastName', 'ASC'],
      ['firstName', 'ASC']
    ],
    group: [
      ['id']
    ]
  })

What do you expect to happen?

It works until 5.18.0

What is actually happening?

the request respond with column reference "id" is ambiguous but I suppose it's related to alias explicitly used in the request (which are no longer valid).

Environment

• Sequelize version: 5.18.1
• Node.js version: 12.x
• Operating System: Windows

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 PUT-YOUR-DIALECT-HERE, with connector library version XXX and database version XXX

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.

[papb]

Did you enable the new { minifyAliases: true } option?

[madmoizo]

@papb no, I didn't change anything. If aliases are not minified by default, the issue could be a side effect of this PR. I can confirm at least it works in 5.17.2 but not in 5.18.0

[papb]

@frlinw Yeah I thought so but just wanted to make sure. The thing is, you put Minified alias issue in the title but the issue itself is only related to minification in the sense that it was the feature which introduced the bug. I will rename it to reduce confusion.

[papb]

One more thing, you posted a lot of code, is all of that really needed to reproduce the issue? Can you shorten it? Also, you have posted a code snippet that shows the problem but is not entirely self-contained (i.e. I can't just copy-paste it and run it), please provide a SSCCE (also known as MCVE/reprex) for it, thanks!

[madmoizo]

Initially it was a question more than an issue, because I'm aware the direct use of aliases is not really a public thing so I wanted a supported way to access aliases (as I thought it was an alias issue).
I guess the issue is because of the group property, probably wrongly scoped.
I'll try to post a simple repro

[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. 🙂

[github-actions]

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

[madmoizo]

This issue is still a thing

await User.findAll({
    attributes: [
      'id',
      [
        Sequelize.fn('sum', Sequelize.col('missions->clientfile->invoice.totalTaxed')),
        'totalRevenue'
      ]
    ],
    include: [{
      association: 'missions',
      attributes: [],
      include: [{
        association: 'clientfile',
        required: true,
        attributes: [],
        include: [{
          association: 'invoice',
          required: true,
          attributes: []
        }]
      }]
    }],
    group: [
      ['id']
    ]
  })

5.17.2

SELECT "User"."id", sum("missions->clientfile->invoice"."totalTaxed") AS "totalRevenue"
FROM "User" AS "User"
LEFT OUTER JOIN (
    "Mission" AS "missions"
    INNER JOIN "Clientfile" AS "missions->clientfile" ON "missions"."clientfileId" = "missions->clientfile"."id"
    INNER JOIN "Invoice" AS "missions->clientfile->invoice" ON "missions->clientfile"."id" = "missions->clientfile->invoice"."clientfileId" )
ON "User"."id" = "missions"."technicianId" GROUP BY "User"."id";

5.18.0 to 7.0.0-alpha.10

SELECT "User"."id", sum("missions->clientfile->invoice"."totalTaxed") AS "totalRevenue" 
FROM "User" AS "User" 
LEFT OUTER JOIN (
   "Mission" AS "missions"
   INNER JOIN "Clientfile" AS "missions->clientfile" ON "missions"."clientfileId" = "missions->clientfile"."id"
   INNER JOIN "Invoice" AS "missions->clientfile->invoice" ON "missions->clientfile"."id" = "missions->clientfile->invoice"."clientfileId"
) ON "User"."id" = "missions"."technicianId" GROUP BY "id"

error: column reference "id" is ambiguous

Dirty workaround

group: [
   ['"User"."id"']
]