Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Fix decimal marshalling in specific locales #103

Closed
wants to merge 2 commits into from

Conversation

bjornpost
Copy link
Collaborator

In certain locales, the decimal separator is a comma instead of a point, so 12.34 becomes 12,34. This causes an issue with marshalling Decimals; as Pheasant tries to store them as 12,34. MySQL can't handle this and stores 12.00.

This PR fixes the issue and adds a testcase. I used the nl_NL in the testcase, but the fixes works for all locales which use a comma as decimal separator (German, Danish, Swedish, French, Italian, etc.)

@bjornpost
Copy link
Collaborator Author

Oh btw, this does not work for Arabic locales, which use ' as a decimal separator. Adding support for those locales as well would require a more complex solution: http://php.net/manual/en/function.floatval.php#92563 . I don't think that support for that is worth the performance penalty.

@harto
Copy link
Collaborator

harto commented Jan 27, 2014

This seems a bit odd. Surely the Mysqli driver can do the right thing with PHP's float type regardless of locale. Is the issue that your float values are being prematurely converted to strings?

@jorisleker
Copy link
Contributor

Floats are casted to strings when creating sql statements in PHP (that's what Pheasant does, isn't it), ergo before they reach the mysqli driver.

PHP's behavior with LC_ALL or LC_NUMERIC set to a locale with ',' as decimal sep is asymmetric. ((float) '2.13' results in 2,13, but (string) 2,13 results in 2)

Question remains whether Pheasant should support other locales in LC_NUMERIC or just discourage that. I think that should clearly be stated somewhere in the docs though.

@bjornpost
Copy link
Collaborator Author

Thoughts @lox?

@lox
Copy link
Owner

lox commented Mar 23, 2014

Interesting problem. This is ultimately solved by moving away from Pheasant's userspace param binding and deferring entirely to the underlying mysqli implementation. I'm going to address this in 2.0.

In the meantime, I'd suggest setting setlocale(LC_NUMERIC, 'en-US') in your code that is using Pheasant.

@lox lox closed this Mar 24, 2014
bjornpost pushed a commit to bjornpost/pheasant that referenced this pull request Apr 14, 2014
@bjornpost bjornpost deleted the fix-float-locale branch April 15, 2015 09:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants