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

v3: sourceKey feature for hasMany doesn't work if where clause specified in include #7141

Closed
LoneWolfPR opened this Issue Jan 24, 2017 · 2 comments

Comments

1 participant
@LoneWolfPR
Contributor

LoneWolfPR commented Jan 24, 2017

I installed the update that adds sourceKey to the hasMany feature. Initially it was working fine, however as soon as I put a where clause in the include of a find call it switched back to ignoring my specified sourceKey and is only looking at the primary key.

Here is my association defined.

...
classMethods: {
  associate: (models) => {
    Contact.hasMany(models.Role, { foreignKey: 'contact__c', sourceKey: 'sfid' });
  },
},

Here is the find method I'm using.

findByAuth0Id (auth0_id) {
  return db.User.find({
    where: { auth0_id },
    include: [{
      model: db.Contact,
      include: [
        {
          model: db.Role,
          where: { role_status__C: 'Active' },
          include: [db.Account],
        }],
    },
      db.Student,
      db.Clickwrap,
      db.Invitation,
      db.Notification,
    ],
  });
}

I had expected to get back a tree starting with the user that is found which would then include it's matching contact record. Then underneath the contact would be each active role with the active accounts attached to those roles.

I get Unhandled rejection SequelizeDatabaseError: operator does not exist: integer = character varying because the join on the contact and role tables is being done via the primary key on the contact table instead of the sourceKey I have specified in the hasMany association. If I leave everything the same, but simply remove the where clause in the include it works.

I'd be willing to work on a fix for this myself. I tried debugging the code, but I can't seem to find exactly where the final query gets written that is causing the problem. If no one else wants to take on fixing it I'd take it up if someone can point me in the right direction. I've gone through has-many.js, and the problem does not appear to be there as far as I can tell.

postgres v9.6.1
sequelize v3.30.0

@LoneWolfPR

This comment has been minimized.

Contributor

LoneWolfPR commented Jan 24, 2017

I've actually found the problem. Currently we have this definition in lib/dialects/abstract/query-generator.js at line 1708

, attrLeft = association instanceof BelongsTo ?
                   association.identifier :
                   left.primaryKeyAttribute

i believe It should be

, attrLeft = association instanceof BelongsTo ?
                   association.identifier :
                   [association.sourceIdentifier || left.primaryKeyAttribute]
@LoneWolfPR

This comment has been minimized.

Contributor

LoneWolfPR commented Jan 24, 2017

I have the repo pulled, and I'm ready to put in the fix, but I tried running the tests first and can't get them to complete. There's also a problem with a file disappearing after anytime tests fail. See this issue for details: #7143

janmeier added a commit that referenced this issue Jan 31, 2017

v3: sourceKey feature for hasMany doesn't work if where clause specif…
…ied in include (#7141) (#7147)

* Initial Commit

* Stops test from removing test/integration/assets/es6project.js

* Fixes impromper syntax when specifying the use of association.sourceIdentifier or left.primaryKeyAttribute for attrLeft. Also includes integration test.

* Updates changelog.

* Adds link to issue in changelog entry.

* Added 'Future' header to changelog entry

* Removed changelog update as requested.

* Updates changelog. Fixes query-generator to properly build queries from hasMany even with an include while respecting a specified sourceKey. Adds an integration test.

* Added back removed whitespace per request.

* Updates test in include altered field name in source key. Fixes were then made to address the test breaking. All tests pass now.

* Adds back removed whitespace.

janmeier added a commit that referenced this issue Jan 31, 2017

cherry-pick: 3168d54
v3: sourceKey feature for hasMany doesn't work if where clause specified in include (#7141) (#7147)

* Initial Commit

* Stops test from removing test/integration/assets/es6project.js

* Fixes impromper syntax when specifying the use of association.sourceIdentifier or left.primaryKeyAttribute for attrLeft. Also includes integration test.

* Updates changelog.

* Adds link to issue in changelog entry.

* Added 'Future' header to changelog entry

* Removed changelog update as requested.

* Updates changelog. Fixes query-generator to properly build queries from hasMany even with an include while respecting a specified sourceKey. Adds an integration test.

* Added back removed whitespace per request.

* Updates test in include altered field name in source key. Fixes were then made to address the test breaking. All tests pass now.

* Adds back removed whitespace.

mkaufmaner added a commit to mkaufmaner/sequelize that referenced this issue Apr 25, 2017

cherry-pick: 3168d54
v3: sourceKey feature for hasMany doesn't work if where clause specified in include (sequelize#7141) (sequelize#7147)

* Initial Commit

* Stops test from removing test/integration/assets/es6project.js

* Fixes impromper syntax when specifying the use of association.sourceIdentifier or left.primaryKeyAttribute for attrLeft. Also includes integration test.

* Updates changelog.

* Adds link to issue in changelog entry.

* Added 'Future' header to changelog entry

* Removed changelog update as requested.

* Updates changelog. Fixes query-generator to properly build queries from hasMany even with an include while respecting a specified sourceKey. Adds an integration test.

* Added back removed whitespace per request.

* Updates test in include altered field name in source key. Fixes were then made to address the test breaking. All tests pass now.

* Adds back removed whitespace.

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

@stale stale bot closed this Jul 7, 2017

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