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.

Sign up for GitHub

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

Jump to bottom

xsd:decimal shouldn't be represented as double #178

Closed
indeyets opened this issue Apr 9, 2014 · 4 comments
Closed

xsd:decimal shouldn't be represented as double #178

indeyets opened this issue Apr 9, 2014 · 4 comments
Labels
bug

Comments

@indeyets
Copy link
Contributor

indeyets commented Apr 9, 2014

Currently, https://github.com/njh/easyrdf/blob/devel/lib/EasyRdf/Literal/Decimal.php#L66 represents xsd:decimal as php's double type.

The problem is, that xsd:decimal is supposed to store arbitrary precision numbers, while double is an approximate floating point number. It's mostly fine for easy cases, but can become a problem as soon as someone tries to store something money-related using such value.

Proposed solution: return value as a string, exactly the way it is stored in document/triplestore. This way, end-user will be able to operate on it using something like BCMath without losing precision

Additionally, floating point values given to constructor should be converted to strings, which use . as a separator (as that is required by standard)
@indeyets indeyets added the bug label Apr 9, 2014
@ianmillard
Copy link

+1, this is an issue

A while back I had a different but related problem: when adding reasonably long alphanumeric codes/identifiers as literal values to a resource, some turned out to be completely numeric.

I believe addLiteral was automatically typing them as xsd:decimal, and subsequently as a result of being stored as a double they came out in turtle serialisation formatted as 1.234e5 or whatever. I actually wanted them to remain as an untyped string, so explicitly casting these values as a string before passing to addLiteral was a quick fix for me on that occasion. However, the wider issue still remains.

@indeyets
Copy link
Contributor Author

indeyets commented Apr 9, 2014

complexity: this is a bug, but a fix would lead to BC break.

@njh I can provide a patch, but would it be ok to land it on "devel" branch? or we need to create separate 1.x branch?

@njh
Copy link
Collaborator

njh commented Apr 9, 2014

I think put it in the devel branch. If it is already slightly broken then an API break probably won't upset too many people.

indeyets added a commit that referenced this issue Apr 11, 2014
@indeyets
"decimal" test corresponds to xsd:decimal now
a3868d4 
refs #178
indeyets added a commit that referenced this issue Apr 11, 2014
@indeyets
test with valid decimals. refs #178
186d160
indeyets added a commit that referenced this issue Apr 11, 2014
@indeyets
proper decimal handling. fixes #178
5908941
indeyets added a commit that referenced this issue Apr 11, 2014
@indeyets
tweaked generic EasyRdf_Literal test. refs #178
2b477eb
@indeyets
Copy link
Contributor Author

fixed

indeyets added a commit that referenced this issue Apr 11, 2014
@indeyets
decimal canonicalisation test. refs #178
650e6f9
indeyets added a commit that referenced this issue Apr 11, 2014
@indeyets
decimal should be locale-agnostic
390c817 
PHP's float->string conversion is locale dependent. It can use arbitrary symbol as decimal-separator.
refs #178
indeyets added a commit that referenced this issue Apr 19, 2014
@indeyets
turtle serializer should treat decimals as strings. refs #178
3636038
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@indeyets @njh @ianmillard