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

many-to-many-to-many query #11796

Closed
incutonez opened this issue Jan 5, 2020 · 19 comments · Fixed by #11825
Closed

many-to-many-to-many query #11796

incutonez opened this issue Jan 5, 2020 · 19 comments · Fixed by #11825
Labels
status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference.

Comments

@incutonez
Copy link

incutonez commented Jan 5, 2020

Issue Description

I'm trying to get an output like this:

[{
  "Id": 1,
  "Name": "Game 1",
  "Teams": [{
    "Id": 1,
    "Name": "Team 1",
    "Users": [{
      "Id": 1,
      "UserName": "User 1"
    }]
  }]
}, {
  "Id": 2,
  "Name": "Game 2",
  "Teams": [{
    "Id": 1,
    "Name": "Team 1",
    "Users": [{
      "Id": 2,
      "UserName": "User 2"
    }]
  }]
}]

Note that Team 1 has 2 different users, but that's only because they're set up that way per game... so a user isn't tied directly to a team, but rather through a team game constraint. Basically, my Game HasMany Teams, and my Game/Team HasMany Users... a many-to-many-to-many relationship. I was trying to follow this thread, but it seems like what they're doing there doesn't actually work, as I tried doing this:

// models/Game.js
module.exports = (sequelize, types) => {
  const GameModel = sequelize.define('Game', {
    Id: {
      type: types.INTEGER,
      primaryKey: true,
      autoIncrement: true
    },
    Name: {
      type: types.STRING,
      allowNull: false
    }
  });

  GameModel.associate = (models) => {
    GameModel.belongsToMany(models.Team, {
      as: 'Teams',
      foreignKey: 'GameId',
      through: models.GameTeam
    });
  };

  return GameModel;
};

// models/Team.js
module.exports = (sequelize, types) => {
  const TeamModel = sequelize.define('Team', {
    Id: {
      type: types.INTEGER,
      primaryKey: true,
      autoIncrement: true
    },
    Name: {
      type: types.STRING,
      allowNull: false
    }
  });

  TeamModel.associate = (models) => {
    TeamModel.belongsToMany(models.Game, {
      as: 'Games',
      foreignKey: 'TeamId',
      through: models.GameTeam
    });
  };

  return TeamModel;
};

// models/User.js
module.exports = (sequelize, types) => {
  const UserModel = sequelize.define('User', {
    Id: {
      type: types.INTEGER,
      primaryKey: true,
      autoIncrement: true
    },
    UserName: {
      type: types.STRING,
      allowNull: false
    }
  });

  return UserModel;
};

// models/GameTeam.js
module.exports = (sequelize, types) => {
  const GameTeamModel = sequelize.define('GameTeam', {
    Id: {
      type: types.INTEGER,
      primaryKey: true,
      autoIncrement: true
    }
  });

  GameTeamModel.associate = (models) => {
    GameTeamModel.belongsToMany(models.User, {
      as: 'Users',
      through: 'GameTeamUser'
    });
  };

  return GameTeamModel;
};

// models/GameTeamUser.js
module.exports = (sequelize, types) => {
  const GameTeamUserModel = sequelize.define('GameTeamUser', {
    Id: {
      type: types.INTEGER,
      primaryKey: true,
      autoIncrement: true
    }
  });
  return GameTeamUserModel;
};

The above models create the tables just fine, with what appears to be the appropriate columns. I then do some inserts and try to use a findAll on the Game model like this:

GameModel.findAll({
  include: [{
    association: GameModel.associations.Teams,
    include: [{
      association: GameTeamModel.associations.Users,
      through: {
        attributes: []
      }
    }],
    through: {
      attributes: []
    }
  }]
});

The query starts to go wrong at the 2nd include with the association of the Users. Because I'm trying to nest the users inside of the teams, I figured the join would attempt to use the unique ID on the through table (GameTeams.Id), but instead, the query ends up using this:

LEFT OUTER JOIN `GameTeamUser` AS `Teams->Users->GameTeamUser` ON `Teams`.`Id` = `Teams->Users->GameTeamUser`.`GameTeamId`

