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

LoadStrategy.JOINED with pagination/populateWhere weird behavior #3871

Closed
Tronikelis opened this issue Dec 24, 2022 · 8 comments · Fixed by #4682
Closed

LoadStrategy.JOINED with pagination/populateWhere weird behavior #3871

Tronikelis opened this issue Dec 24, 2022 · 8 comments · Fixed by #4682
Labels
bug Something isn't working
Milestone

Comments

@Tronikelis
Copy link

Tronikelis commented Dec 24, 2022

Describe the bug
It seems that when using the JOINED strategy the populateWhere gets ignored in queries without pagination. But when you try to paginate with the JOINED strategy weird results are returned, more detailed info in the repo that I created.

Everything works as expected with the SELECT_IN strategy

To Reproduce
I have created a repo https://github.com/Tronikelis/mikro-orm-issue-1

import { LoadStrategy, Reference } from "@mikro-orm/core";

import { orm, Action, Pet, User } from "./config";

(async () => {
    const { em: _em, migrator } = await Promise.resolve(orm);

    const em = _em.fork();

    // 10 users
    for (let i = 0; i < 10; i++) {
        const user = new User();

        // 10 pets
        for (let i = 0; i < 10; i++) {
            const pet = new Pet("name - " + Math.random().toString());
            pet.User = Reference.create(user);

            // 10 actions
            for (let i = 0; i < 10; i++) {
                const action = new Action("name - " + Math.random().toString());
                pet.Action = Reference.create(action);
                em.persist(action);
            }

            em.persist(pet);
        }

        em.persist(user);
    }

    await em.flush();
    em.clear();

    // 1 - simple working query (em.find)
    const simpleFind = await em.find(
        User,
        {},
        {
            strategy: LoadStrategy.JOINED, // does not matter which strategy
            populate: ["Pets"],
            limit: 2, // with or without, works
        }
    );

    console.log(JSON.parse(JSON.stringify({ simpleFind })));

    em.clear();

    // 2 - populateWhere find JOINED
    const populateWhereJoined = await em.find(
        User,
        {},
        {
            populate: ["Pets"],
            populateWhere: {
                Pets: {
                    name: {
                        // should populate all pets ?
                        $like: "name%",
                    },
                },
            },
            strategy: LoadStrategy.JOINED,
        }
    );

    // this ignores the populateWhere filter as it seems
    // select ... from "user" as "u0" left join "pet" as "p1" on "u0"."id" = "p1"."user_id"

    console.log(JSON.parse(JSON.stringify({ populateWhereJoined })));

    em.clear();

    // 3 - populateWhere find SELECT_IN
    const populateWhereSelectIn = await em.find(
        User,
        {},
        {
            populate: ["Pets"],
            populateWhere: {
                Pets: {
                    name: "yoyo",
                },
            },
            strategy: LoadStrategy.SELECT_IN,
        }
    );

    // [query] select "u0".* from "user" as "u0" [took 1 ms]
    // [query] select "p0".* from "pet" as "p0" where "p0"."user_id" in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30) and "p0"."name" = 'yoyo' order by "p0"."user_id" asc

    // this works as intended, most users have 0 pets, while some have 1 with the name "yoyo"
    console.log(JSON.parse(JSON.stringify({ populateWhereSelectIn })));

    em.clear();

    // 4 - populateWhere pagination JOINED
    const [paginationJoined] = await em.findAndCount(
        User,
        {},
        {
            strategy: LoadStrategy.JOINED,
            populate: ["Pets"],
            populateWhere: {
                Pets: {
                    name: "yoyo",
                },
            },
            limit: 30, // will test with and without below
        }
    );

    // with limit: 30

    // [query] select count(*) as "count" from "user" as "u0" [took 1 ms]
    // [query] select "u0"."id", "p1"."id" as "p1__id", "p1"."name" as "p1__name", "p1"."user_id" as "p1__user_id", "p1"."action_id" as "p1__action_id" from "user" as "u0" left join "pet" as "p1" on "u0"."id" = "p1"."user_id" where "u0"."id" in (select "u0"."id" from (select "u0"."id" from "user" as "u0" left join "pet" as "p1" on "u0"."id" = "p1"."user_id" group by "u0"."id" limit 2) as "u0") and "p1"."name" = 'yoyo' [took 2 ms]

    // the weird thing is that here I can see that the query is trying to fetch the pets with the name "yoyo"
    // but in the second (2 - populateWhere find JOINED) example, it did not even try

    // weird, it fetched only one record where the populateWhere filter was found
    console.log(JSON.parse(JSON.stringify({ paginationJoined }))); // { paginationJoined: [ { id: 12, Pets: [Array] } ] }

    // without limit: 30

    // the results and query is the same the second query, (2 - populateWhere find JOINED)

    em.clear();

    // 5 - populateWhere pagination SELECT_IN
    const [paginationSelectIn] = await em.findAndCount(
        User,
        {},
        {
            strategy: LoadStrategy.SELECT_IN,
            populate: ["Pets"],
            populateWhere: {
                Pets: {
                    name: "yoyo",
                },
            },
            limit: 30,
        }
    );

    // everything works as expected here
    console.log(JSON.parse(JSON.stringify({ paginationSelectIn })));

    // conclusion
    // no problems with the SELECT_IN (at least from my testing)

    // JOINED in the second example seems to ignore populateWhere, expected behavior?
    // but if it is expected, then why does populateWhere kinda work when using limit (4 example) ?

    // shouldn't the resulting users and their pets be the same no matter which LoadStrategy you use
})();

