-
Notifications
You must be signed in to change notification settings - Fork 736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
I'm facing problem with json_decode() #717
Comments
I'm assuming you're doing something along the lines of this? $query = new Query();
$query->setSort(array(
'category_order' => array(
'order' => 'asc',
),
); The solution for me was to simply do this: $query = new Query();
$query->setSort(array(
'category_order' => array(
'order' => 'asc',
'missing' => PHP_INT_MAX -1,
),
); Not sure why this worked or where I came up with the solution, perhaps @ruflin can shed some light on this and whether it's an issue or not? If it is, I'd also be happy to have a look at it. |
@CrAzE124 So far I didn't know about this problem. But I assume you found also the solution here: remicollet/pecl-json-c#13 ? |
@ruflin yeah that's where I found it. It's weird though, I don't really see how or why that solves the problem in terms of the JSON overflow. |
It just tell Elasticsearch to use |
Thanks guys... I'll try to do this and I'm write here if that solve my problem or not. Regards! |
I'm not sure if we should handle this "problem" in Elastica itself or should leave it to the engineer to solve? |
Is it not possible to loop through the |
@CrAzE124 I think that would work and would prevent the problem. Pull request? ;-) |
That means Elastica is going to edit all requests containing a sort to handle this case; that may not be the best thing to do - Elastica should not do stuffs on the queries if I do not ask him.
|
@damienalexandre I agree with you in that it's a bit of a dirty way to do it, but I do believe it lessens the burden on the developer to add We could probably catch that specific overflow exception and alert the user of a possible solution in a "cleaner" way (as in re-throwing a custom error which says something like |
The solution could also have performance implication. The cleanest one would be the exception option mentioned by @damienalexandre. In the other issue there is also a mention of using JSON_BIGINT_AS_STRING. Does this work? |
That would work but only in 5.4+ basically. See for example this implementation where you munge the json after recieving it and if the int is longer than PHP_INT_MAX rewrite it as a string before passing it to json_decode() if you don't have JSON_BIGINT_AS_STRING support. |
As Elastica "officially" only supports 5.4+ this wouldn't be an issue, right? |
We could even introduce this as a "Util" method to keep it compatible with PHP 5.3 and remove it later. |
It also seems to work around it when jsonc is present unless I'm misreading it. In which case we'd still want it even in 5.4+ |
Anyone time for a pull request? ;-) |
I also need this as well Temporarily added this in the parse function to fix it: // add code to set use JSON_BIGINT_AS_STRING by default
if (!isset($args[2])) {
$args[2] = 512;
}
if (isset($args[3])) {
$args[3] = $args[3] | JSON_BIGINT_AS_STRING;
} else {
$args[3] = JSON_BIGINT_AS_STRING;
} |
@yellow1912 Where exactly did you put these lines in? Which PHP Version are you using? |
@ruflin I'm using PHP 5.5.12. The file is JSON.php Here is the gist: https://gist.github.com/yellow1912/47418d628415599a5123 |
Interesting solution. I actually think we could use exactly that code in Elastica. What do you think? |
I was hit by this issue to but I wouldn't want Elastica to some magic here; I as a developer need to know such corner cases and incompatibilities. The solution is to manually add the |
@mfn I understand the concern that Elastica should not "overwrite" potential php settings. At the same time we should allow the developer to also set a default without having to set it for every "setSort". So I would suggest to take the code suggested by @yellow1912 and make it available through a variable in the client which can be set to true. Something like bigintConversion. And of course, the documentation must be improved. |
Experienced the same issue today when trying to sort on a nested field that can also have a null value. Adding the missing key with the @ruflin Any idea if and when that solution will be merged/tagged as a release? |
@oldskool Unfortunately so far there is no pull request open for this change. In case we get a pull request it will directly go into the next release. Feel free to open one with the code provided by @yellow1912. |
@oldskool Cool, thanks. See my comments. |
Add JSON_BIGINT_AS_STRING solution for #717
Hi,
I'm facing problem with Notice:
json_decode(): integer overflow detected
.The weird is that in my dev enviroment this work fine but when I put this in the stage enviroment with another hardware and strucuture (AWS) I'm get this error.
The full log is:
The text was updated successfully, but these errors were encountered: