More access to field definition and data in custom types #6015

Open
opened 2026-01-22 15:24:49 +01:00 by admin · 9 comments
Owner

Originally created by @Padam87 on GitHub (Jul 9, 2018).

Feature Request

Q A
New Feature yes
RFC yes
BC Break no

Summary

This is mainly a question, but it may become a feature request.

We use and store a lot of money https://github.com/moneyphp/money objects. These are tied to the same entity, and their currency is always the same.

Right now, there are 2 solutions to do this.

1. Create an embeddable mapping for Money\Money, and store it directly.
This is nice and clean on the PHP side, and if you only have one money field in your entity, it is perfect. When you have 10, you are going to end up with a table with 10 amount, and 10 currency fields.

2. Store the currency, and the 10 amounts; instanciate the Money objects in the getters.
This way, you get a nice database, but a really messy Entity, with a lot of problems:

  • Getters instanciating the Money object.
  • Embeddables don't have access to their owners, so if you have amounts stored there, you have no access to the currency (possible to "fix" with postLoad listener inject).
  • Setters problems: You either only accept Money objects (to keep consistency with the getter), and deal with the conversion, or you can accept bigint / Money (to simpify usage).

I know this is a very specific problem, and the doctrine library is not supposed to solve it.
However, we developers should be able to create an adequate solution.

I wanted to create a custom type to solve this, but unfortunately I couldn't.

  • convertToPHPValue has no access to other values (even raw values would be fine here)
  • Also tried to use convertToPHPValueSQL to concatenate the fields into something like EUR 10000, but this method has no access to the field definition (I wanted to set the name of the currency field in the options)

Then I started thinking about using a postLoad listener to change the bigint values to Money. This would have worked, but I don't really feel good about it, as it borders on black magic territory.

So I went with option 1, mainly because it will be easy to refactor once I have a real solution.

Could it be possible to improve Types in Doctrine3? Give some more context to the methods mentioned above?

Originally created by @Padam87 on GitHub (Jul 9, 2018). ### Feature Request | Q | A |------------ | ------ | New Feature | yes | RFC | yes | BC Break | no #### Summary This is mainly a question, but it may become a feature request. We use and store a lot of money https://github.com/moneyphp/money objects. These are tied to the same entity, and their currency is always the same. Right now, there are 2 solutions to do this. **1. Create an embeddable mapping for Money\Money, and store it directly.** This is nice and clean on the PHP side, and if you only have one money field in your entity, it is perfect. When you have 10, you are going to end up with a table with 10 amount, and 10 currency fields. **2. Store the currency, and the 10 amounts; instanciate the Money objects in the getters.** This way, you get a nice database, but a really messy Entity, with a lot of problems: - Getters instanciating the Money object. - Embeddables don't have access to their owners, so if you have amounts stored there, you have no access to the currency (possible to "fix" with postLoad listener inject). - Setters problems: You either only accept Money objects (to keep consistency with the getter), and deal with the conversion, or you can accept bigint / Money (to simpify usage). I know this is a very specific problem, and the doctrine library is not supposed to solve it. However, we developers should be able to create an adequate solution. I wanted to create a custom type to solve this, but unfortunately I couldn't. - `convertToPHPValue` has no access to other values (even raw values would be fine here) - Also tried to use `convertToPHPValueSQL` to concatenate the fields into something like ` EUR 10000`, but this method has no access to the field definition (I wanted to set the name of the currency field in the options) Then I started thinking about using a `postLoad` listener to change the bigint values to Money. This would have worked, but I don't really feel good about it, as it borders on black magic territory. So I went with option 1, mainly because it will be easy to refactor once I have a real solution. Could it be possible to improve Types in Doctrine3? Give some more context to the methods mentioned above?
Author
Owner

@Padam87 commented on GitHub (Jul 9, 2018):

I would be curious what @sagikazarmark and @frederikbosch think about this.

@Padam87 commented on GitHub (Jul 9, 2018): I would be curious what @sagikazarmark and @frederikbosch think about this.
Author
Owner

@frederikbosch commented on GitHub (Jul 10, 2018):

