When an aggregated field relies on fields that are empty (or do not exist if that field is only present for one content type/bundle) the switch case still occurs and max(), min(), etc are called on an empty array resulting in a notice.

Array must contain at least one element in Drupal\search_api\Plugin\search_api\processor\AggregatedFields->addFieldValues()

CommentFileSizeAuthor
#4 2994638-4--empty_min_max_aggregations.patch4.44 KBdrunken monkey
#4 2994638-4--empty_min_max_aggregations--tests_only.patch3.76 KBdrunken monkey
#2 2994638-2--empty_min_max_aggregations.patch700 bytesdrunken monkey
search_api.check-aggregated-empty.patch573 bytesgrayle

Comments

Grayle created an issue. See original summary.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Component: General code » Plugins
StatusFileSize
new 2994638-2--empty_min_max_aggregations.patch700 bytes

Thanks for reporting this!
You’re right, those two aggregations won’t work on empty arrays, just like “First”/“Last” which we’ve already fixed. However, since the other aggregations still make sense (especially “Count”), we shouldn’t use that general check, but instead just add more checks for the individual aggregations.

Patch attached, please test/review!

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Since this was not caught with any of our tests, we should probably add one to it?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2994638-4--empty_min_max_aggregations--tests_only.patch3.76 KB
new 2994638-4--empty_min_max_aggregations.patch4.44 KB

*grumble, grumble* … stupid Joris and his stupid tests … *grumble, grumble*

(Tests-only == interdiff.)

The last submitted patch, 4: 2994638-4--empty_min_max_aggregations--tests_only.patch, failed testing. View results

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

Woo, tests!

  • drunken monkey committed d3ea1f6 on 8.x-1.x 
    Issue #2994638 by drunken monkey, Grayle, borisson_: Fixed warning in...
drunken monkey’s picture

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

:O

Committed. Thanks again, everyone!

Status: Fixed » Closed (fixed)

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