Double include when reloading an instance with scopes

[Dam-Buty]

What are you doing?

Trying to use a scope within an include. The example is contrived but in our codebase we actually reuse lots of named scopes. (This is a reopen of #10501 , as the bug appears to still exist on version 5.8.6.)

/**
 * Model definitions
 */
const User = sequelize.define('user', {
  id: { type: Sequelize.INTEGER, primaryKey: true },
  name: Sequelize.STRING
})

const Project = sequelize.define('project', {
  id: { type: Sequelize.INTEGER, primaryKey: true },
  title: Sequelize.STRING,
  userid: { type: Sequelize.INTEGER, references: { model: 'user', key: 'id' } }
})

const Task = sequelize.define('task', {
  id: { type: Sequelize.INTEGER, primaryKey: true },
  description: Sequelize.STRING,
  projectid: { type: Sequelize.INTEGER, references: { model: 'project', key: 'id' } }
})

/**
 * Associations
 */
Project.belongsTo(User, { foreignKey: 'userid' })
Task.belongsTo(Project, { foreignKey: 'projectid' })

/**
 * Go for bug
 */
Task.findByPk(1, {
  include: [
    Project.scope({ include: [User] }) // This is the offending include
  ]
}).then(task => {
  // The reload() will crash because it will try to include Project->User twice
  task.reload().then(task => {
    console.log(task)
  })
})

To Reproduce

• Fetch Sequelize instance with a scope containing an include
• Try to reload that instance

What do you expect to happen?

The task instance should just reload.

What is actually happening?

I get an SQL error and a crash :

sqlite: Unhandled rejection SequelizeDatabaseError: SQLITE_ERROR: ambiguous column name: project->user.id
PostgreSQL : Unhandled rejection SequelizeDatabaseError: table name "project->user" specified more than once

The offending SQL looks like this (sqlite)

select
	`task`.`id`,
	`task`.`description`,
	`task`.`projectid`,
	`project`.`id` as `project.id`,
	`project`.`title` as `project.title`,
	`project`.`userid` as `project.userid`,
	`project->user`.`id` as `project.user.id`,
	`project->user`.`name` as `project.user.name`,
	`project->user`.`id` as `project.user.id`,
	`project->user`.`name` as `project.user.name`
from
	`task` as `task`
left outer join `project` as `project` on
	`task`.`projectid` = `project`.`id`
left outer join `user` as `project->user` on
	`project`.`userid` = `project->user`.`id`
left outer join `user` as `project->user` on
	`project`.`userid` = `project->user`.`id`
where
	`task`.`id` = 1;

Environment

• Dialect:

  • postgres
  • sqlite

• Dialect library version: "sqlite3": "^4.0.6", "pg": "^7.4.1"

• Database version: XXX

• Sequelize version: 5.8.6

• Node Version: v10.15.3

• OS: Ubuntu 19.04

• Tested with latest release:

  • No
  • Yes, specify that version: 5.8.6

[WaylonOKC]

I think the original issue was all the way back in V3 (#4658). Hopefully someone will write a test for this since it keeps getting introduced.

[WaylonOKC]

Any update on this?

[WaylonOKC]

This has been around for awhile now and is really the only thing preventing us from upgrading to V5. Any update?

[papb]

Sorry, no updates for now. I wanted to refactor the way scopes work, I would surely pay attention to fix this as a side-effect, but that would take too much time that I really don't have at the moment.

[papb]

I think the original issue was all the way back in V3 (#4658). Hopefully someone will write a test for this since it keeps getting introduced.

Suddenly I understood what you're saying here! This was already fixed once (in #5106), right? And appeared again. This is unfortunate but at least there is an easier way to start re-investigating :)

[Dam-Buty]

Hi ! this bug has been tested and reproduced in 5.21.13 . Is there any fix planned ?

[davobutt]

For a workaround doing model.unscoped().reload() seems to retain the includes from the original query, but not reapply the default scope. In simple cases that will work. But obviously once this gets fixed and you update it's going to fail again.

[juliocanares]

example to reproduce in sequelize 6.3.5

const Feature = sequelize.define()

const Vendor = sequelize.define()

Vendor.hasMany(Feature)

Vendor.scope('withFeatures', () => ({include:[Feature]}))

const vendor = await models.Vendor.scope(['withFeatures']).findOne()

vendor.reload() // failed with "SequelizeDatabaseError: Not unique table/alias: 'Features'"

digging in the code I could see that reload uses

this._options.include https://github.com/sequelize/sequelize/blob/master/src/model.js#L4331 from the reloaded model and inject the scopes again in the findAll method which is called by findOne, in findAll this._conformIncludes https://github.com/sequelize/sequelize/blob/master/src/model.js#L1807 does not call to this._uniqIncludes https://github.com/sequelize/sequelize/blob/master/src/model.js#L850 that does the include deduplication, but it also fails in this case because of the omit function that uses ${include.model && include.model.name}-${include.as} https://github.com/sequelize/sequelize/blob/master/src/model.js#L854.

I fixed it by adding this._uniqIncludes right after this._conformIncludes and explicitly setting the alias in the scope

Vendor.scope('withFeatures', () => ({include:[{model: Feature, as: 'Features'}]}))

[durek1337]

@juliocanares Your workaround with defining the "as"-property explicitly solved my problem. I don't like this solution because it's redundant. But it helped me, thanks!

This is my code. I simply added , as : 'Pupils' in my scope-definition.

Pupil.belongsToMany(Course, {through: PupilCourse, foreignKey : "pupil_id", otherKey : "course_id"})
Course.belongsToMany(Pupil,{through: PupilCourse, foreignKey : "course_id", otherKey : "pupil_id"})

Course.addScope("complete",{
      include : [
        {model : Pupil, attributes : ["id"], as : 'Pupils'}
      ]
    })

This seems to be a known bug(?) since 2015 or earlier. Is this really that hard to fix? 🤔
#3855
#3973

A simple solution would be a default-value for the "as"-property with the Modelname in plural, or not?

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

[tigermax139]

Issue still actual for Sequelize v6

[Valetek]

Hello, as said upper, this is still present in Sequelize v6.5 too

After some investigation the issue is happening in sequelize/lib/model.js line 4070 (reload function)

    options = Utils.defaults({
      where: this.where()
    }, options, { 
      include: this._options.include || undefined // This include the previous include
    });

    const reloaded = await this.constructor.findOne(options); // This will trigger again the includes in the scopes

as a workaround I made this in my reload call

reload({ include: [] }) 
// This will override the include in _options and remove the duplicated call, should work even if it's not empty until it's not the same include than the one in the scope

Would be good if this case could be resolved still ... scopes are useful