Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequelize adds id(PK) of main table in findAll with 'group' and currupts a query with it in posgre. #7534

Open
rusekr opened this issue Apr 17, 2017 · 42 comments

Comments

@rusekr
Copy link

rusekr commented Apr 17, 2017

Hello! I have Products model, and Vendors model. With assocation that Vendor has many products and product has one vendorId. I need filter vendors by product properties and/or vendor properties. Using postgre 9.6 and camelcase columns. Sequelize 3.30.2

Therefore object for querying:

 Products.findAll({
      attributes: [],
      where: { type: 'service' },
      include: [{model: Vendors, attributes: ['id', 'name', 'description'], as: 'vendor', required: true, where: { approved: true } }],
      group: ['vendor.id']
    })

Generates incorrect query (with "Products"."id"):

SELECT "Products"."id", "vendor"."id" AS "vendor.id", "vendor"."name" AS "vendor.name", "vendor"."description" AS "vendor.description" FROM "Products" AS "Products" INNER JOIN "Vendors" AS "vendor" ON "Products"."vendorId" = "vendor"."id" AND "vendor"."approved" = true WHERE "Products"."type" = 'service' GROUP BY "vendor"."id";

Query without "Products"."id" works fine and returns vendorId`s filtered by vendor parameters (or/and product parameters):

SELECT "vendor"."id" AS "vendor.id", "vendor"."name" AS "vendor.name", "vendor"."description" AS "vendor.description" FROM "Products" AS "Products" INNER JOIN "Vendors" AS "vendor" ON "Products"."vendorId" = "vendor"."id" AND "vendor"."approved" = true WHERE "Products"."type" = 'service' GROUP BY "vendor"."id";

Is any way to tell Sequelize to not include primary key if I don`t want it in attributes?

Thanks!

upd. workaround #7534 (comment)

@sushantdhiman sushantdhiman added dialect: mssql For issues and PRs. Things that involve MSSQL (and do not involve all dialects). dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). labels Apr 28, 2017
@rusekr
Copy link
Author

rusekr commented Apr 29, 2017

As for Mysql - it ignores primary key id in first query and returns expected result. But that not postgre..

@stale stale bot added the stale label Jun 29, 2017
@stale
Copy link

stale bot commented Jun 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@rusekr
Copy link
Author

rusekr commented Jun 29, 2017

bump

@stale stale bot removed the stale label Jun 29, 2017
@javier-perez
Copy link

javier-perez commented Jul 1, 2017

I have the same issue with mysql (ONLY_FULL_GROUP_BY enabled), the query is showing gratitude.id even I dont have it in the attributes array.

gratitudeModel.findAll({
include: [
{ attributes: [], model: sequelize.model('skill'), as: 'skill' }
],
attributes: [
sequelize.col('skillId'),
[sequelize.fn('sum', { senderId: userId } ), 'given'],
[sequelize.fn('sum', { senderId: userId } ), 'received'],
],
where: {
networkId: networkId,
$or: [
{ 'receiverId': userId },
{ 'senderId': userId }
]
},
group: [ 'skillId']
})

Error

Executing (default): SELECT gratitude.id, skillId, sum(senderId = '1') AS given, sum(senderId = '1') AS received FROM gratitude AS gratitude LEFT OUTER JOIN skill AS skill ON gratitude.skillId = skill.id WHERE gratitude.networkId = '1' AND (gratitude.receiverId = '1' OR gratitude.senderId = '1') GROUP BY skillId;
info: error: stats - Method: find: ER_WRONG_FIELD_WITH_GROUP: Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'feedback.gratitude.id' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by
error: SequelizeDatabaseError: ER_WRONG_FIELD_WITH_GROUP: Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'feedback.gratitude.id' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

Thanks

@stale stale bot added the stale label Aug 30, 2017
@stale
Copy link

stale bot commented Aug 30, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@wdonet
Copy link

wdonet commented Sep 4, 2017

Using: Sequelize 4.5.0 and Mysql 5.7.19

Exactly this happened to me . As @rusekr mentioned, it can be ignored in MySQL through doing something like below (but not a good practice):
SET SESSION sql_mode =''