I figured the ON would be GameTeams.Id = Teams->Users->GameTeamuser.GameTeamId, but I don't know why it's not, and how to adjust it... I've tried using a custom on in my include (per the docs), but it seems to be ignored completely. Anyone have any advice? Or possibly a better way of structuring this, so it works the way I want it to?

StackOverflow / Slack attempts

StackOverflow thread.

Additional context

package.json deps (using SQLite as the backing DB):

"dependencies": {
    "cookie-parser": "~1.4.4",
    "debug": "~2.6.9",
    "express": "~4.16.1",
    "ip": "^1.1.5",
    "morgan": "~1.9.1",
    "sequelize": "^5.21.3",
    "sequelize-cli": "^5.5.1",
    "sqlite3": "^4.1.1"
  }

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
[✔] I don't know.

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.

@incutonez
Copy link
Author

I wanted a thread for this, so I could post other things that I've done, instead of making the question overly long on SO. Since creating that thread, I've tried going the raw SQL route, and it works, except it doesn't turn the response into the desired JSON structure. This is the query that I'm doing:

router.get('/', async (req, res, next) => {
sequelize.query('SELECT Game.Id, Game.Name, Teams.Id AS `Teams.Id`, Teams.Name AS `Teams.Name`,`Teams.GameTeam`.Id AS `Teams.GameTeam.Id`,`Teams.GameTeam`.GameId AS `Teams.GameTeam.GameId`,`Teams.GameTeam`.TeamId AS `Teams.GameTeam.TeamId`,`Teams.Users`.Id AS `Teams.Users.Id`,`Teams.Users`.UserName AS `Teams.Users.UserName`,`Teams.Users.GameTeamUser`.GameTeamId AS `Teams.Users.GameTeamUser.GameTeamId`,`Teams.Users.GameTeamUser`.UserId AS `Teams.Users.GameTeamUser.UserId` FROM Games AS Game LEFT OUTER JOIN GameTeams AS `Teams.GameTeam` ON Game.Id = `Teams.GameTeam`.GameId LEFT OUTER JOIN Teams AS Teams ON Teams.Id = `Teams.GameTeam`.TeamId LEFT OUTER JOIN GameTeamUser AS `Teams.Users.GameTeamUser` ON `Teams.GameTeam`.Id = `Teams.Users.GameTeamUser`.GameTeamId LEFT OUTER JOIN Users AS `Teams.Users` ON `Teams.Users`.Id = `Teams.Users.GameTeamUser`.UserId', {
    model: db.Game,
    include: [{
      model: db.Team,
      as: 'Teams',
      include: [{
        model: db.GameTeam,
        as: 'GameTeam'
      }, {
        model: db.User,
        as: 'Users',
        include: [{
          model: db.GameTeamUser,
          as: 'GameTeamUser'
        }]
      }]
    }]
  }).then((recs) => {
    res.send(recs);
  });
});

And this is the resulting JSON:

[
  {
    "Id": 1,
    "Name": "NO TEAMS",
    "Teams.Id": null,
    "Teams.Name": null,
    "Teams.GameTeam.Id": null,
    "Teams.GameTeam.GameId": null,
    "Teams.GameTeam.TeamId": null,
    "Teams.Users.Id": null,
    "Teams.Users.UserName": null,
    "Teams.Users.GameTeamUser.GameTeamId": null,
    "Teams.Users.GameTeamUser.UserId": null
  },
  {
    "Id": 2,
    "Name": "TEAMS",
    "Teams.Id": 1,
    "Teams.Name": "Team 1",
    "Teams.GameTeam.Id": 2,
    "Teams.GameTeam.GameId": 2,
    "Teams.GameTeam.TeamId": 1,
    "Teams.Users.Id": 3,
    "Teams.Users.UserName": "User 3",
    "Teams.Users.GameTeamUser.GameTeamId": 2,
    "Teams.Users.GameTeamUser.UserId": 3
  },
  {
    "Id": 2,
    "Name": "TEAMS",
    "Teams.Id": 3,
    "Teams.Name": "Team 3",
    "Teams.GameTeam.Id": 1,
    "Teams.GameTeam.GameId": 2,
    "Teams.GameTeam.TeamId": 3,
    "Teams.Users.Id": 1,
    "Teams.Users.UserName": "User 1",
    "Teams.Users.GameTeamUser.GameTeamId": 1,
    "Teams.Users.GameTeamUser.UserId": 1
  },
  {
    "Id": 2,
    "Name": "TEAMS",
    "Teams.Id": 3,
    "Teams.Name": "Team 3",
    "Teams.GameTeam.Id": 1,
    "Teams.GameTeam.GameId": 2,
    "Teams.GameTeam.TeamId": 3,
    "Teams.Users.Id": 2,
    "Teams.Users.UserName": "User 2",
    "Teams.Users.GameTeamUser.GameTeamId": 1,
    "Teams.Users.GameTeamUser.UserId": 2
  }
]

