Skip to content

[DoctrineBridge][PropertyInfo] Treat Doctrine decimal type as string #19924

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

Merged
merged 1 commit into from
Sep 13, 2016

Conversation

teohhanhui
Copy link
Contributor

Q A
Branch? 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

Using float for decimal type defeats the purpose of avoiding rounding errors / loss of precision.

See http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/types.html#decimal

@sstok
Copy link
Contributor

sstok commented Sep 13, 2016

Status: reviewed

@teohhanhui teohhanhui changed the title [PropertyInfo] Treat Doctrine decimal type as string [DoctrineBridge][PropertyInfo] Treat Doctrine decimal type as string Sep 13, 2016
@teohhanhui teohhanhui force-pushed the propertyinfo-doctrine-decimal branch from 76ed973 to 62d28f9 Compare September 13, 2016 08:51
@javiereguiluz
Copy link
Member

@teohhanhui just asking: could this be a BC break for some application?

@teohhanhui
Copy link
Contributor Author

I'd see it as a bug, i.e. we were returning the wrong type. But of course, yes, applications might break if they depend on a certain type being returned. (I don't think that should be considered part of our BC promise.)

@fabpot
Copy link
Member

fabpot commented Sep 13, 2016

Thank you @teohhanhui.

@fabpot fabpot merged commit 62d28f9 into symfony:2.8 Sep 13, 2016
fabpot added a commit that referenced this pull request Sep 13, 2016
… as string (teohhanhui)

This PR was merged into the 2.8 branch.

Discussion
----------

[DoctrineBridge][PropertyInfo] Treat Doctrine decimal type as string

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

Using `float` for decimal type defeats the purpose of avoiding rounding errors / loss of precision.

See http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/types.html#decimal

Commits
-------

62d28f9 [DoctrineBridge][PropertyInfo] Treat Doctrine decimal type as string
@teohhanhui teohhanhui deleted the propertyinfo-doctrine-decimal branch September 14, 2016 04:22
This was referenced Oct 3, 2016
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.

5 participants