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

Deep eager load chains does not produce valid sql, regression from sequelize 3 #9869

Open
linpekka opened this issue Sep 4, 2018 · 68 comments
Labels
P1: important For issues and PRs. regression For issues and PRs. A bug that does not happen in an earlier version of Sequelize. topic: limit & offset For issues and PRs. Things that involve queries with limit and/or offset. type: bug

Comments

@linpekka
Copy link

linpekka commented Sep 4, 2018

What are you doing?

When upgrading from sequelize 3, i noticed that some lookups does no longer produce valid sql.

'use strict';

const Sequelize = require('sequelize');
const Op = Sequelize.Op;

let Model, ModelB, ModelC, ModelD, ModelE;

/**
 * @param {boolean} useEnum
 */
async function test(useEnum) {
  const db = new Sequelize('test', 'user', 'user', {
    host: '127.0.0.1',
    dialect: 'postgresql',
    "quoteIdentifiers": true, 
    "underscored": true
  });  

  Model = db.define('Model', { not_id : Sequelize.INTEGER }, {});
  ModelB = db.define('ModelB', { model_id : Sequelize.INTEGER }, {});
  ModelC = db.define('ModelC', { modelB_id : Sequelize.INTEGER }, {});
  ModelD = db.define('ModelD', { modelC_id : Sequelize.INTEGER, modelE_id : Sequelize.INTEGER }, {});
  ModelE = db.define('ModelE', {}, {});

  Model.hasMany(ModelB, {foreignKey: 'model_id'});
  ModelB.belongsTo(Model, {foreignKey: 'model_id'});

  ModelB.hasMany(ModelC, {foreignKey: 'modelB_id'});
  ModelC.belongsTo(ModelB, {foreignKey: 'modelB_id'});

  ModelC.hasMany(ModelD, {foreignKey: 'modelC_id'});
  ModelD.belongsTo(ModelC, {foreignKey: 'modelC_id'});

  ModelD.belongsTo(ModelE, {foreignKey: 'modelE_id'});
  ModelE.hasMany(ModelD, {foreignKey: 'modelE_id'});

  await db.sync({ force: true });

  await populateDb();

  console.log('Find that does not reproduce the issue')
  const t1 = await Model.findOne({where: {id:1}, include:[
    {
      model:ModelB,
      required: true,
      include:[
        {
          model:ModelC,
          required: true,
          include:[
            {
              model:ModelD,
              required: true,
              include:[
                {
                  model:ModelE,
                  required: true
                }
              ]
            }
          ]
        }
      ]
    }
  ]});

  console.log('Find that reproduces the issue')
  const t2 = await Model.findOne({where: {not_id:2}, include:[
    {
      model:ModelB,
      required: true,
      include:[
        {
          model:ModelC,
          required: true,
          include:[
            {
              model:ModelD,
              required: true,
              include:[
                {
                  model:ModelE,
                  required: true
                }
              ]
            }
          ]
        }
      ]
    }
  ]});

  await db.close();
}

async function populateDb() {
  const model = await Model.create({ not_id:2});
  const modelB = await ModelB.create({ model_id:model.id });
  const modelC = await ModelC.create({ modelB_id:modelB.id });

  const modelE = await ModelE.create({ });

  const modelD = await ModelD.create({ modelC_id:modelC.id, modelE_id:modelE.id });
}

async function main() {
  try {
    console.log('Bad sql');
    await test();
  } catch (err) {
    console.error(err);
    process.exit(1);
  }
}

main();

What do you expect to happen?

I want the findOne to generate valid sql

What is actually happening?

SequelizeDatabaseError: missing FROM-clause entry for table "ModelBs->ModelCs->ModelDs->ModelE"

Output, either JSON or SQL

