- Issue created by @tgoeg
- 🇩🇪Germany hexabinaer Berlin, Germany
Thanks for noting this! If nothing less it helped me understanding what my current problem is.
- First commit to issue fork.
- Merge request !52Draft: Issue #3444466: Using facets on numerical fields makes them disappear → (Open) created by bcizej
- Status changed to Needs work
about 1 year ago 8:55pm 7 June 2024 - 🇸🇮Slovenia bcizej
@tgoeg Thanks for reporting this.
I have added a draft MR that could be tested. The idea behind the solution/workaround is before indexing items, go through all the fields and for any numeric types add a duplicated field to the index with
__facet__
prefix and a value converted to string. Then when generating facets we go through each one and check if any of them are numeric type - if they are run the facet query on meilisearch with prefixed__facet__
instead of the original one. At the end move those__facet__
results to the proper/original facet.Patch can be tested but it is nowhere ready for merging or production, it is in mvp state and there are issues with
filterableAttributes
re-setting sometimes, losing the__facet__
prefixed attributes etc.Can be tested by applying the patch, having a numeric field in the index and reindexing the entire index.
- 🇦🇹Austria tgoeg
Hi and thanks for your quick patch!
It will take some time to check this, as I am currently buried in work with no end in sight, can @hexabinaer probably jump in for the time being?
I think the algorithm is legit and most likely a better approach as it keeps the numeric fields in their correct form and allows sorting, etc. and only converts to strings for the purpose of facetting, which I think should be OK for values with different digits.
This should definitely be tested, as I also had to workaround this problem recently as I had a numeric field indexed as string for faceting to work properly, but also wanted to use it to sort results. That didn't work out properly, as it was0<value<100
and so the single digit values were sorted in the wrong way, i.e. alphanumerically,9 > 80
, which is not what visitors expect.
As you only add a numeric field for faceting, this should be cleaner.When looking at the patch, this line jumped out:
- $facetResults = $this->getFacetData($query->getIndex()->id(), array_keys($facets), [$filters], $keys); + $facetResults = $this->getFacetData($index, array_keys($facets), [$filters], $keys);
In all the other locations
$index=$query->getIndex()
but not
$index=$query->getIndex()->id()
You know I am no real dev so this might be totally OK, just wanted to mention it.
- First commit to issue fork.
- 🇸🇮Slovenia deaom
Tested manually, (be aware if using search api with Drupal 11.2 as there is an issue when creating a new view) by adding two fields, first one an integer and then also a decimal to my content. Added facets for both fields, first only for integer then added the float one. The facets were present, the search did work, sorting by int field also worked correctly.
Updated the facet test to test the weight if set as int and also added additional field order number as a float. Updated the check to also check for int and float, not just integer and decimal as depending on how the fields as added that's the type it gets.
Will play around some more to see if there are any other issues, but if fields are re-indexed there should be no issues. Leaving the tags as is (needs work) as it needs some more manual testing. - 🇸🇮Slovenia deaom
Managed to "break" the facets when something in index got changed and no re-index was needed (boost for example). In that instance the filterable attributes got lost as the setOption did not save the data. To not add additional field to the options (as they are part of search api index), changed the adding of the filterable attributes to the third party settings that are already available on the index. Those settings do not get lost on an index save when no re-indexing is needed, and on re-index get populated, so changes are reflected. Will leave to needs work as the MR is still in draft and if any other issues are found.
- 🇸🇮Slovenia deaom
The stage in processor was replaced from preprocess_index to add_properties so possible fields are added to the index before indexing. This removes additional handling of the added fields as they are present in index. The code creates a duplication of a numeric field by prepending __as_string to the original name and copies the value from the original field. The copy field can then be used in facets.
The tests are passing, I've also tested manually and the facets are displayed and working. The original fields are not affected by the change.
To actually have more test coverage a processor test could also be added. Adding this comment so anybody following the issue knows the changes.
The changes can be manually tested, but as additional test could be added and MR being in draft, leaving the status to needs work. - 🇸🇮Slovenia deaom
Added additional test for processor to test that the fields are actually getting added and that values match. Now it kinda can be reviewed. As the MR is still in draft leaving the status to needs work, but can be tested.