Expected behavior
I would expect that there would be no difference between these 2 strategies: JOINED/SELECT_IN

Versions

Dependency Version
node 16.19.0
typescript 4.9.4
mikro-orm 5.6.1
your-driver postgres

Happy holidays btw 👍

@Tronikelis
Copy link
Author

Maybe related to #2985 ?

@B4nan
Copy link
Member

B4nan commented Dec 25, 2022

Both the populateWhere and pagination were developed mainly for the select in strategy, IIRC the joined strategy implied usage of populateWhere: 'infer' - you can't really have different conditions for the relations and the root entity with joined strategy. Will need to review the repro as well as backtrack some of the old issues to be sure. If there are some invalid combinations in the find options, we should throw to let the user know the result would be weird anyway... Or maybe automatically switch to select-in strategy in such case.

In any case, if you can propose ways for how the queries can work for the weird cases, it would help a lot. With select-in strategy its quite easy to reason about those things, but for the joined strategy, I am often not sure how to do it.

@Tronikelis
Copy link
Author

Tronikelis commented Dec 25, 2022

you can't really have different conditions for the relations and the root entity with joined strategy

Well, correct me if I'm wrong, but in postgres (idk about other drivers) you can left join with an 'and' operator like this:

select * from "user"
left join "pet" on "pet"."user_id" = "pet"."id" and "pet"."name" = 'y'

notice the and ... = 'y', this should work like a populateWhere

bare-bones query:

select * from "user"
left join "pet" on "pet"."user_id" = "pet"."id"

All the pets are populated as they should be

the query with left join ... on ... and:

select * from "user"
left join "pet" on "pet"."user_id" = "pet"."id"
and "pet"."name" = '1931dasda'

None of the pets are populated etc..

@B4nan
Copy link
Member

B4nan commented Dec 25, 2022

Well, I believe with pagination it's much more complicated, but I haven't really checked the exact repro yet.

Btw I would appreciate less screenshots and more code examples, this is quite a mess :]

@Tronikelis
Copy link
Author

Tronikelis commented Dec 25, 2022

Well, I believe with pagination it's much more complicated, but I haven't really checked the exact repro yet.

Yeah, pagination with joins is tricky

But let's take one example query generated by mikro orm

select
  "u0"."id",
  "p1"."id" as "p1__id",
  "p1"."name" as "p1__name",
  "p1"."user_id" as "p1__user_id",
  "p1"."action_id" as "p1__action_id"
from
  "user" as "u0"
  left join "pet" as "p1" on "u0"."id" = "p1"."user_id"
where
  "u0"."id" in (
    select
      "u0"."id"
    from
      (
        select
          "u0"."id"
        from
          "user" as "u0"
          left join "pet" as "p1" on "u0"."id" = "p1"."user_id"
        group by
          "u0"."id"
        limit
          30
      ) as "u0"
  )
  and "p1"."name" = 'yoyo'

