Fix handling of NULL values when indexing

Patch works for me

Thanks for reporting and even creating a patch!

The patch looks good, but is too specific. The problem seems to be that NULL values are incorrectly indexed as an integer 0, thus creating the confusion. I think if we index NULL values correctly, we'll also eliminate this bug.

Please test the attached patch! If it doesn't solve your problem, we might have to change facet query …

Thanks! Patch #4 is working fine.

Hm, sorry but the patch in #4 has a bad side effect for me. I lost all results.
Patch #3 hides the zero in the facet filter results but not the filter entry (now with empty text instead of 0). "Add facet for missing values" is off.

I'm using views to display results of the search index.
After applying your patch I re-indexed everything. Now facets work as expected but the view does not display anything.

I've now found the problem in the creation of the facet queries. The code for the "missing" facet was only suitable for multi-valued fields and didn't take single-valued ones into account.
The attached patch should fix this.
(The indexing code is the same as in #3 so you won't have to re-index again, if you've already installed/tested the first patch.)

Thank you so much. Patch #10 is working. Now it is possible to turn on/off "Add facet for missing values".

Great, committed!
Thanks for helping, everyone!

Hi guys,

I took a try with the patch #10 and checked the latest dev version (in which it's inluded) and after indexing content I got the following warning errors:

>Warning: Invalid argument supplied for foreach() in SearchApiDbService->indexItem() (line 342 of search_api_db/service.inc).

I would assume it's because I got cases where $type is text and $value NULL or empty.
$value = $this->convert($field['value'], $type, $field['original_type'], $index);

In which case, in the foreach below (line 342 of the latest dev: beta3+4-dev), there is a warning due to:
foreach ($value as $token)
and $value is NULL

So I tried applying the patch from:
http://drupal.org/node/1329192#comment-5647194
and everything worked fine, from what I have tested so far with strings, text, taxonomy reference, product, etc...

Pretty much, if it's a single value it wouldn't be an array, in which case it's fine to test if it's null or not. if it's an array and this array is empty (returned from convert, for example), then there's no warning in the foreach that simply skips the loop (empty array).

Lastly, I just wanted to get back to #4 in this ticket.
It seems there is still some difference between the patch and the latest commit:
- else {
+ else if (isset($value)) {

Indeed, I did the test and it does make a difference: prevents indexing of NULL items, in my case, for strings/text.

So to summarize:
I've applied:
http://drupal.org/node/1649042#comment-6158914
and
http://drupal.org/node/1329192#comment-5647194
and:
- else {
+ else if (isset($value)) {

right now I seem to be having consistent results on search queries and facets.
I haven't identified any side effect yet.
Still, I'm wondering about the empty facets cases or '0' integer values, so I would be glad to hear your feedback on this.

Attached is the patch with the 2 previous small changes, against latest dev: 7.x-1.0-beta3+4-dev

How does the patches work for you? Is anyone getting the same warning?
Thanks in advance for your feedbacks.

changing status

latest dev seems te work fine for me.

Oops, you're right, that could happen I guess. Thanks for spotting this!
Please try, whether the attached patch fixes this!

I created a patch from #13 and #16 that applies to beta3.

#16 works fine for me.

However, there a could be an alternate behavior than hiding the empty values :
There could a facet item showing "None" or something like that, so one could select all results that have no values for this facet (In my case, taxonomy term).

Still, we may have to make a difference between nodes with term reference empty (optional reference) e.g. "None", and node types that don't have a term reference e.g. "Undefined".

But #16 patch makes me happy for now!

Where is it supposed to aply the patch?

#16 fixes If node has empty body i get Invalid argument supplied

#17 will not apply on latest dev version
Checking patch service.inc...
error: while searching for:

+++ b/service.inc
@@ -426,11 +432,19 @@ class SearchApiDbService extends SearchApiAbstractService {
       if (is_array($value)) {
         foreach ($value as $v) {
           $v = $this->convert($v, $type, $original_type, $index);

not found

Nope, but it should apply to beta3. I made it to fix on a live site, thought I would share.

I've had problems with empty body texts and applied both #13 and #16. Now the errors are gone :)

Still errors with the latest version:

Warning: Invalid argument supplied for foreach() in SearchApiDbService->indexItem() (line 342 of /home/***/public_html/sites/all/modules/search_api_db/service.inc).

I re-created the patch from #17 that applies to beta4.

#24 is working for us.
Patch didn't apply cleanly in our env due to a different path (sites/all/modules/... vs sites/all/modules/contrib/...) - but the code works.

Patch #24 solve the issue for me also!

Tested #27 and it worked !

Thanks! #24 solve the problem for me!

Best regards
Frank

Finally fixed this.
Sorry for the long wait, and thanks a lot for the re-rolls and testing!

Really happy I found this, updated to latest dev (from 1.0-beta3) + reindex fixed my problems. Great work folks.

+1 for #27, #24