'SELECT "Model".*, "ModelBs"."id" AS "ModelBs.id", "ModelBs"."model_id" AS "ModelBs.model_id", "ModelBs"."createdAt" AS "ModelBs.createdAt", "ModelBs"."updatedAt" AS "ModelBs.updatedAt", "ModelBs->ModelCs"."id" AS "ModelBs.ModelCs.id", "ModelBs->ModelCs"."modelB_id" AS "ModelBs.ModelCs.modelB_id", "ModelBs->ModelCs"."createdAt" AS "ModelBs.ModelCs.createdAt", "ModelBs->ModelCs"."updatedAt" AS "ModelBs.ModelCs.updatedAt", "ModelBs->ModelCs->ModelDs"."id" AS "ModelBs.ModelCs.ModelDs.id", "ModelBs->ModelCs->ModelDs"."modelC_id" AS "ModelBs.ModelCs.ModelDs.modelC_id", "ModelBs->ModelCs->ModelDs"."modelE_id" AS "ModelBs.ModelCs.ModelDs.modelE_id", "ModelBs->ModelCs->ModelDs"."createdAt" AS "ModelBs.ModelCs.ModelDs.createdAt", "ModelBs->ModelCs->ModelDs"."updatedAt" AS "ModelBs.ModelCs.ModelDs.updatedAt" FROM (SELECT "Model"."id", "Model"."not_id", "Model"."createdAt", "Model"."updatedAt", "ModelBs->ModelCs->ModelDs->ModelE"."id" AS "ModelBs.ModelCs.ModelDs.ModelE.id", "ModelBs->ModelCs->ModelDs->ModelE"."createdAt" AS "ModelBs.ModelCs.ModelDs.ModelE.createdAt", "ModelBs->ModelCs->ModelDs->ModelE"."updatedAt" AS "ModelBs.ModelCs.ModelDs.ModelE.updatedAt" FROM "Models" AS "Model" WHERE "Model"."not_id" = 2 AND ( SELECT "ModelBs"."model_id" FROM "ModelBs" AS "ModelBs" INNER JOIN "ModelCs" AS "ModelCs" ON "ModelBs"."id" = "ModelCs"."modelB_id" INNER JOIN "ModelDs" AS "ModelCs->ModelDs" ON "ModelCs"."id" = "ModelCs->ModelDs"."modelC_id" INNER JOIN "ModelEs" AS "ModelCs->ModelDs->ModelE" ON "ModelCs->ModelDs"."modelE_id" = "ModelCs->ModelDs->ModelE"."id" WHERE ("ModelBs"."model_id" = "Model"."id") LIMIT 1 ) IS NOT NULL LIMIT 1) AS "Model" INNER JOIN "ModelBs" AS "ModelBs" ON "Model"."id" = "ModelBs"."model_id" INNER JOIN "ModelCs" AS "ModelBs->ModelCs" ON "ModelBs"."id" = "ModelBs->ModelCs"."modelB_id" INNER JOIN "ModelDs" AS "ModelBs->ModelCs->ModelDs" ON "ModelBs->ModelCs"."id" = "ModelBs->ModelCs->ModelDs"."modelC_id";'

__Dialect:postgres
__Dialect version:(PostgreSQL) 9.6.5
__Database version:(PostgreSQL) 9.6.5
__Sequelize version:4.38.0
__Tested with latest release:4.38.0

Note : Your issue may be ignored OR closed by maintainers if it's not tested against latest version OR does not follow issue template.

@sushantdhiman
Copy link
Contributor

How I am supposed to solve this :D Please distil your issue to proper test case which represents what is not working


Please use Github Issue Tracker only for reporting bugs, requesting new features or discussions. Ask questions on Stackoverflow sequelize.js tag or Slack. While I would like to help answer any questions but it still take too much time.

If you are reporting a bug please isolate it as SSCCE, present it nicely and follow issue template. This will help maintainers and contributors understand your issue quickly.

If you are requesting a feature spend some time to properly present your use case with some examples, current and expected outcome.

( What is SSCCE ? You can find template for Sequelize here )

A few good example of SSCCE

  1. ResourceRequest timed out - connection not reestablished after db restarted #8014 (comment)
  2. removeAttribute('id') results in undefined: null data value #7318 (comment)
  3. toJSON converting booleans to "t" (true) or "f" (false) #8749 (comment)

@linpekka
Copy link
Author

linpekka commented Sep 5, 2018

I edited the original post with a new self contained testcase structured similarily to #8749

@linpekka
Copy link
Author

linpekka commented Sep 5, 2018

@sushantdhiman - seems like i cannot reopen it, or perhaps I am just blind. If I dont hear from you in a couple of hours I guess I will open a new defect with the new test data in it.

@linpekka
Copy link
Author

linpekka commented Sep 5, 2018