I took this from my repro 4 example

It returns only 1 record, user with the pet named yoyo

Now if I move the and "p1"."name" = 'yoyo' before the where
Like this:

select
  "u0"."id",
  "p1"."id" as "p1__id",
  "p1"."name" as "p1__name",
  "p1"."user_id" as "p1__user_id",
  "p1"."action_id" as "p1__action_id"
from
  "user" as "u0"
  left join "pet" as "p1" on "u0"."id" = "p1"."user_id"
  and "p1"."name" = 'yoyo'
where
  "u0"."id" in (
    select
      "u0"."id"
    from
      (
        select
          "u0"."id"
        from
          "user" as "u0"
          left join "pet" as "p1" on "u0"."id" = "p1"."user_id"
        group by
          "u0"."id"
        limit
          30
      ) as "u0"
  )

It returns 30 users, and only 1 has the pets populated, this should be the expected behavior? (same as SELECT_IN)

@B4nan
Copy link
Member

B4nan commented Dec 25, 2022

Yes, the select-in behavior is indeed the expected one.

@B4nan B4nan added the bug Something isn't working label Dec 28, 2022
@B4nan B4nan added this to the 6.0 milestone Feb 6, 2023
B4nan added a commit that referenced this issue Feb 8, 2023
B4nan added a commit that referenced this issue Feb 8, 2023
@B4nan
Copy link
Member

B4nan commented Feb 8, 2023

Implemented in v6.

@B4nan B4nan closed this as completed Feb 8, 2023
B4nan added a commit that referenced this issue Feb 12, 2023
B4nan added a commit that referenced this issue Feb 17, 2023
B4nan added a commit that referenced this issue Feb 17, 2023
B4nan added a commit that referenced this issue Feb 26, 2023
B4nan added a commit that referenced this issue Mar 19, 2023
B4nan added a commit that referenced this issue Apr 6, 2023
B4nan added a commit that referenced this issue Apr 10, 2023
B4nan added a commit that referenced this issue Apr 12, 2023
B4nan added a commit that referenced this issue Apr 26, 2023
jsprw pushed a commit to jsprw/mikro-orm-full-text-operators that referenced this issue May 7, 2023
jsprw pushed a commit to jsprw/mikro-orm-full-text-operators that referenced this issue May 7, 2023
B4nan added a commit that referenced this issue May 14, 2023
B4nan added a commit that referenced this issue May 14, 2023
B4nan added a commit that referenced this issue May 24, 2023
B4nan added a commit that referenced this issue May 26, 2023
B4nan added a commit that referenced this issue Jun 11, 2023
@B4nan
Copy link
Member

B4nan commented Sep 9, 2023

FYI I decided to port this to the 5.x branch and include it in the next release, let's hope it won't be seen as breaking, but since it aligns with how the loading strategies work, it feels more like a fix. I will also try to narrow how the filters work (#704, #2440).

B4nan added a commit that referenced this issue Sep 9, 2023
Closes #3871

Signed-off-by: Martin Adámek <banan23@gmail.com>
B4nan added a commit that referenced this issue Sep 9, 2023
When using joined strategy, the `populateWhere` condition is now applied
to the joins via `on` conditions instead of using a `where` condition.
This aligns how the two loading strategies work, as the `populateWhere`
will affect only what gets joined, but not the root entity query.

Closes #3871

Signed-off-by: Martin Adámek <banan23@gmail.com>
B4nan added a commit that referenced this issue Sep 10, 2023
B4nan added a commit that referenced this issue Sep 20, 2023
B4nan added a commit that referenced this issue Sep 24, 2023
B4nan added a commit that referenced this issue Sep 30, 2023
B4nan added a commit that referenced this issue Oct 2, 2023
B4nan added a commit that referenced this issue Oct 17, 2023
B4nan added a commit that referenced this issue Oct 21, 2023
B4nan added a commit that referenced this issue Oct 25, 2023
B4nan added a commit that referenced this issue Nov 2, 2023
B4nan added a commit that referenced this issue Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants