Skip to content
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

attempt to fix bad inserted values for photo lat/lng #98

Merged
merged 2 commits into from
Aug 30, 2019
Merged

Conversation

julien-nc
Copy link
Member

Trying to fix #95.

I don't know how we can get strange lat/lng values but I hope at least the symptom is fixed with this PR.

@tacruc
Copy link
Collaborator

tacruc commented Aug 30, 2019

I don't know how we can get strange lat/lng values but I hope at least the symptom is fixed with this PR.

Me neither thats why I just posted the log.

@julien-nc
Copy link
Member Author

There's no rush but:

Once #94 is resolved and this fix is tested, accepted and merged, I'll probably publish v0.1.2

@tacruc
Copy link
Collaborator

tacruc commented Aug 30, 2019

Yeah I know, I tested it on my development system with sqlite database but there the problem did not occur even without the fix.
Now I'm testing on my productive system, but there are around 100k pictures on it, so it will take some time.

@julien-nc
Copy link
Member Author

Do you mean it occurred without the fix and then you tried again and it didn't occur (still without the fix)?

If so, it will be hard to be sure we avoid this kind of crash.

@tacruc
Copy link
Collaborator

tacruc commented Aug 30, 2019

Do you mean it occurred without the fix and then you tried again and it didn't occur (still without the fix)?

If so, it will be hard to be sure we avoid this kind of crash.

Yeah, well I putted the picture onto my dev installation, which uses sqlite so there is a difference now I try if I can reporduce the problem on my production system. But this takes time due to the number of pictures.

@julien-nc
Copy link
Member Author

You could add the file in a fresh user account and scan for this user only.

@tacruc
Copy link
Collaborator

tacruc commented Aug 30, 2019

Ok the error can be reproduced with mysql database but not with sqlite.

@julien-nc
Copy link
Member Author

Were you running this PR's branch while testing on your mysql setup?

@tacruc
Copy link
Collaborator

tacruc commented Aug 30, 2019

No I first tried to reproduce the error. It allways happs on mysql but not on sqlite.
Now I applied the PR, but the error is not fixed.
Can it be that NAN is numeric?
https://www.php.net/manual/en/function.is-numeric.php#114061

@julien-nc
Copy link
Member Author

It seems impossible to me 😄. I just tried in interactive mode with php7.2 and it's not.

@julien-nc
Copy link
Member Author

Ouch, strange thing:

if (is_numeric(NAN)) echo 'yep';

It appears NAN is numeric and "NAN" is not. But how does the constant NAN reaches the variables we use?

Could you quickly try to change the check and also verify that lat and lng are different than NAN?

Signed-off-by: Arne Hamann <[email protected]>
@julien-nc
Copy link
Member Author

Nice! I guess you tried it and it works 😄.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Doctrine\DBAL\Exception\DriverException]
3 participants