When using the database as backend, the getFacets function in Database.php returns the wrong facet results.

When having more then 1 facet-group, only the facetgroups later in the query conditiongroup will be limited.

The actual result are correct, but the facet items and facet-count does not get updated.
The order in the conditiongroup is relevant for this issue to reproduce.

I also tested solr backend (and core search, which Facets also supports), but those do not have the issue.
Solr backend uses the same search api connector, so I suppose we do send the correct query.

Obviously, this breaks Facets.

CommentFileSizeAuthor
#5 2809753-5--fix_query_clone.patch3.04 KBdrunken monkey
#5 2809753-5--fix_query_clone--tests_only.patch2.11 KBdrunken monkey
#4 search_api_db_facetissue.gif103.24 KBstrykaizer

Comments

StryKaizer created an issue. See original summary.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Issue summary: View changes
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Priority: Critical » Major
Status: Active » Postponed (maintainer needs more info)

Sorry, but I don't know what you're trying to say. Could you try to explain again what is happening, or how to reproduce the error – or, probably best, maybe provide a patch adding a failing test for this? From your description, I can't really tell what the problem is.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Status: Postponed (maintainer needs more info) » Needs work
StatusFileSize
new search_api_db_facetissue.gif103.24 KB

I created a gif to show the issue.

When filtering on Type, COlor works fine and gets limitted.
When filtering on Color, Type does not get limitted as expected.

This only occurs in search_api_db (solr and drupal core search work fine)

demo issue

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new 2809753-5--fix_query_clone--tests_only.patch2.11 KB
new 2809753-5--fix_query_clone.patch3.04 KB

OK, managed to reproduce it, write a test and finally find and fix the problem. While we took care to clone the query when unsetting conditions for OR facets, the query object's __clone() method was incomplete – we apparently forgot to implement it until #2641396: Add a way to add warnings or ignored keys right on the query, and then didn't include all the necessary object-valued properties.
Patch attached, please test/review!

Oh, and just btw: As long as there's no patch, "Active" would have been the right status. NR, NW and RTBC only apply if there's a patch to review, work on or commit.

The last submitted patch, 5: 2809753-5--fix_query_clone--tests_only.patch, failed testing.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I think this change looks good, but let's have @StryKaizer confirm that this actually works. I will try to verify manually as well.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Status: Needs review » Reviewed & tested by the community

Tested and works as expected.
Thanks!!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

OK, great to hear. Thanks a lot for testing and reviewing!
Committed.
Thanks again!

  • drunken monkey committed 40556bc on 8.x-1.x 
    Issue #2809753 by drunken monkey: Fixed issues with multiple OR facets.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.