It's so close... I just need to figure out how to return associations in the desired structure.

@incutonez
Copy link
Author

incutonez commented Jan 5, 2020

Following this thread, I was able to get the raw query to output the structure that I need. Basically, I had to add the property hasJoin: true to my query options AND I had to validate the options with db.Game._validateIncludedElements(options);. Resulting JSON:

[
  {
    "Id": 1,
    "Name": "NO TEAMS",
    "Teams": []
  },
  {
    "Id": 2,
    "Name": "TEAMS",
    "Teams": [
      {
        "Id": 1,
        "Name": "Team 1",
        "Users": [
          {
            "Id": 3,
            "UserName": "User 3"
          }
        ]
      },
      {
        "Id": 3,
        "Name": "Team 3",
        "Users": [
          {
            "Id": 1,
            "UserName": "User 1"
          },
          {
            "Id": 2,
            "UserName": "User 2"
          }
        ]
      }
    ]
  }
]

However, it'd still be nice to know how to do this without creating the raw query, mainly because it's possible the tables change, and I need the query to update without having to edit it myself. If anyone has any advice on that, I'm all ears, but it seems like the on isn't respected for belongsToMany queries?

@incutonez
Copy link
Author

As a quick test, I wanted to see if I could use the on keyword for a hasMany relationship, and I was able to, so it seems like on for belongsToMany either isn't working for SQLite, or it hasn't been implemented.

@incutonez
Copy link
Author

incutonez commented Jan 6, 2020

Started doing some debugging, and in sequelize/lib/dialects/abstract/query-generator.js, I modified generateThroughJoin... as this is the code path that's taken for belongsToMany. In there, I changed this if to look for include.on in the else branch, and the sourceJoinOn append after.

if (topLevelInfo.subQuery && !include.subQuery && include.parent.subQuery && !parentIsTop) {
  // If we are minifying aliases and our JOIN target has been minified, we need to use the alias instead of the original column name
  const joinSource = this._getAliasForField(tableSource, `${tableSource}.${attrSource}`, topLevelInfo.options) || `${tableSource}.${attrSource}`;

  sourceJoinOn = `${this.quoteIdentifier(joinSource)} = `;
}
else {
  // If we are minifying aliases and our JOIN target has been minified, we need to use the alias instead of the original column name
  const aliasedSource = this._getAliasForField(tableSource, attrSource, topLevelInfo.options) || attrSource;

  // STARTING MY PATCH
  if (include.on) {
    sourceJoinOn = this.whereItemsQuery(include.on, {
      prefix: this.sequelize.literal(this.quoteIdentifier(throughAs)),
      model: include.model
    });
  }
  else {
    // This is the original code that was here
    sourceJoinOn = `${this.quoteTable(tableSource)}.${this.quoteIdentifier(aliasedSource)} = `;
  }
}
if (!include.on) {
  // Original code
  sourceJoinOn += `${this.quoteIdentifier(throughAs)}.${this.quoteIdentifier(identSource)}`;
}
// ENDING MY PATCH

findAll options:

