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

Duplicate column names for associations and misleading documentation #9328

Open
atomanyih opened this issue Apr 19, 2018 · 12 comments
Open

Duplicate column names for associations and misleading documentation #9328

atomanyih opened this issue Apr 19, 2018 · 12 comments
Labels
existing workaround For issues. There is a known workaround for this issue. status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. type: bug type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference.

Comments

@atomanyih
Copy link

Summary

We ran into an issue recently where sequelize generates queries with duplicate column names that cause mysql to errors in subqueries.
A similar issue documented here: #3035 allowed us to fix the issue by specifying the foreignKey.
However this appears to be in conflict with the documentation which suggests that foreignKey is camelCase by default

It seems like sequelize has mismatched casing assumptions: association definitions assume camelCase while the query builder assumes UpperCamelCase

Longer explanation

We have three models, Basket, Apple, Farmer with associations as defined below:

Basket.belongsTo(models.Farmer)
Basket.hasMany(models.Apple)
Farmer.hasMany(models.Basket)

If we findAll

Basket.findAll()

It generates the following query. (note that farmerId is duplicated)

SELECT `id`,
       `farmerId`,
       `FarmerId`
FROM   `Baskets` AS `Basket`;

If we instead want only 10 Baskets, plus any Apples in each Basket:

Basket.findAll({
  limit: 10,
  include: [
    {
      model: Apple
    }
  ]
})

It generates the following query with a subquery

SELECT `Basket`.*,
       `Apples`.`id`    AS `Apples.id`,
FROM   (SELECT `Basket`.`id`,
               `Basket`.`farmerId`,
               `Basket`.`FarmerId`
        FROM   `Baskets` AS `Basket`
        LIMIT  10) AS `Basket`
       LEFT OUTER JOIN `Apples` AS `Apples`
                    ON `Basket`.`id` =
`Apples`.`BasketId`;

Our duplicated columns are now in a subquery. When this query is run in mysql, it throws an error!

SequelizeDatabaseError: Duplicate column name 'FarmerId'

We can fix this by specifying the foreign key on our relationships:

Basket.belongsTo(models.Farmer, { foreignKey: 'farmerId' })
Basket.hasMany(models.Apple)
Farmer.hasMany(models.Basket, { foreignKey: 'farmerId' })

However, the documentation suggests that foreignKey should already be camelCase by default. It's not super clear to me why the query builder would assume UpperCamelCase. This seems like a bug.

Info

Dialect: mysql, (generates the same queries for sqlite, but doesn't cause an error)
Dialect version: default?
Database version: mysqld Ver 5.7.21 for osx10.12 on x86_64 (Homebrew)
Sequelize version: 4.17.0 and 5.0.0-beta.3

@sushantdhiman
Copy link
Contributor

sushantdhiman commented Apr 20, 2018

UpperCamelCase attributes are generated because model name is defined with upper case like

const Basket = sequelize.define('Basket', { .... });

Well its up for discussion, @sequelize/orm what do you suggest here? Enforce proper camel case key even if model name is upper cased?

I think many user prefer PascalCase naming for model, but camel case for attributes name. It make sense to keep attributes camelCased not PascalCased

@sushantdhiman sushantdhiman added the status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. label Apr 20, 2018
@heisian
Copy link

heisian commented Apr 22, 2018

I think the implication is that the foreign key column should assume that it is in camelCase even if the model is in PascalCase, which I think is a logical assumption.

@parkerdan
Copy link

Yeah...not the best bug to try and sort out. I guess I will just be explicit in all model associations

@sangaman
Copy link

sangaman commented Oct 31, 2018

Same issue here, I was getting the Duplicate column name errors for PascalCase properties after adding several associations to existing models where I didn't explicitly specify foreignKey because it seemed like the default value should have been fine based on documentation. The solution for me as well was to go and explicitly specify each foreignKey using camelCase.

@HansHammel
Copy link

still an issue

@papb papb added the type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference. label Sep 18, 2019
@broox
Copy link

broox commented Mar 25, 2020

i just ran into this duplicate column bug after updating from sequelize 4.44.4 to 5.21.5.
the solution to explicitly define the foreignKey works, but it's kinda gross that you have to do that.

@crypto-titan
Copy link

@broox thank you for your help, I just downgraded my Sequelize version to actually fix this issue.
@sequelize you should fix this seriously...

@saeidfiy
Copy link

thank u , i have this issue , it help me

@kingsleyramos
Copy link

I tried to downgrade to 4.44.4, currently on 5.21.3, also tried 6.3.3, I'm still having the same issue for all three versions

@afsaneGhafouri
Copy link

I am using version 5.18.4 and I have the same issue. 😞

@matiasperz
Copy link

matiasperz commented Aug 1, 2020

Every time I ran into this kind of problem, either I use one of these 2 options into the query config:

  • subQuery: false
  • distinct: true

Somehow, most of the time it ends up working.

@ruofanwei
Copy link

Every time I ran into this kind of problem, either I use one of these 2 options into the query config:

  • subQuery: false
  • distinct: true

Somehow, most of the time it ends up working.

thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
existing workaround For issues. There is a known workaround for this issue. status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. type: bug type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference.
Projects
None yet
Development

No branches or pull requests