Wonder why sequelize adds the PK. Following docs , it says about options.attributes

A list of the attributes that you want to select, or an object with include and exclude keys...

Then for options.attributes.include

Select all the attributes of the model, plus some additional ones...

Then for options.attributes.exclude

Select all the attributes of the model, except some few ...

That means that the only options for selecting the right columns on my query is to use options.attributes with an array value instead of object value. But then the PK gets added to the resultant query.

Doesn't make sense to add the PK whenever you join models or doing that without having a way to exclude it.

@stale stale bot removed the stale label Sep 4, 2017
@anilkonsal
Copy link

Adding raw: true to the main object solves this issue.

@rusekr
Copy link
Author

rusekr commented Oct 11, 2017

@anilkonsal Thanks for the tip! But "raw" heavely changes parse result logic though.

@rusekr
Copy link
Author

rusekr commented Oct 11, 2017

update: adding raw :true not solved problem in 3.30.2

@rusekr
Copy link
Author

rusekr commented Oct 29, 2017

update: adding raw :true not solved problem in 4.19.0

@rusekr
Copy link
Author

rusekr commented Dec 8, 2017

bug still present in 4.27.0..

@avrebarra
Copy link

avrebarra commented May 25, 2018

Any updates on this issue? @rusekr


I tried this query:

await Coupon.all({
      subQuery: false,
      attributes: ['tag'],
      include: [
        {
          association: 'coupon_histories',
        },
      ],
      group: 'tag',
    });

I was expecting something like this:

 SELECT 
    `coupon`.`tag`,
    COUNT(`coupon_histories`.`coupon_id`)
FROM
    `coupons` AS `coupon`
    LEFT OUTER JOIN
    `coupon_histories` AS `coupon_histories` ON `coupon`.`id` = `coupon_histories`.`coupon_id`
GROUP BY `coupon`.`tag`

Yet Sequelize is resulting this query:

SELECT 
    `coupon`.`id`,
    `coupon`.`tag`,
    `coupon_histories`.`id` AS `coupon_histories.id`,
    `coupon_histories`.`user_id` AS `coupon_histories.user_id`,
    `coupon_histories`.`coupon_id` AS `coupon_histories.coupon_id`,
    `coupon_histories`.`order_id` AS `coupon_histories.order_id`
FROM
    `coupons` AS `coupon`
    LEFT OUTER JOIN
    `coupon_histories` AS `coupon_histories` ON `coupon`.`id` = `coupon_histories`.`coupon_id`
GROUP BY `tag`;

@rusekr
Copy link
Author

rusekr commented May 29, 2018

It still adding ID. And no response from maintainers.

@NikxDa
Copy link

NikxDa commented May 29, 2018

This should get some attention, it is a persistent and deal-breaking problem.

@rusekr
Copy link
Author

rusekr commented May 29, 2018

It breaks aggregation

@avrebarra
Copy link

for now I resorted to raw queries. made me add another deps like sql-bricks, dotties to make the sql queries bit more customizable and manageable.

it's working quite well, but i'd still rather have this issue fixed though to have uniform and simple data fetching method across my system.

@Alexsey
Copy link
Contributor

Alexsey commented Jul 19, 2018

if you are using group with include then you must set included attrubutes: []. Trik is that you are listing needed attributes in the top attributes and if you are not using MySQL that allow having non-aggregated non-grouped attributes in SELECT then you must wrap them with some dummy function, like MAX