Updated testcase, removed a few models that did not affect reproduction of the issue

@papigers
Copy link

papigers commented Sep 5, 2018

@sushantdhiman @linpekka A similar bug can be found at my issue #9861.
They might be both be a cause of the same bug, but my issue also adds another info on the issue: the temporary solution of using duplicating: false and how using scope breaks this solution.

@linpekka
Copy link
Author

linpekka commented Sep 6, 2018

@papigers, I see the similarities, hopefully enough information will soon be gathered so someone that maintains the project can figure out the issue, or issues that may well be the case. If you are looking for workarounds, what I ended up doing was to split the query into 2 smaller chained lookups, which in the end may not be that bad for performance since scheduling of the big sql might not be optimal. Another thing that could perhaps help is to split the where clause up and move it into the includes, not sure if that has any effect, but it looks like the missing table is used in the where part of the generated query.

@sushantdhiman
Copy link
Contributor

Please try against PR #9900 that tries to fix this issue, test case is generated after studying similar issues.

You can install this patch using npm install git://github.com/sequelize/sequelize.git#fix-deep-includes --save for testing its working. If enough users confirm this fix works I will merge and release it.

@papigers
Copy link

papigers commented Sep 9, 2018

@sushantdhiman So I've tested it a bit and it seems it helps, but not entirely:
This references my issue (#9861):

  1. The deep include I've mentioned seems to work without adding duplicating: false;
  2. Limit breaks it again with the same error:
SELECT "Video".*, "channel"."id" AS "channel.id", "channel"."name" AS "channel.name" FROM (SELECT "Video"."id", "Video"."createdAt", "Video"."name", "Video"."description" FROM "Videos" AS "Video" WHERE ("Video"."channelId" = \'MOCK_USER_ID\' OR "Video"."privacy" = \'public\' OR ((("videoACL"."id" = \'MOCK_USER_ID\' AND "videoACL"."type" = \'USER\') OR ("videoACL"."id" IN (NULL) AND "videoACL"."type" = \'AD_GROUP\')) AND "Video"."privacy" != \'public\') OR ((("channel->channelACL"."id" = \'MOCK_USER_ID\' AND "channel->channelACL"."type" = \'USER\') OR ("channel->channelACL"."id" IN (NULL) AND "channel->channelACL"."type" = \'AD_GROUP\')) AND "Video"."privacy" = \'channel\')) AND "Video"."published" = true LIMIT
12) AS "Video" LEFT OUTER JOIN "Channels" AS "channel" ON "Video"."channelId" = "channel"."id" LEFT OUTER JOIN "ChannelAccesses" AS "channel->channelACL" ON "channel"."id" = "channel->channelACL"."ChannelId" LEFT OUTER JOIN "VideoAccesses" AS "videoACL" ON "Video"."id" = "videoACL"."VideoId";
  1. When the same query with the deep includes is transferred to a scope, things not working either (even without a limit):
'SELECT "Video"."id", "Video"."createdAt", "Video"."name", "Video"."description", "channel"."id" AS "channel.id", "channel"."name" AS "channel.name" FROM "Videos" AS "Video" LEFT OUTER JOIN "Channels" AS "channel" ON "Video"."channelId" = "channel"."id" LEFT OUTER JOIN "VideoAccesses" AS "videoACL" ON "Video"."id" = "videoACL"."VideoId" WHERE ("Video"."channelId" = \'MOCK_USER_ID\' OR "Video"."privacy" = \'public\' OR ((("videoACL"."id" = \'MOCK_USER_ID\' AND "videoACL"."type" = \'USER\') OR ("videoACL"."id" IN (NULL) AND "videoACL"."type" = \'AD_GROUP\')) AND "Video"."privacy" != \'public\') OR ((("channel->channelACL"."id" = \'MOCK_USER_ID\' AND "channel->channelACL"."type" = \'USER\') OR ("channel->channelACL"."id" IN (NULL) AND "channel->channelACL"."type" = \'AD_GROUP\')) AND "Video"."privacy" = \'channel\'));'

@linpekka
Copy link
Author

linpekka commented Sep 10, 2018

@sushantdhiman, it does seem to fix this issue. I tried running the tests in my real project against this version as well, for extra testing, even if the issue here has been worked around in these tests, but failed with an error message. I noticed that the version you refered to seems to be based on version 5, and I am currently on version 4, so if it is possible to get a version 4 variant of the fix, I would be happy to run the rest of my tests against it.

"version": "5.0.0-beta.12"

The error message was from sequelize-hierarchy, so may be that the version of it I have been using is not compatible with sequelize 5, or that other parts of my code needs to be updated to comply with sequalize 5, but since I'm currently not done upgrading to version 4 I would not like to jump forward to version 5 yet.

"sequelize-hierarchy": "^1.3.2"

TypeError: semverSelect(...) is not a function
at module.exports (.../node_modules/sequelize-hierarchy/lib/errors.js:32:4)
at module.exports (.../node_modules/sequelize-hierarchy/lib/index.js:23:2)

@linpekka
Copy link
Author

linpekka commented Sep 17, 2018

@sushantdhiman, i have encountered what I think is a new instance of the same issue, an eager load that used to work but breaks when upgrading to sequelize v4. If I can get a patch of sequelize 4 instead of sequelize 5 I would be happy to test against it to verify that works after the fix, as well as the original querys that are the base of the reproduction case for this issue.

@sushantdhiman
Copy link
Contributor

@linpekka This patch should apply cleanly to v4 branch. For testing purpose you can simply replace these line inside node_modules/sequelize/*** as there is not build step code should be same.

@linpekka
Copy link
Author

linpekka commented Sep 17, 2018

I applied the changes to sequelize 4, and tested the new instance of eager load and limit that I found that resulted in a missing FROM-clause entry. Unfortunately it did not fix that one, so I guess I will have to create a new repro and a new issue for this case.

@linpekka
Copy link
Author

I also tested on one of the original queries that was base for the reproduction in this issue, and while the fix seems to take care of the missing FROM-clause entry, i run into issues with the column resolve part of the query, I guess it is because my aliases are not properly handled. I will try to find time to create a new repro-case that triggers this as well.

@linpekka
Copy link
Author

I did not find any new queries that was not generated, or negative side effect from the fix in my test-suite, i only had issue with queries that did not work before the fix.

@linpekka
Copy link
Author

linpekka commented Sep 18, 2018

I backed to a previous version of the reproduction script, which is more similar to my original query, and managed to reproduce the new issue I got when testing the fix. Edit - minimized the repro case a bit.

If I run this against the pull request from @sushantdhiman, i get the following error:
SequelizeDatabaseError: column "Ds.Es.Gs.H.I.J_id" does not exist

And this is the sql:

SELECT "C".*, "Ds"."id" AS "Ds.id", "Ds"."C_id" AS "Ds.C_id", "Ds"."createdAt" AS "Ds.createdAt", "Ds"."updatedAt" AS "Ds.updatedAt", "Ds->Es"."id" AS "Ds.Es.id", "Ds->Es"."D_id" AS "Ds.Es.D_id", "Ds->Es"."createdAt" AS "Ds.Es.createdAt", "Ds->Es"."updatedAt" AS "Ds.Es.updatedAt", "Ds->Es->Gs"."id" AS "Ds.Es.Gs.id", "Ds->Es->Gs"."E_id" AS "Ds.Es.Gs.E_id", "Ds->Es->Gs"."H_id" AS "Ds.Es.Gs.H_id", "Ds->Es->Gs"."createdAt" AS "Ds.Es.Gs.createdAt", "Ds->Es->Gs"."updatedAt" AS "Ds.Es.Gs.updatedAt", "Ds->Es->Gs->H->I->J"."id" AS "Ds.Es.Gs.H.I.J.id", "Ds->Es->Gs->H->I->J"."createdAt" AS "Ds.Es.Gs.H.I.J.createdAt", "Ds->Es->Gs->H->I->J"."updatedAt" AS "Ds.Es.Gs.H.I.J.updatedAt", "Ds->Es->Gs->H"."id" AS "Ds.Es.Gs.H.id", "Ds->Es->Gs->H"."G_id" AS "Ds.Es.Gs.H.G_id", "Ds->Es->Gs->H"."createdAt" AS "Ds.Es.Gs.H.createdAt", "Ds->Es->Gs->H"."updatedAt" AS "Ds.Es.Gs.H.updatedAt", "Ds->Es->Gs->H"."I_id" AS "Ds.Es.Gs.H.I_id", "Ds->Es->Gs->H->I"."id" AS "Ds.Es.Gs.H.I.id", "Ds->Es->Gs->H->I"."J_id" AS "Ds.Es.Gs.H.I.J_id", "Ds->Es->Gs->H->I"."createdAt" AS "Ds.Es.Gs.H.I.createdAt", "Ds->Es->Gs->H->I"."updatedAt" AS "Ds.Es.Gs.H.I.updatedAt" FROM (SELECT "C"."id", "C"."createdAt", "C"."updatedAt" FROM "Cs" AS "C" WHERE ( SELECT "Ds"."C_id" FROM "Ds" AS "Ds" INNER JOIN "Es" AS "Es" ON "Ds"."id" = "Es"."D_id" INNER JOIN "Gs" AS "Es->Gs" ON "Es"."id" = "Es->Gs"."E_id" INNER JOIN "Hs" AS "Es->Gs->H" ON "Es->Gs"."H_id" = "Es->Gs->H"."id" INNER JOIN "Is" AS "Es->Gs->H->I" ON "Es->Gs->H"."I_id" = "Es->Gs->H->I"."id" WHERE ("Ds"."C_id" = "C"."id") LIMIT 1 ) IS NOT NULL LIMIT 1) AS "C" INNER JOIN "Ds" AS "Ds" ON "C"."id" = "Ds"."C_id" INNER JOIN "Es" AS "Ds->Es" ON "Ds"."id" = "Ds->Es"."D_id" INNER JOIN "Gs" AS "Ds->Es->Gs" ON "Ds->Es"."id" = "Ds->Es->Gs"."E_id" LEFT OUTER JOIN "Js" AS "Ds->Es->Gs->H->I->J" ON "Ds.Es.Gs.H.I.J_id" = "Ds->Es->Gs->H->I->J"."id" INNER JOIN "Hs" AS "Ds->Es->Gs->H" ON "Ds->Es->Gs"."H_id" = "Ds->Es->Gs->H"."id" INNER JOIN "Is" AS "Ds->Es->Gs->H->I" ON "Ds->Es->Gs->H"."I_id" = "Ds->Es->Gs->H->I"."id";
'use strict';

const Sequelize = require('sequelize');
const Op = Sequelize.Op;

let C, D, E, G, H, I, J;

async function test() {

  const db = new Sequelize('test', 'user', 'user', {
    host: '127.0.0.1',
    dialect: 'postgresql',
    "quoteIdentifiers": true, 
    "underscored": true
  }); 

  C = db.define('C', {  }, {});
  D = db.define('D', { C_id : Sequelize.INTEGER }, {});
  E = db.define('E', { D_id : Sequelize.INTEGER }, {});
  G = db.define('G', { E_id : Sequelize.INTEGER, H_id : Sequelize.INTEGER }, {});
  H = db.define('H', { G_id : Sequelize.INTEGER}, {});
  I = db.define('I', { J_id : Sequelize.INTEGER }, {});
  J = db.define('J', { }, {});

  C.hasMany(D, {foreignKey: 'C_id'});
  D.belongsTo(C, {foreignKey: 'C_id'});

  D.hasMany(E, {foreignKey: 'D_id'});
  E.belongsTo(D, {foreignKey: 'D_id'});

  E.hasMany(G, {foreignKey: 'E_id'});
  G.belongsTo(E, {foreignKey: 'E_id'});

  G.belongsTo(H, {foreignKey: 'H_id'});
  H.hasMany(G, {foreignKey: 'H_id'});

  H.belongsTo(I, {foreignKey: 'I_id'});
  I.hasMany(H, {foreignKey: 'I_id'});

  I.belongsTo(J, {foreignKey: 'J_id'});
  J.hasMany(I, {foreignKey: 'J_id'});


  await db.sync({ force: true });

  await populateDb();

  console.log('Find that does not reproduce the issue')
  const t1 = await C.findOne({
    include:[
    {
      model:D,
      required:true,
      include:[
        {
          model:E,
          required:true,
          include:[
            {
              model:G,
              required:true,
              include:[
                {
                  model:H,
                  required:true,
                  include:[
                    {
                      model:I,
                      required:true,
                      include:[
                        {
                          required:true,
                          model:J
                        }
                      ]
                    }
                  ]
                }
              ]
            }
          ]
        }
      ]
    }
  ]});

  console.log('Find that does reproduce the issue, only diff is the missing required on model J')
  const t2 = await C.findOne({
    include:[
    {
      model:D,
      required:true,
      include:[
        {
          model:E,
          required:true,
          include:[
            {
              model:G,
              required:true,
              include:[
                {
                  model:H,
                  required:true,
                  include:[
                    {
                      model:I,
                      required:true,
                      include:[
                        {
                          model:J
                        }
                      ]
                    }
                  ]
                }
              ]
            }
          ]
        }
      ]
    }
  ]});

  await db.close();
}

async function populateDb() {
  const c = await C.create({ });
  const d = await D.create({ C_id:c.id });
  const e = await E.create({ D_id:d.id });

  const j = await J.create({  });
  const i = await I.create({ J_id:j.id });
  const h = await H.create({ I_id:i.id });

  const g = await G.create({ E_id:e.id, H_id:h.id });

}

async function main() {
  try {
    console.log('Bad sql');
    await test();
  } catch (err) {
    console.error(err);
    process.exit(1);
  }
}

main();

@linpekka
Copy link
Author

linpekka commented Sep 25, 2018

Here is another query i found that does not produce a valid sql, earlier logged as a separate issue #9931.

'use strict';

const Sequelize = require('sequelize');
const Op = Sequelize.Op;

let A, B, C;

async function test() {

  const db = new Sequelize('test', 'user', 'user', {
    host: '127.0.0.1',
    dialect: 'postgresql',
    "quoteIdentifiers": true, 
    "underscored": true
  });

  A = db.define('A', { str: Sequelize.STRING }, {});
  B = db.define('B', { a_id : Sequelize.INTEGER, c_id : Sequelize.INTEGER }, {});
  C = db.define('C', { str: Sequelize.STRING }, {});

  B.belongsTo(A, {foreignKey: 'a_id'});
  A.hasMany(B, {foreignKey: 'a_id'});

  B.belongsTo(C, {foreignKey: 'c_id'});
  C.hasMany(B, {foreignKey: 'c_id'});

  await db.sync({ force: true });

  await populateDb();

  console.log('Find that does not reproduce the issue')
  const t1 = await A.findAll({
    where: {str:'a'}, 
    include:[
    { model: B, required:true, include:{model:C, where:{str:'a'}} }
  ]});

  console.log('Find that reproduces the issue, only limit differs')
  const t2 = await A.findAll({
    where: {str:'a'}, 
    limit: 6,
    include:[
    { model: B, required:true, include:{model:C, where:{str:'a'}} }
  ]});

  await db.close();
}


async function populateDb() {
  const c = await C.create({ str:'a' });
  const a = await A.create({ str:'a' });
  const b = await B.create({ c_id:c.id, a_id:a.id });
}

async function main() {
  try {
    console.log('Bad sql');
    await test();
  } catch (err) {
    console.error(err);
    process.exit(1);
  }
}

main();

What do you expect to happen?
I want the findAll to generate valid sql

What is actually happening?
Got an error - error: missing FROM-clause entry for table "Bs"

Output, either JSON or SQL

SELECT "A".*, "Bs"."id" AS "Bs.id", "Bs"."a_id" AS "Bs.a_id", "Bs"."c_id" AS "Bs.c_id", "Bs"."createdAt" AS "Bs.createdAt", "Bs"."updatedAt" AS "Bs.updatedAt" FROM (SELECT "A"."id", "A"."str", "A"."createdAt", "A"."updatedAt", "Bs->C"."id" AS "Bs.C.id", "Bs->C"."str" AS "Bs.C.str", "Bs->C"."createdAt" AS "Bs.C.createdAt", "Bs->C"."updatedAt" AS "Bs.C.updatedAt" FROM "As" AS "A" INNER JOIN "Cs" AS "Bs->C" ON "Bs"."c_id" = "Bs->C"."id" AND "Bs->C"."str" = 'a' WHERE "A"."str" = 'a' AND ( SELECT "Bs"."a_id" FROM "Bs" AS "Bs" INNER JOIN "Cs" AS "C" ON "Bs"."c_id" = "C"."id" AND "C"."str" = 'a' WHERE ("Bs"."a_id" = "A"."id") LIMIT 1 ) IS NOT NULL LIMIT 6) AS "A" INNER JOIN "Bs" AS "Bs" ON "A"."id" = "Bs"."a_id";

@1valdis
Copy link

1valdis commented Oct 29, 2018

By the way, subQuery: false fixes SQL, but breaks limit/offset. The only option I found so far to make them work together is to make a custom subquery using QueryGenerator.

@adrienboulle
Copy link
Contributor

adrienboulle commented Nov 21, 2018

@sushantdhiman your correction does not work if one level of include is not required

example :

this._m1.findAll({
  include: [
    {
      model: this._m2,
      required: false,
      include: [
        {
          model: this._m3,
          required: true,
          include: [
            {
              model: this._m4,
              required: true,
              include: [
                {
                  model: this._m5,
                  required: false,
                },
              ],
            },
          ],
        },
      ],
    },
  ],
})

@justraman
Copy link

@HevertonPires did you solved the problem? if yes? how.

@HevertonPires
Copy link

HevertonPires commented Dec 9, 2020

@justraman It was not solved!

@Alejopdp
Copy link

Alejopdp commented Jan 5, 2021

Hi there,

Did anyone achieve to solve this? I'm using v6.3.4.

Using separated: true didn't work in my case

Thanks.

@l2ysho
Copy link
Contributor

l2ysho commented Jan 6, 2021

Hi there,

Did anyone achieve to solve this? I'm using v6.3.4.

Using separated: true didn't work in my case

Thanks.

only workaround that worked for me was raw querry

@boobo94
Copy link

boobo94 commented Jan 15, 2021

Hi there,

Did anyone achieve to solve this? I'm using v6.3.4.

Using separated: true didn't work in my case

Thanks.

Raw query for the moment.

@goshander
Copy link

Same issue (((

@Ranguna
Copy link

Ranguna commented Jan 22, 2021

This effectively makes it impossible to make any sort of non trivial queries.

Was there any progress on this on the last two years ?

@l2ysho
Copy link
Contributor

l2ysho commented Jan 23, 2021

This effectively makes it impossible to make any sort of non trivial queries.

Was there any progress on this on the last two years ?

nope

@justraman
Copy link

What i have learnt from sequelize is that don't use an ORM which does not support query builder.
I'm using knex.js it took time to understand but I'm happy with it.
You can also look for Objection.js atleast it gives you a query builder in case like this.

@abhi5658
Copy link
Contributor

You can use separate: true if you can convert your M : N relation to M : 1 : 1 : N
What I mean is... Lets say you have 3 tables -

- Model1
- Model2
- Model1_Model2_Junction 

You can define a association that is a “Super Many-to-Many relationship” (Read here: https://sequelize.org/master/manual/advanced-many-to-many.html - Section: Through tables versus normal tables and the "Super Many-to-Many association" )
It works as below:

// You might already be having belongsToMany - that is fine - keep it as is
Model1.hasMany( Model1_Model2_Junction, {...} ); // <-- this is the magic
Model1_Model2_Junction.belongsTo( Model2, {...} ); // I hope this is already present

Model1.findAll({
  subQuery: false, // this helps not to get a sub query which tries to save our pagination/limit+offset
  // try not to use a where condition that involves columns from included Models 
  // ( e.g. dont use '$Model2.id$ : 5 - something like this )
  limit: 10,
  offset: 10,
  include: [{
    separate: true, // now this is hasMany! Yeah!
    model: Model1_Model2_Junction, 
      include: [{
        model: Model2,
      }],
  }],
});

The result that you will obtain will be having 1 level extra junction table's details inside which you will have your Model2 data. Just reformat it to first level by mapping/iterating through it. And you are done 🚀

Note : 💡 Whichever M:N association you include while also using limit+offset, convert all of them to Super Many-to-Many and add separate: true in that findAll.

@Imfdj
Copy link

Imfdj commented Apr 12, 2021

Again, subQuery: false doesn't fix the problem - while the generated query does work, limit and offset are broken (ie. returning less results than there are, so in 100 correct results, 10 offset, 10 limit, there may - and in fact most of the time will be - less than the expected 10 results).

There is no workaround, apart from creating a custom query.
Please, this is a major issue!

In version 6.6.2, this issue remains unsolved!!!

@TMFLGR
Copy link

TMFLGR commented Apr 24, 2021

Same Issue:

"pg": "^8.5.1",
"pg-hstore": "^2.3.3",
"sequelize": "^6.6.2",
  • postgres v12.4

@droplessdeclan

This comment was marked as abuse.

@justraman
Copy link

justraman commented Oct 20, 2021 via email

@RavenXce

This comment was marked as abuse.

@l2ysho
Copy link
Contributor

l2ysho commented Oct 27, 2021

Really useless ORM, can't support basic things. Waste of time.

Actually it is pretty good, I am waiting for resolving my PR only 2 weeks. 👍

@alexgbor
Copy link

alexgbor commented Mar 3, 2022

Any update on this issue?

@ephys ephys unassigned papb Mar 4, 2022
@ephys ephys added P1: important For issues and PRs. and removed status: awaiting investigation labels Mar 4, 2022
@ephys
Copy link
Member

ephys commented Mar 4, 2022

I can confirm that unfortunately the issue is still present in latest Sequelize (6.17).
My only advice right now if you're encountering this issue is to use a raw query.

We're a bit short on maintainers and the ones that are active already have their hands very full so I can't promise when a fix will be made available.

If someone is willing to open a PR that includes tests & a fix, we'll gladly merge it!

@collin-pham

This comment was marked as spam.

@matsafiks
Copy link

in my case separate: true is working for me inside include object. separate: true will build another query for relations. so that limit and offsetwill excellently work for the parent model.

If separate: true, runs a separate query to fetch the associated instances, only supported for hasMany associations

Example-

db.Users.findAndCountAll({
    distinct: true,
    limit,
    offset,
    include:[
        {
            separate: true, //<- magic
            model: Friends
        },
        {
            separate: true, //<- magic
            model: School,
            include: [
                {
                    model: Class
                }
            ]
        },
    ]
})

it work for me

@higuitamartinez
Copy link

higuitamartinez commented Jul 9, 2022

For doing a where with some field(s) from the models n:m with separate: true and limit, a way is the following:

//Here get the user_id according to where
const usersId = await Users.findAll({
    attributes: ['user_id'],
    include:[
        {
            model: Friends,
            attributes: []
        },
        {
            model: School,
            attributes: [],
            include: {
                model: Class,
                attributes: []
            }
        }
    ],
    where: {
        //If we use this with separate: true or limit is a error 'Unknown Column'
        '$school.class.class_id$': { //This names can see in the server console
            [Op.or]: [class_id_1, class_id_2]
        },
        '$friends.friend_id$': friend_id
    }
});

const arrayId = [];
usersId.forEach(user => {
    arrayId.push(user.user_id)
});

//Here get the data with limit and offset
const users = await Users.findAndCountAll({
    include:[
        {
            model: Friends,
            separate: true, //For all m:n
        },
        {
            model: School,
            include:{
                model: Class
            }
        },
    ],
    distinct: true,
    limit,
    offset,
    where: {
        user_id: {
            [Op.in]: arrayId //Get data with WHERE IN
        }
    }
});

@jkuri
Copy link

jkuri commented Oct 4, 2022

I also solved this as higuitamartinez suggested, adding a separate query above and then include array of ids as [Op.in] statement in where condition. That is basically something that sequelize should do automatically imho.

@HyopeR
Copy link

HyopeR commented Oct 26, 2022

This problem still persists. Limit and offset should be supported as a basis. I think pagination should be applied to the result after eager loading has taken place.

@ephys
Copy link
Member

ephys commented Oct 26, 2022

I'm working on it but it's going to be a little while before it's ready. I'm redesigning findAll from the ground up to make sure all relevant options interact well with each-other (this is the current draft).

@l2ysho
Copy link
Contributor

l2ysho commented Nov 3, 2022

I'm working on it but it's going to be a little while before it's ready. I'm redesigning findAll from the ground up to make sure all relevant options interact well with each-other (this is the current draft).

well It looks really promising 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1: important For issues and PRs. regression For issues and PRs. A bug that does not happen in an earlier version of Sequelize. topic: limit & offset For issues and PRs. Things that involve queries with limit and/or offset. type: bug
Projects
Development

No branches or pull requests