sequelize: Sequelize should combine duplicated properties in the WHERE clauses of multiple scopes by ANDing them instead of replacing them

Issue Description

Is your feature request related to a problem? Please describe.

Sequelize’s “combine where clause when using multiple scopes” feature is severely limited because it replaces duplicated properties instead of ANDing them.

For example combining

where: { id: 1 }

with

where: { company_id: 2 }

results in the expected

where: { id: 1, company_id: 2}

This is the equivalent of combining

WHERE id = 1

with

WHERE company_id = 2

to produce

WHERE id = 1 AND company_id = 2

However, if I wanna combine, e.g.

WHERE id = 1 OR status = 'P'

with

WHERE company_id = 2 OR company_status = 'P'

to produce

WHERE (id = 1 OR status = 'P') AND (company_id = 3 OR company_status = 'P')

I CANNOT do this with combined scopes. Because if I combine

where: { [Op.or]: { id: 1, status: 'P' } }

with

where: { [Op.or]: { id: company_id, company_status: 'P' } }

I get

where: { [Op.or]: { id: company_id, company_status: 'P' } }

Meaning the latter is just overwriting the former. Another example that does NOT work is combining

where: { id: { [Op.gt]: 5 } }

with

where: { id: { [Op.lt]: 50 } }

which would have been super useful.

Describe the solution you’d like

The trouble is that when Sequelize finds a duplicate key in the where clauses of multiple scopes, whether it is a column name, or an Op.* operator, it just replaces them!

This also means that Model.scope([a, b]) is not guaranteed to be Model.scope([a, b]).

My solution is to first realize that

where: { id: 1, company_id: 2}

is really a shorthand for

where: {
  [Op.and]: [ { id: 1 }, { company_id: 2} ]
}

So Sequelize should combine duplicated properties in multiple scopes by ANDing them together.

So combining

where: { id: 1 }

with

where: { company_id: 2 }

makes

where: { 
  [Op.and]: [
    { id: 1 }
    { company_id: 2 }
  ]
}

which is the same as

where: { id: 1, company_id: 2}

And now the other combinations work too. E.g.

where: { [Op.or]: { id: 1, status: 'P' } }

with

where: { [Op.or]: { id: company_id, company_status: 'P' } }

makes

where: {
  [Op.and]: [
    { [Op.or]: { id: 1, status: 'P' } }
    { [Op.or]: { id: company_id, company_status: 'P' } }
  ]
}

And combining

where: { id: { [Op.gt]: 5 } }

with

where: { id: { [Op.lt]: 50 } }

makes

where: {
  [Op.and]: [
    { id: { [Op.gt]: 5 } }
    { id: { [Op.lt]: 50 } }
  ]
}

Why should this be in Sequelize

Scopes is a core part of Sequelize. I am unsure if my proposal is possible in a plugin. In addition, based on numerous other tickets and stack overflow questions, my proposed behavior is what a lot (perhaps even most) people expect, and are surprised when it isn’t.

Describe alternatives/workarounds you’ve considered

= N.A. =

Additional context

= N.A. =

Issue Template Checklist

Is this issue dialect-specific?

  • No. This issue is relevant to Sequelize as a whole.
  • Yes. This issue only applies to the following dialect(s): XXX, YYY, ZZZ

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.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 15 (10 by maintainers)

Most upvoted comments

Okely Dokely. I’ll try to carve out some time to take a look at it this week. Thanks for your prompt responses!

Unfortunately, no - I kinda dropped the ball on this. I was unable to make time. But I still long for this, so I haven’t completely dropped it from my todo list just yet.

The first definitely is trying to solve the same problem I am having. I shall try to take a look at them. Thanks!

Thanks for the report. This would be a breaking change, but since we are in the beta release process of v6 anyway, it could be a good time to implement this.