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
Comment #1
taran2lPatch attached. Please review.
Comment #2
summit commentedHi,
using this patch I got:
Off all to be indexed commerce displays!
I had to change it on other lines
Any one have a solution for this please?
Greetings, Martijn
Comment #3
drunken monkeyThanks 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!
Comment #4
taran2lYour 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 ?
Comment #5
drunken monkeyThanks for testing, great to hear it works!
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.)
Comment #6
drunken monkey#2114593: Add list of floats to test module was committed and a new Search API version released – this should run now.
Comment #8
drunken monkey#5: 1916474-5--multiple_decimal_values_rounded--combined.patch queued for re-testing.
Comment #10
drunken monkeyMaybe if I re-attach them? (Test details show that it still uses the old 1.8 version of Search API.)
Comment #11.0
(not verified) commentedRemoved white spaces in the PHP code.
Comment #12
drunken monkey10: 1916474-5--multiple_decimal_values_rounded--combined.patch queued for re-testing.
Comment #14
drunken monkeyI'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 …
Comment #15
drunken monkey10: 1916474-5--multiple_decimal_values_rounded--combined.patch queued for re-testing.
Comment #17
drunken monkeyUgh. At least things fail for a real reason now … (On the other hand, it still used Search API 1.8. :-/)
Re-rolls attached.
Comment #20
drunken monkey17: 1916474-17--multiple_decimal_values_rounded--tests_only.patch queued for re-testing.
Comment #21
drunken monkey17: 1916474-17--multiple_decimal_values_rounded--combined.patch queued for re-testing.
Comment #23
drunken monkeyFINALLY!
Thanks to Randy Fay: committed.
Phew!
Issue #1916474 by drunken monkey: Fixed indexing of multi-valued float fields.