Steps to reproduce:
1. Add decimal/float number field with multiple values to the index
2. Create a node with couple decimal/float values
3. Index content
4. Indexed values will be integers instead of floats due the bug

Affected code (server.inc, line 385-392):

if (is_array($value)) {
  foreach ($value as $v) {
    if ($v !== NULL) {
      $values[$v] = TRUE;
    }
  }
  $values = array_keys($values);
}

array_keys converts float keys to integers.

Comments

taran2l’s picture

Patch attached. Please review.

summit’s picture

Hi,

using this patch I got:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '10100191-137' for key 1

Off all to be indexed commerce displays!

I had to change it on other lines

(624)   $values[] = $v;

Any one have a solution for this please?
Greetings, Martijn

drunken monkey’s picture

Status: Active » Needs review
StatusFileSize
new444 bytes

Thanks for reporting! Didn't know that, I assumed floats would get cast to string. Also thanks for attempting to fix the issue, but as Summit points out, this patch would lead to exceptions when a value is present multiple times.
The attached patch should properly fix this bug. Please test!

taran2l’s picture

Your patch is so clever and simple! Nice work.

I'll test with my data asap.

Works just fine.

BTW: Seems like my original patch passes all tests, but fails in real life. We might need to cover this use case and create new test ?

drunken monkey’s picture

Thanks for testing, great to hear it works!

We might need to cover this use case and create new test ?

Would be best, yes, but it's sadly not that easy. Since we depend on the test module from the Search API, the new float field would have to be added there, and we'd have to wait for the next Search API release to use it.
Still, good tests usually pay off, so let's just do that. Besides, a new Search API release should be out soon. So, why not? Use the attached patches combined with the one in #2114593-1: Add list of floats to test module, that should work. For the test bot, we'll have to wait for the next Search API release. (Therefore, not setting to "needs review" right now.)

drunken monkey’s picture

Status: Postponed » Needs review

#2114593: Add list of floats to test module was committed and a new Search API version released – this should run now.

Status: Needs review » Needs work

The last submitted patch, 1916474-5--multiple_decimal_values_rounded--combined.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1916474-5--multiple_decimal_values_rounded--combined.patch, failed testing.

drunken monkey’s picture

Maybe if I re-attach them? (Test details show that it still uses the old 1.8 version of Search API.)

Status: Needs review » Needs work

The last submitted patch, 1916474-5--multiple_decimal_values_rounded--combined.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Removed white spaces in the PHP code.

drunken monkey’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: 1916474-5--multiple_decimal_values_rounded--combined.patch, failed testing.

drunken monkey’s picture

I'm sad now. :(

Anyone got any idea how to bring the test bot to use the latest version? Last time it worked after not even a day …

drunken monkey’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: 1916474-5--multiple_decimal_values_rounded--combined.patch, failed testing.

drunken monkey’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new2.75 KB
new3.18 KB

Ugh. At least things fail for a real reason now … (On the other hand, it still used Search API 1.8. :-/)
Re-rolls attached.

Status: Needs review » Needs work

Status: Needs review » Needs work

The last submitted patch, 17: 1916474-17--multiple_decimal_values_rounded--combined.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
drunken monkey’s picture

Status: Needs review » Needs work
drunken monkey’s picture

Status: Needs work » Fixed

FINALLY!
Thanks to Randy Fay: committed.
Phew!
Issue #1916474 by drunken monkey: Fixed indexing of multi-valued float fields.

Status: Fixed » Closed (fixed)

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