Improved search engine for numeric search queries #428
Conversation
| } | ||
|
|
||
| if (in_array($metadata['dataType'], array('text', 'string'))) { | ||
| if (is_numeric($searchQuery)) { |
There was a problem hiding this comment.
Shouldn't you add && in_array($metadata['dataType'], array('integer', 'number')) ?
There was a problem hiding this comment.
I've made some changes. I'd appreciate if you could review it again. Thanks!
|
Seems good to me actually. @ogizanagi ? 😄 |
|
|
||
| if (is_numeric($searchQuery) && $isNumericField) { | ||
| $queryConditions->add(sprintf('entity.%s = :exact_query', $name)); | ||
| $queryParameters['exact_query'] = 0 + $searchQuery; // adding '0' turns the string into a numeric value |
There was a problem hiding this comment.
I don't understand this ? 😕 Why is it needed ?
There was a problem hiding this comment.
It's more relevant than typecasting it. We could indeed typecast depending on whether it's an integer or a float, but this solution seems good to me.
But yeah, when I saw this, I was like "wtf?", but I think it makes sense.
There was a problem hiding this comment.
Adding 0 is a trick to convert a numeric string into its numeric counterpat, no matter if it's an integer or float. There is no single PHP function to do this (you must combine intval(), etc.)
If we don't add the 0 and the search query is 56 for example, this is the query I get:
If we add the 0 and search again 56, this is the query:
There was a problem hiding this comment.
We could also check whether the "numeric" field type is an integer or a float, and then typecast correctly to avoid comparing a float to an integer, I've just realized that. Is postgresql restrictive in comparing integer and floats ?
There was a problem hiding this comment.
@javiereguiluz : but how is the 'entity.%s = :exact_query' part transformed to WHERE c0_.id IN ('56') in the first screenshot, by juste removing the type converting ?...
My doubt was about the necessity to have the proper type in the bind parameters.
There was a problem hiding this comment.
Because I took the first screenshot with another pull request. This is the right screenshot when no 0 is added to the numeric query:
There was a problem hiding this comment.
From Doctrine doc:
Calling setParameter() automatically infers which type you are setting as value. This works for integers, arrays of strings/integers, DateTime instances and for managed entities.
There was a problem hiding this comment.
The doc is clear ... but it doesn't work like that for me :(
There was a problem hiding this comment.
@javiereguiluz : I think I misunderstand it. I might be tired. Anyway, thanks for the effort you always put into explaining or exposing anything 😃
This fixes #422.