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

through: {attributes: []} does not remove attributes #5590

Closed
jmwilkinson opened this issue Mar 15, 2016 · 19 comments
Closed

through: {attributes: []} does not remove attributes #5590

jmwilkinson opened this issue Mar 15, 2016 · 19 comments

Comments

@jmwilkinson
Copy link

Setting through attributes to [] doesn't exclude the id columns from being selected.

The model query:

componentsPromise = models.Component.findAll({
    include: [{
        attributes: [],
        model: models.Platform,
        through: {
            attributes: []
        },
        as: 'platforms',
        where: {
            id: {
                in: ids
            }
        }
    }],
    group: ['component.id'],
    having: ['COUNT(?) = ?', 'component.id', ids.length]
})

Defining the association:

Component.belongsToMany(Platform, {
    through: 'platform_components',
    as: 'platforms',
    foreignKey: 'component_id',
    otherKey: 'platform_id'
});

Platform.belongsToMany(Component, {
    through: 'platform_components',
    as: 'components',
    foreignKey: 'platform_id',
    otherKey: 'component_id'
});

The result query:

SELECT
  "component"."id",
  "component"."component_name",
  "component"."component_type",
  "component"."created_at",
  "component"."updated_at",
  "platforms.platform_components"."platform_id"  AS "platforms.platform_components.platform_id",
  "platforms.platform_components"."component_id" AS "platforms.platform_components.component_id"
FROM "components" AS "component" INNER JOIN
  ("platform_components" AS "platforms.platform_components" INNER JOIN "platforms" AS "platforms"
      ON "platforms"."id" = "platforms.platform_components"."platform_id")
    ON "component"."id" = "platforms.platform_components"."component_id" AND "platforms"."id" IN ('1', '2', '3')
GROUP BY "component"."id", "platforms.platform_components.platform_id", "platforms.platform_components.component_id"
HAVING COUNT('component.id') = 3;

This is a major problem in postgres, as this query will fail because the group by must include the through table primary keys, which subsequently makes the query not function.

@mickhansen
Copy link
Contributor

We need the id for deduplication.
Unfortunetaly we don't currently have a way of detecting that a thing is only used for aggregation.

@jmwilkinson
Copy link
Author

This may be a reason to have a "join" parameter. It would run a join against a table without needing the data in those tables.

Something like:

models.User.findAll({
  join: [{
    model: models.Role
    where: {
      created_at: TODAY
    }
  }]
})

@mickhansen
Copy link
Contributor

Wait, i just looked at your model query again, i don't see any include statements? And you have the through stament as a top level option.

Please include your actual code and i can see if there's any issues.

@jmwilkinson
Copy link
Author

I just edited the formatting. My include was spaced in too far, making it look like it was on the same level as the through. What I have there is (was) the actual code.

@mickhansen
Copy link
Contributor

Hmm with that code it shouldn't select anything, that's odd, should just work.

@jmwilkinson
Copy link
Author

If you have a chance to verify this behaviour, perhaps we can mark this a bug and work on a fix.

@xizhao
Copy link

xizhao commented Mar 16, 2016

#5602 may be related; I can't seem to specify which attributes are selected on the through table. Instead it looks like an all-or-nothing op

@mickhansen
Copy link
Contributor

I'll mark as a bug untill i can verify either way.

@ningshen
Copy link

ningshen commented Apr 23, 2016

+1.
I found If I comment if statement at:
https://github.com/sequelize/sequelize/blob/master/lib/model.js#L1390

It will work.

@mickhansen
Copy link
Contributor

@ningshen That will great reduce deduplication performance since we can no longer match primary keys though.

@jeff-kilbride
Copy link

This was a relatively simple fix, so I backported it to my installed sequelize (v3.24.0). The change works as expected with normal output. However, the issue still exists if you set raw: true in your query options.

Took me a while to figure out, because I was still getting the through table keys in my output. When I removed raw: true, they finally went away. So, just a heads up! I put a question on StackOverflow the other day, before I found this thread, if you want to see my setup:

http://stackoverflow.com/questions/38930799/how-to-remove-through-table-attributes

If it's another simple fix to correct this, I'd be more than happy to test it on my system.

@felixfbecker
Copy link
Contributor

@jeff-kilbride feel free to open a PR to back-port to v3

@rodrigoclp
Copy link

includeIgnoreAttributes: false, solves the problem in sequelize 4.

@RodrigoCS
Copy link

I need to use includeIgnoreAttributes in order to exclude with through: { attributes: [] } but that also excluded my set attributes on the include, you can solve that setting the included attributes on the main attributes, like so:

models.User.findAll({
  attributes: [Sequelize.literal(`${joinModelName}.id AS id`)]
})

@wilmerjpg
Copy link

includeIgnoreAttributes: false, solves the problem in sequelize 4.

Thanks, works for me!

@internationalbridgeinc
Copy link

includeIgnoreAttributes: false makes the through: { attributes: [] } superfluous. But sometimes you need to join model attributes. I hope this can be fixed.

@papb
Copy link
Member

papb commented Oct 8, 2019

Hello, this issue is old and our issue templates have changed since then; investigating this issue as-is would need extra effort from maintainers (and our time is really short). Can someone please open a new issue about this, filling our modern templates, to save time from the maintainers? Especially including a nice SSCCE, preferably from our sequelize-sscce repository. For those hoping for a fix, doing this would greatly help and in theory is something anyone encountering this issue should be able to do. Thanks for understanding! Don't forget to link to this issue in the new issue.

@gvsakhil
Copy link

gvsakhil commented Nov 11, 2020

We have this issue still in Sequilize 6.3.5

@abdullahEmumba
Copy link

includeIgnoreAttributes: false, solves the problem in sequelize 4.

2 days I spent looking for the right answer and here it was. You don't know how much I am relived right now!!!!!!!

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

No branches or pull requests