Products.findAll({
  attributes: [
        [sequelize.fn('MAX', sequelize.col('vendor.id'), 'id'],
        [sequelize.fn('MAX', sequelize.col('vendor.name'), 'name'],
        [sequelize.fn('MAX', sequelize.col('vendor.description'), 'description']
  ],
  where: { type: 'service' },
  include: [{
    model: Vendors,
    as: 'vendor', 
    attributes: [],
    required: true,
    where: { approved: true }
  }],
  group: ['vendor.id']
})

I know that it looks horrible, but it works

@joewoodhouse
Copy link

Still stuck on this issue, anyone looking?

@ehsankhfr
Copy link

ehsankhfr commented Sep 15, 2018

Generally, there are two solutions:
1- to revise your queries and make them sorted by the standards this error is expecting
2- to disable the rule:
in sequelize:

sequelize.query(
                    `SET GLOBAL sql_mode=(SELECT REPLACE(@@sql_mode,'ONLY_FULL_GROUP_BY',''))`,
                    { raw: true }
                ).then(() => {
                    console.log('variable is set')
                }).catch((err) => {
                    console.log('variable is not set')
                    console.error(err)
                })

or, editing /etc/mysql/my.cnf:

[mysqld]  
sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION"

I recommend the first solution(revising the queries), but solution 2 may be fine if you feel confident about the queries.

@yasnovo
Copy link

yasnovo commented Mar 15, 2019

Hi, any updates on this issue?
It seems to still present in 4.38.0.

@papb
Copy link
Member

papb commented Jul 29, 2019

If #3142 is fixed, would this issue be fixed too?

@papb papb added status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action and removed dialect: mssql For issues and PRs. Things that involve MSSQL (and do not involve all dialects). dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). investigate labels Jul 29, 2019
@rusekr
Copy link
Author

rusekr commented Aug 14, 2019

If #3142 is fixed, would this issue be fixed too?

Can`t test now but saw #3142 and it not fixes fact when model has PK and PK automatically added to any query with 'group' property which errores in postgres. I will rename issue to reflect this.

This by seeing the code might fix current issue: #11283

@rusekr rusekr changed the title Sequelize adds id of main table and currupts a query with it. Sequelize adds id(PK) of main table in findAll with 'group' and currupts a query with it in posgre. Aug 14, 2019
@rusekr
Copy link
Author

rusekr commented Mar 5, 2020

Waiting for response from #11283

@jsvargas
Copy link

If you add an attribute named 'id' with another value, sequelize returns all the expected values.
For example:

Products.findAll({
  attributes: [
        [sequelize.fn('CONCAT', sequelize.col('Product.id'), ' ', sequelize.col('Vendor.id')), 'id']
        [sequelize.col('vendor.name'), 'name'],
        [sequelize.col('vendor.description'), 'description']
  ],
  include: [{
    model: Vendors,
    attributes: [],
    required: true,
    where: { approved: true }
  }],
  group: ['vendor.id']
})

Here, concat is used to generate an id for each Product/Vendor association.

@cagdas001
Copy link

cagdas001 commented Oct 1, 2020

This is still an issue with MSSQL. raw: true and even sequelize.literaral (for order and group) doesn't work, it still adds the PK.

await Incident.findAll({
    attributes: ['incidentHandler.name', [models.sequelize.fn('count', '*'), 'count']],
    include: [
      {
        model: User,
        attributes: [],
        as: 'incidentHandler',
        required: true
      }
    ],
    limit: 7,
    order: models.sequelize.literal('count desc'),
    group: ['incidentHandler.name'],
    raw: true
  })

generates

SELECT [incidentHandler].[name], count(N'*') AS [count] FROM [dbo].[Incident] AS [Incident] INNER JOIN [dbo].[Users] AS [incidentHandler] ON [Incident].[IncidentHandler] = [incidentHandler].[Id] GROUP BY [incidentHandler].[name] ORDER BY count desc ORDER BY [Incident].[Id] OFFSET 0 ROWS FETCH NEXT 7 ROWS ONLY;

we need to get rid of the automatically added ORDER BY [Incident].[Id]

Unfortunately, I ended up using raw query:

  const topHandlers = await models.sequelize.query(
    'SELECT IncidentHandler.name AS name, count(*) AS count ' +
    'FROM Incident AS Incident ' +
    'INNER JOIN Users AS IncidentHandler ON Incident.IncidentHandler = IncidentHandler.Id ' +
    'GROUP BY name ' +
    'ORDER BY count desc ' +
    'OFFSET 0 ROWS FETCH NEXT 7 ROWS ONLY;',
    { type: Sequelize.QueryTypes.SELECT }
  )

@ashwinkjoseph
Copy link

ashwinkjoseph commented Nov 26, 2020

This issue still exists for me, but for nested includes.
My sequelize version is 6.3.5.
This is my query:

Model1.findAll({
        attributes: [
          [Sequelize.fn('date_trunc', groupBy, Sequelize.literal(`"model1"."createdAt" AT TIME ZONE '${timeZone}'`)), 'aggregateattribute1'],
          [Sequelize.fn('sum', Sequelize.col('Model1attribue4')), 'aggregateattribute2'],
          [Sequelize.fn('count', Sequelize.col('"model1".id')), 'aggregateattribute3'],
        ],
        where: {
          model1attribute3: Model1Status.paid,
          model1attribute1: false,
          model1attribute2: Model1Type.package,
          createdAt: {
            [Sequelize.Op.gte]: startTime,
            [Sequelize.Op.lte]: endTime ? endTime : startTime,
          },
        },
        group: [Sequelize.fn('date_trunc', 'day', Sequelize.literal(`"Model1"."createdAt" AT TIME ZONE '${timeZone}'`))],
        include: [
          {
            association: Model1.associations.Model2,
            where: {
              driverId: ctx.user.id,
              status: Model2Status.completed,
            },
            attributes: [],
            include: [
              {
                association: Model2.associations.Model3,
                attributes: [
                  [
                    Sequelize.fn('sum', Sequelize.literal('COALESCE("model2->model3"."attribute1", NOW()) - "model2->model3"."attribute2"')),
                    'attribute3',
                  ]
                ],
                required: true,
              },
            ],
            required: true,
          },
        ],
      })

This works perfectly fine until the first include but breaks at the second nested include. It includes the ID of Model3 in the select query

Here's the generated query -

SELECT date_trunc('day', "model1"."createdAt" AT TIME ZONE 'US/Central') AS "aggregateattribute1", sum("Model1attribue4") AS "aggregateattribute2", count("model1"."id") AS "aggregateattribute3", "model2->model3"."id" AS "model2.model3.id", sum(COALESCE("model2->model3"."attribute1", NOW()) - "model2->model3"."attribute2") AS "model2.model3.attribute3" FROM "Model1" AS "model1" INNER JOIN "Model2" AS "model2" ON "model1"."foreignkeytomodel2" = "model2"."model2primarykey" AND "model2"."model2foreignkeytouser" = 2 AND "model2"."status" = 'completed' INNER JOIN "Model2" AS "model2" ON "model2"."primarykeyofmodel2" = "model3"."foreignKeytomodel2" WHERE "model1"."model1attribute3" = 'paid' AND "model1"."model1attribute1" = false AND "model1"."model1attribute2" = 'package' AND ("model1"."createdAt" >= '2020-11-25 09:01:25.288 +00:00' AND "model1"."createdAt" <= '2020-11-25 09:01:25.288 +00:00') GROUP BY date_trunc('day', "model1"."createdAt" AT TIME ZONE 'US/Central');

As you can see, the model3's id is also being added into the select query even though I haven't specified it in attributes list.

I will be resorting to raw queries for now.

I am willing to help you resolve this issue by making a PR if any of you can guide me on where to get started.

@fanker
Copy link

fanker commented Jan 11, 2021

I have the same issue in mysql, with sequelize@5.22.3

@jamt0
Copy link

jamt0 commented Apr 27, 2021

I have the same issue in mysql

@github-actions github-actions bot removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Apr 27, 2021
@GhettoBurger996
Copy link

Got the same issue in PostgreSQL using sequelize version 6.6.2

@kippllo
Copy link

kippllo commented Jul 14, 2021

This is still an issue for me in Sequelize v6.3.5 with MSSQL.
I guess the devs hate us... 🤣

I wonder if this will ever get fixed...

@rusekr
Copy link
Author

rusekr commented Jul 16, 2021

Workaround is to declare reverse relation from Vendors to Products of type HasMany (it will be using existing foreign key from direct relation) and write something like (vendors centric):

 Vendors.findAll({
    where: { approved: true },
    include: [{model: Products, attributes: ['type'], as: 'products', required: true, where: { type: 'service' } }],
    distinct: ['id']
  })

Thinking now it's best way to group by/distinct entity by entity-centric query. Just don't hesitate to declare reverse relation.

@Angrigo
Copy link

Angrigo commented Feb 21, 2022

still an issue sequelize 6.12.5

@rusekr
Copy link
Author

rusekr commented Feb 22, 2022

still an issue sequelize 6.12.5

It seems like pk is used internally for critical parts of making objects from tables data after select. Workaround: #7534 (comment)

@sastro381
Copy link

still an issue on sequelize 6.21.4 please some one help us

@geleto
Copy link

geleto commented Mar 17, 2023

Amazing that this issue has not been fixed for 6 years, doesn't it affect anyone who wants to use group and include in findAll?
Here's my hideous workaround:

const { QueryInterface } = require('sequelize/lib/dialects/abstract/query-interface.js');
const QueryTypes = require('sequelize/lib//query-types');

QueryInterface.prototype.select = async function(model, tableName, optionsArg) {
  const options = { ...optionsArg, type: QueryTypes.SELECT, model };
  let query = this.queryGenerator.selectQuery(tableName, options, model);
  if(tableName=='Products' && options.group?.length && options.include?.length)
    query = query.replace(', "Products"."id"','');
  return await this.sequelize.query( query, options );
}

@callmeteus
Copy link

callmeteus commented Apr 8, 2023

It's still an issue for today and affects literally any query that uses include, group and attributes together.
Why this hasn't been fixed yet? Hasn't even got enough attention for such an annoying bug :/

@mmkhmk
Copy link

mmkhmk commented May 9, 2023

I'm facing the same issue. Any updates?

@tybro0103
Copy link

tybro0103 commented Aug 14, 2023

Here's another example of this issue, for which the workaround mentioned in this thread does not help:

Post.findAll({
  attributes: [
    [sequelize.literal(`COALESCE(SUM("Post"."characterCount"), 0)`), 'totalCharacters'],
    [sequelize.literal(`COUNT(DISTINCT "Post"."userId")`), 'userCount'],
  ],
  include: [
    {association: 'user', attributes: []},
  ],
  where: {
    '$user.role$': {[Op.or]: [{[Op.not]: 'admin'}, null]},
  },
});

Generated SQL:

SELECT 
  "Post"."id",
  COALESCE(SUM("Post"."characterCount"), 0) AS "totalCharacters", 
  COUNT(DISTINCT "Post"."userId") AS "userCount"
FROM 
  "Posts" AS "Post" 
  LEFT OUTER JOIN "Users" AS "user" ON "Post"."userId" = "user"."id" 
WHERE 
  "user"."role" != 'admin' OR "user"."role" IS NULL;

As you can see, that "Post"."id" breaks the whole thing :(

Previously, I simply needed:

Post.findAll({
  attributes: [
    [sequelize.literal(`COALESCE(SUM("Post"."characterCount"), 0)`), 'totalCharacters'],
    [sequelize.literal(`COUNT(DISTINCT "Post"."userId")`), 'userCount'],
  ],
});

which worked fine...but then of course additional conditions were eventually needed.

I've posted this to StackOverflow as well.

@tybro0103
Copy link

Looks like these issues may be related:

Perhaps there are more... I only scanned through the issues a little.

@danielshoo
Copy link

bump. Still an issue. Sequelize v6.32.1 w/ MySQL 8.0.33.

@siniradam
Copy link

Any news on this? (6.37.3) PostgreSQL still have the same problem.

@drylith
Copy link

drylith commented Jun 15, 2024

Same issue on: "sequelize": "^6.37.3"

In fact, if you try the same aggregation functions on the including model itself, it is no problem.

It all comes down the fact, that sequelize adds the primaryKey to the selected cols when include another model. I'm not sure why it does it this way.

The only way around i found was to create your models such that you can group them over your primaryKey after including the other model.

The real solution to this problem would be to figure out, why sequelize adds the pk col to the selected columns before the group by operation is done AND deleting it after before returning it. Since it only appear when you are including another model, this should have something to do with sequelize needing the pk to "join" the models (ofcourse it needs it) but not removing it after...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.