orm icon indicating copy to clipboard operation
orm copied to clipboard

Duplicates in some selections with GROUP+HAVING

Open stepapo opened this issue 1 year ago • 6 comments

Describe the bug

Sometimes ORM adds columns to GROUP statement, which leads to retrieving duplicate rows.

To Reproduce

Looking for books with any of Tags ID 2 or 3, which at the same time have more than one tag. Should be two books, but results in three, including one duplicate.

$books = $this->orm->books->findBy([
    ICollection::AND,
    [CompareGreaterThanFunction::class, [CountAggregateFunction::class, 'tags->id'], 1],
    ['tags->id' => [2, 3]],
]);
Assert::same($books->count(), 2);
SELECT "books".* 
FROM "books" AS "books" 
LEFT JOIN "books_x_tags" AS "books_x_tags__COUNT" ON ("books"."id" = "books_x_tags__COUNT"."book_id") 
LEFT JOIN "tags" AS "tags__COUNT" ON ("books_x_tags__COUNT"."tag_id" = "tags__COUNT"."id") 
LEFT JOIN "books_x_tags" AS "books_x_tags_any" ON ("books"."id" = "books_x_tags_any"."book_id") 
LEFT JOIN "tags" AS "tags_any" ON ("books_x_tags_any"."tag_id" = "tags_any"."id") 
GROUP BY "books"."id", "tags_any"."id" 
HAVING ((COUNT("tags__COUNT"."id") > 1) AND (("tags_any"."id" IN (2, 3))))

Expected behavior

Adding "tags_any"."id" IN (2, 3) to HAVING requires column "tags_any"."id" to be added in GROUP, which leads to duplicates in selection. In MySQL, the column could be moved from GROUP to SELECT, but in Postgres, that does not do the trick. In both it would be probably ok to SELECT DISTINCT, or to remove column from GROUP and move condition to WHERE (as it was in 4.0 version).

Versions

  • Database: PostgreSQL 13.13.0
  • Orm: dev-main
  • Dbal: 5.0.0-rc4

stepapo avatar Apr 10 '24 19:04 stepapo

Thank you for another great report. I hope this is the last one, not because you would stop, but because it is that way :D

However, this time the fix... is complicated.

hrach avatar Apr 10 '24 19:04 hrach

Undestood. I also think it's the last one. I found only one more and it was already in v4.0. Now it's fixed, sort of, by NotSupportedException "Relationship cannot be fetched...". I'm worried that the same is going to happen to this one too :-D

stepapo avatar Apr 10 '24 21:04 stepapo

Hm, MySQL is not working correctly (#667) with DISTINCT. Sadly, I cannot move it from having to where, because it would stop working with OR.

hrach avatar Apr 11 '24 13:04 hrach

I cannot move it from having to where, because it would stop working with OR.

Seems to me that OR is already solved by including condition "tags_any"."id" IN (2, 3) in LEFT JOIN ... ON. Works well. Maybe the same solution could be used for AND?

$books = $this->orm->books->findBy([
    ICollection::OR,
    [CompareGreaterThanFunction::class, [CountAggregateFunction::class, 'tags->id'], 1],
    ['tags->id' => [2, 3]],
])->fetchAll();
SELECT "books".* 
FROM "books" AS "books" 
LEFT JOIN "books_x_tags" AS "books_x_tags__COUNT" ON ("books"."id" = "books_x_tags__COUNT"."book_id") 
LEFT JOIN "tags" AS "tags__COUNT" ON ("books_x_tags__COUNT"."tag_id" = "tags__COUNT"."id") 
LEFT JOIN "books_x_tags" AS "books_x_tags_any" ON ("books"."id" = "books_x_tags_any"."book_id")
LEFT JOIN "tags" AS "tags_any" ON (
    ("books_x_tags_any"."tag_id" = "tags_any"."id") 
    AND "tags_any"."id" IN (2, 3)
) 
GROUP BY "books"."id" 
HAVING ((COUNT("tags__COUNT"."id") > 1) OR ((COUNT("tags_any"."id") > 0)))

stepapo avatar Apr 12 '24 11:04 stepapo

Just for reference, I tried simply commenting out this line and both OR and AND cases now work as well as all other tests in MySQL and Postgres. However, it brings down performance significantly for simple AND filters that use AnyAggregator.

I figured that current DbalExpressionResult probably does not allow combining both WHERE and HAVING conditions within a single query. Without that, it might be hard to achieve correct results and optimal performance at the same time. Understandably, complex filters need to be all in one or the other.

It is still a great job making most of this work correctly and this case is not a "must have" for me.

stepapo avatar Apr 15 '24 14:04 stepapo

Thanks for exploring, yes, I had a few ideas but didn't have a chance to explore them. I hope I'll get to it at the end of this week :)

hrach avatar Apr 16 '24 07:04 hrach