Loopback-datasource-juggler: Find + relation inclusion with a limit is broken

[seriousben]

Since 2.8.0, this is broken.

The query that used to work:

Post.find({
include: [{
  relation: 'likes',
  scope: {
    where: {userId: currentUserId},
    include: [{
      'owner': 'avatar'
    }],
    limit: 1
  }
}],
limit: 10,
order: 'createdAt DESC'
}

The problem is that the limit gets put on the merged Relation query like so:

QUERY:  Like {
 "userId": "55548654af697c1d7073412d",
 "postId": {
  "$in": [
   "556847b15a0fc6014959edad",
   "556847b15a0fc6014959edac",
   "556847b05a0fc6014959edab",
   "556847b05a0fc6014959edaa",
   "556847b05a0fc6014959eda9",
   "556847b05a0fc6014959eda8",
   "556847b05a0fc6014959eda7",
   "556847b05a0fc6014959eda6",
   "556847b05a0fc6014959eda5",
   "556847b05a0fc6014959eda4",
   "556847b05a0fc6014959eda3",
   "556847b05a0fc6014959eda2",
   "556847b05a0fc6014959eda1",
   "556847b05a0fc6014959eda0",
   "556847b05a0fc6014959ed9f",
   "556847b05a0fc6014959ed9e",
   "556847b05a0fc6014959ed9d",
   "556847af5a0fc6014959ed9c",
   "556847af5a0fc6014959ed9b",
   "556847af5a0fc6014959ed98",
   "556847af5a0fc6014959ed9a",
   "556847af5a0fc6014959ed97",
   "556847af5a0fc6014959ed96",
   "556847af5a0fc6014959ed94",
   "556847af5a0fc6014959ed95",
   "556839a25a0fc6014959ed5c",
   "556839a15a0fc6014959ed5a",
   "556839a15a0fc6014959ed5b"
  ]
 }
}
====>
cursor.limit(1);
<====

[seriousben]

/cc @raymondfeng - Didnt have time to create a test for you, but this should provide you with enough information for now.

[raymondfeng]

1. There is a typo: orer: 'createdAt DESC' should be order: 'createdAt DESC'
2. I assume you want to find most recent 10 posts including at most 1 like which also includes the owner. What's wrong with the limit(1)?

[seriousben]

Thanks @raymondfeng - Fixed the typo. Just me adding it manually to this issue.

In this example, I want to include the likes of a user limiting it to 1 like _per_ post. Right now the optimizations are making it do 1 like for all posts.

[raymondfeng]

What's the mismatch between the code behavior and your expectation?

[seriousben]

The mismatch is that I expect one Like per Post and not 1 Like for all Posts.

Used to return:

posts: [{/*postData*/, likes: [/*at most one like*/]}, {postData, likes: [/*at most one like*/]}, {postData, likes: [/*at most one like*/]}]

Now returns:

posts: [{/*postData*/, likes: [/*at most one like*/]}, {postData, likes: [/*no likes*/]} {postData, likes: [/*no likes*/]}]

The limit seems to be limiting the whole query now and is not by post.

(Do you prefer that I mention you? @raymondfeng or not?)

[ernie58]

I'm having this issue too.
It worked with these packages:
"loopback": "^2.8.0",
"loopback-boot": "^2.4.0",
"loopback-connector-mysql": "^1.6.0",
"loopback-datasource-juggler": "^2.21.0",

Yesterday I ran an update, and it broke:
"loopback": "^2.18.0",
"loopback-boot": "^2.8.0",
"loopback-connector-mysql": "^1.6.0",
"loopback-datasource-juggler": "^2.29.2",

[mrfelton]

Also hitting upon this issue. applying a limit to a relationship scope limits the overall number of related items returned instead of the number of related items per parent item.

[superkhau]

Can someone here provide a link to a test project on Github? See https://github.com/strongloop/loopback/wiki/Issues#bug-report

[zanemcca]

@superkhau I modified include.test.js to test for this scenario.
https://github.com/zanemcca/loopback-datasource-juggler

[zanemcca]

I think this commit caused the bug.

I like the idea of doing only two DB requests to get the limited included relations and their parents but I'm not aware of a way of doing that without loading all included relations and then limiting from the application code which would defeat the purpose of the limit.

I think the best bet might be to default back to the old way of including relations when they have a limit on them and using the optimized version when they do not. Thoughts?

[zanemcca]

Hey @superkhau have you had a chance to look at this yet?
What do you think think of my suggestion in my last post?

[superkhau]

I haven't had a chance to look into this yet, will try to get to it this sprint. I'm not sure about the suggestion, but @raymondfeng can give you some feedback here.

@raymondfeng Please advise on the suggestion by @zanemcca ^

[raymondfeng]

If the an included model has pagination clause (limit/offset), we cannot use the inq operator to find all child instances in one query. I think we have to query the DB per foreign key independently in such cases unless the underlying DB has the ability to paginate by groups.

@ningsuhen What's your take?

[ningsuhen]

Apart from this, there are few cases where the old way is required. What about we add an option to choose the old method when required? say a flag "optimizeQueries" (or singleQuery or any suggestions) which defaults to true.

Post.find({
include: [{
  relation: 'likes',
  scope: {
    where: {userId: currentUserId},
    include: [{
      'owner': 'avatar'
    }],
    limit: 1
  },
  optimizeQueries: false
}],
limit: 10,
order: 'createdAt DESC'
}

[zanemcca]

I think optimizeQueries or singleQuery do not accurately reflect that the results will be different.

[zanemcca]

Maybe consolidateQueries ?

[raymondfeng]

@ningsuhen We can introspect the filter object to see if limit/offset/skip is required and decide which function to call. I don't see the need to introduce a new flag.

[ningsuhen]

@raymondfeng , I'm just concerned that when a developer sets the limit/offset/skip, it can unintentionally change the performance drastically. With the flag, the developer will be aware of the performance considerations and can only opt for it when it suits (even I think that flag is not a neat solution though).

Additionally, there can also be a case where we might want the current implementation with limits. Say, I want to paginate all the items/posts within few selected categories, it can be done with current implementation. (Or let's say, to sort all included items and limit to 10 results).

[ulion]

Also hit by this bug, is this resolved currently?

[raymondfeng]

@ningsuhen I think we should probably introduce a pageSize for the inq for two purposes:

1. If limit/skip/offset is present in the include, pageSize should be set to 1. We need to run the query for FKs one by one
2. We should have a maximum pageSize as many DBs have a limit of number of items in inq. The query by FKs should be run page by page and join the results afterward.

Do you have time to take a slab?

[raymondfeng]

Can you test out https://github.com/strongloop/loopback-datasource-juggler/commit/fa580ed098a12d9aff576cd67ca2c40d6f4ea59a?

[superkhau]

I've merged in the fixes. Please let me know if works for you guys! ;)

[cspeer]

I'm also struggling with this. I'm running on:

"loopback": "2.27.0",
"loopback-connector-mongodb": "^1.15.2",
"loopback-datasource-juggler": "2.46.0",

Is there a roadmap as to when this fix will be released?

[rlloyd2001]

This is not working for me. I'm using MySQL and the limit on an include isn't working right. tested with loopback-datasource-juggler: 2.47.0

[superkhau]

@rlloyd2001 Can you open a new PR with a sample repo to reproduce your situation? We don't usually respond to closed issues.

[rlloyd2001]

@superkhau I'll add it to my todo list, but I don't know when I'll be able to make an example. Solving using a remote method with two loopback finds on the backend for now.

[hopduong]

Hi, @superkhau I know this issue already closed but I met this issue on my project now. Could you guys please help to solve:

"loopback": "2.36.0",
"loopback-connector-mongodb": "^3.9.2",
"loopback-datasource-juggler": "3.1.1",

This project is still using old versions but based on the comments above, this issue should be fixed.
Query include example:

// include a comment for each post
{
                relation: 'comments',
                scope: {
                    where: {
                        disabled: {
                            neq: true
                        }
                    },
                    order: 'likeCount DESC, id DESC',
                    limit: 1
                }
            }