@Padam87 I don't use Doctrine, but use a similar solution as number 1. Why is it a problem to have 20 database fields for 10 value objects? I mean the number of columns is hardly limited (4096 in case for mysql).

@frederikbosch commented on GitHub (Jul 10, 2018): @Padam87 I don't use Doctrine, but use a similar solution as number 1. Why is it a problem to have 20 database fields for 10 value objects? I mean the number of columns is hardly limited (4096 in case for mysql).
Author
Owner

@Padam87 commented on GitHub (Jul 10, 2018):

It is more a "dislike" than a problem. We could argue redundancy, but since it is 3 chars long I wont. 😄

Currency is always the same for the 10 amounts. The way to store this normally is 1 currency field, and 10 amounts.

Embeddables should not do that (obviously). Custom types should be able to, but they cannot due to the lack of context in the 2 methods mentioned above.

@Padam87 commented on GitHub (Jul 10, 2018): It is more a "dislike" than a problem. We could argue redundancy, but since it is 3 chars long I wont. 😄 Currency is always the same for the 10 amounts. The way to store this normally is 1 currency field, and 10 amounts. Embeddables should not do that (obviously). Custom types should be able to, but they cannot due to the lack of context in the 2 methods mentioned above.
Author
Owner

@frederikbosch commented on GitHub (Jul 10, 2018):

@Padam87 If you ask me, you are solving a problem that does not exist. I don't see why you want to spend time in reducing the number of columns in a database.

@frederikbosch commented on GitHub (Jul 10, 2018): @Padam87 If you ask me, you are solving a problem that does not exist. I don't see why you want to spend time in reducing the number of columns in a database.
Author
Owner

@Padam87 commented on GitHub (Jul 10, 2018):

Hm, maybe it's just me, but just because I use an ORM, I don't ignore my database schema.

@Padam87 commented on GitHub (Jul 10, 2018): Hm, maybe it's just me, but just because I use an ORM, I don't ignore my database schema.
Author
Owner

@frederikbosch commented on GitHub (Jul 10, 2018):

@Padam87 That's not the point. I totally agree with you that using an ORM should never mean ignoring the database schema.

However, a table can simply contain some overhead with the same data in different columns. Otherwise you should create a table currencies and then have all currencies in that table and reference it with a currency_id. I think you are trying to abstract something that should not be abstracted.

@frederikbosch commented on GitHub (Jul 10, 2018): @Padam87 That's not the point. I totally agree with you that using an ORM should never mean ignoring the database schema. However, a table can simply contain some overhead with the same data in different columns. Otherwise you should create a table `currencies` and then have all currencies in that table and reference it with a `currency_id`. I think you are trying to abstract something that should not be abstracted.
Author
Owner

@Ocramius commented on GitHub (Jul 10, 2018):

Mapping the same field to multiple values is out of discussion anyway, but mapping N distinct fields to M distinct value objects is something that we are considering for ORM v3.

@Ocramius commented on GitHub (Jul 10, 2018): Mapping the same field to multiple values is out of discussion anyway, but mapping N distinct fields to M distinct value objects is something that we are considering for ORM v3.
Author
Owner

@Padam87 commented on GitHub (Jul 10, 2018):

@Ocramius Would that cover mapping currency + amount1 to money1 and currency + amount2 to money2?

It would be perfect. :)

@Padam87 commented on GitHub (Jul 10, 2018): @Ocramius Would that cover mapping currency + amount1 to money1 and currency + amount2 to money2? It would be perfect. :)
Author
Owner

@Padam87 commented on GitHub (Jul 10, 2018):

@frederikbosch Instead of this:
https://gist.github.com/Padam87/6cbf9efd2be9fd28c4c70b7b16ab38ca

I would like to have this schema:
https://gist.github.com/Padam87/ef41e3e0aa1a2546ce2580f761ef2a7e

@Padam87 commented on GitHub (Jul 10, 2018): @frederikbosch Instead of this: https://gist.github.com/Padam87/6cbf9efd2be9fd28c4c70b7b16ab38ca I would like to have this schema: https://gist.github.com/Padam87/ef41e3e0aa1a2546ce2580f761ef2a7e
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6015