include: [{
  association: db.Game.associations.Teams,
  through: {
    attributes: []
  },
  include: [{
    association: db.GameTeam.associations.Users,
    through: {
      attributes: []
    },
    on: {
      '$Teams.GameTeam.Id$': {
        [Op.eq]: sequelize.col('Teams.Users.GameTeamUsers.GameTeamId')
      }
    }
  }]
}]

I honestly just took a wild stab at this, so I'm sure it breaks something, but it does allow me to use custom ons... still going to dig around, and see if I'm missing something out of the box that I can use.

incutonez pushed a commit to incutonez/JefBox that referenced this issue Jan 6, 2020
@papb
Copy link
Member

papb commented Jan 7, 2020

Hello! Thanks for opening this issue and posting all your attempts.

The bad news is that I don't have time to read everything you posted thoroughly right now.

Basically, my Game HasMany Teams, and my Game/Team HasMany Users... a many-to-many-to-many relationship.

The great news is that I read this part, and I know a great way to do that. No need to patch anything in Sequelize. It just works. I needed exactly this recently (actually what I needed was a many-to-many-to-many-to-many-to-many relationship, which is more than enough to help you) 😬

Please wait a few more days, in fact I already started updating the Sequelize manuals to show how to do this. Hopefully in the near future I will have a documentation PR that will give you a direct answer (unless you have more specific needs that I didn't read in your follow-up posts...).

@papb papb added status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference. labels Jan 7, 2020
@papb papb self-assigned this Jan 7, 2020
@incutonez
Copy link
Author

That's wonderful news! I at least have a way of getting the data the way I want to, so I've unblocked myself and can update the code with the proper way when you update the docs. Thanks!

@papb papb removed the status: wip For issues and PRs. Applied when the PR is not ready yet / when work to close the issue has started. label Jan 15, 2020
@incutonez
Copy link
Author

@papb I followed your Many-to-many-to-many relationships and beyond article... thanks for that, by the way! But there's one thing that I'm a little confused on... is there a cute way of joining the Team information as properties in the output? e.g. this is my current output:

{
  "GameTeams": [
    {
      "Id": 1,
      "GameId": 3,
      "TeamId": 1,
      "Users": [
        {
          "Id": 1,
          "UserName": "User 1"
        },
        {
          "Id": 2,
          "UserName": "User 2"
        }
      ],
      "Team": {
        "Id": 1,
        "Name": "Team 1"
      }
    }
  ]
}

Basically, I want to shift the Team object into the GameTeams properties, so it'd look like this:

{
  "GameTeams": [
    {
      "Id": 1,
      "GameId": 3,
      "TeamId": 1,
      "TeamName": "Team 1",
      "Users": [
        {
          "Id": 1,
          "UserName": "User 1"
        },
        {
          "Id": 2,
          "UserName": "User 2"
        }
      ]
    }
  ]
}

I can accomplish this by having an include like this:

{
  model: models.GameTeam,
  required: false,
  attributes: [
    [conn.literal('`GameTeams->Team`.Name'), 'TeamName']
  ],
  include: [{
    model: models.User,
    through: {
      attributes: []
    }
  }, {
    model: models.Team,
    attributes: []
  }]
}

But I don't like doing the literal like that. Do you have any advice? Thanks in advance.

@papb
Copy link
Member

papb commented Jan 16, 2020

Hello! Glad it helped.

The only thing that comes to my mind is a simple JS post-processing...

for (const gameTeam of result.GameTeams) {
  gameTeam.TeamName = gameTeam.Team.Name;
}

@papb
Copy link
Member

papb commented Jan 16, 2020

Actually another thing just came to my mind, but might be a bit ugly, I'm not sure. It is adding a TeamName virtual attribute on the GameTeam model that only gets populated if the Team is present, like this:

const GameTeam = sequelize.define('GameTeam', {
  /* normal attributes */,
  TeamName: {
    type: DataTypes.VIRTUAL,
    get() {
      if (!this.Team) return undefined;
      return this.Team.Name;
    }
});

@incutonez
Copy link
Author

Hmm, the virtual is interesting... It only becomes a little tedious if there are more properties I want to add. Thanks for the comments though... I'd at least not be doing the literal like that.

@incutonez
Copy link
Author

Actually another thing just came to my mind, but might be a bit ugly, I'm not sure. It is adding a TeamName virtual attribute on the GameTeam model that only gets populated if the Team is present, like this:

const GameTeam = sequelize.define('GameTeam', {
  /* normal attributes */,
  TeamName: {
    type: DataTypes.VIRTUAL,
    get() {
      if (!this.Team) return undefined;
      return this.Team.Name;
    }
});

As a follow-up, this doesn't work as well as I was hoping... I can't use attributes: [] on the Team include because if I did, by the time it gets to the virtual getter, the Team object is not actually there. I can kind of kludge it by unsetting the Team object/deleting it, but that's not ideal. I'll keep playing with this though... maybe something else will click.

@incutonez
Copy link
Author

@sushantdhiman Could you reopen this, please? The updates to the docs have not actually given me the output that I desire, so this is still an issue.

@papb papb reopened this Jan 17, 2020
@papb
Copy link
Member

papb commented Jan 17, 2020

@incutonez may I ask why is a specific way of having the data output structured such a big deal?

@papb papb removed their assignment Jan 17, 2020
@papb papb added the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Jan 17, 2020
@incutonez
Copy link
Author

@papb It's fairly common to have a data signature like what I originally posted... Usually the join table isn't included and rather the associated entity takes its place. At least, that's how I've seen it in things like LINQ, so I'm not sure if that's common, but from a data signature, it makes sense... Don't want overly nested objects. Does that make sense?

@papb
Copy link
Member

papb commented Jan 19, 2020

@incutonez Hmmm, I don't know, to be honest what you asked here seemed unintuitive to me (I may be too used to Sequelize though). As for the code structure in your first post, I don't know a way to get it directly, sorry.

However, may I ask why can't you just write some javascript code that re-strucutres the result in whatever way you like? I doubt the performance impact of doing that will be too relevant.

@incutonez
Copy link
Author

@papb, no worries... appreciate the help you've given thus far.

I actually have overridden the toJSON call for my Game instances... it does seem to be the easiest way now, and I think it'll help out with all of these other associations that I'm doing. It feels wrong because I'm going to have to maintain yet another part of the code, which kind of goes back to, why am I using an ORM if I'm essentially reworking the return structure entirely?

For anyone interested, this is what I'm doing in toJSON:

GameModel.prototype.toJSON = function() {
  const teams = [];
  const data = Object.assign({}, this.get());
  const gameTeams = data.GameTeams;
  if (gameTeams) {
    for (let i = 0; i < gameTeams.length; i++) {
      const gameTeam = gameTeams[i];
      const team = gameTeam.Team.get();
      const users = [];
      for (let j = 0; j < gameTeam.Users.length; j++) {
        users.push(gameTeam.Users[j].get());
      }
      team.Users = users;
      teams.push(team);
    }
    data.Teams = teams;
    delete data.GameTeams;
  }
  return data;
};

@papb
Copy link
Member

papb commented Jan 19, 2020

Do you agree that this issue ended up changing topics? Your current problem now is about configuring the output structure of Sequelize finder methods, and no longer really a many-to-many-to-many problem. Do you agree? If yes, could you please open a new issue focused on that, so we can track the problem there and close this one? What do you think?

@incutonez
Copy link
Author

I'm not entirely convinced it has changed... it started out with me asking how to get that output, and I still haven't learned how to do that naturally through the ORM without doing manual manipulation... unless that is the only way of doing it. It really sounds like this is not possible in the current version though, so I guess I'll just have to chalk it up to that's what I get for using this ORM.

@jedwards1211
Copy link
Contributor

@papb although your new many-to-many-to-many example in the docs is a solution, it's not ideal because it requires two join tables (GameTeam and PlayerGameTeam), whereas using raw SQL, it's possible to have a single join table with all three foreign keys. Imagine if someone has a pre-existing database with a three-way join table and tries to start using Sequelize on it...they would have to keep using raw SQL. I think an API like this would be a welcome addition:

sequelize.joinThrough(PlayerGameTeam, {
  playerId: Player,
  teamId: Team,
  gameId: Game,
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants