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

Add possibility to use fields instead of column for unique constraint #8345

Closed
alexander-schranz opened this issue Nov 23, 2020 · 5 comments
Closed
Milestone

Comments

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Nov 23, 2020

Currently when using a unique constraint like:

<unique-constraints>
    <unique-constraint columns="locale,product_key" />
</unique-constraints>

you are forced to define the column name for the fields e.g.:

<field name="productKey" column="product_key" type="string" nullable="true" unique="true"/>

if you don't do this and somebody configure another naming_strategy for your entity it will not longer work as the column does then not exist.

It would be a great feature if you could use unique-constraints based on field name and doctrine orm will read the column name from the configured field e.g.:

<unique-constraints>
    <unique-constraint fields="locale,productKey" />
</unique-constraints>

<!-- no column is needed here now -->
<field name="productKey" type="string" nullable="true" unique="true"/>

What do you think about this?

@beberlei
Copy link
Member

beberlei commented Dec 6, 2020

This is a great idea, there are some caviats.

  • What about associations? Is every join column added? That would make sense.
  • Unique constraints need to be parsed after column/fields so that mapping info is available on CöassMetsdataInfo. This is not the case for all mapping drivers right now

@beberlei beberlei added this to the 2.9.0 milestone Dec 6, 2020
@beberlei
Copy link
Member

beberlei commented Feb 6, 2021

The logic of resolving Unique Constraint (and Index as well) fields to columns would need to be done in SchemaTool, because access to NamingStrategy is needed.

@Lustmored
Copy link
Contributor

I took a shot at this and results are in #8523 - feedback is very welcome :)

Lustmored added a commit to Lustmored/orm that referenced this issue Mar 9, 2021
Lustmored added a commit to Lustmored/orm that referenced this issue Apr 2, 2021
beberlei added a commit that referenced this issue Apr 18, 2021
* Add possibility to use fields instead of column for unique constraint and indexes (#8345)

* Document changes in annotation reference

* phpcs

* Ensure exactly one of fields/columns is set for index/uniqueConstraint

* Adapt docs to fields/columns changes

* phpcs

* Implement fields in Attribute driver and fix mapping classes constructors.

* Coding Standard

* Apply suggestions from code review

Co-authored-by: Grégoire Paris <[email protected]>

* phpcs

Co-authored-by: Jakub Caban <[email protected]>
Co-authored-by: Grégoire Paris <[email protected]>
@beberlei
Copy link
Member

Fixed in #8629

@alexander-schranz
Copy link
Contributor Author

@beberlei @Lustmored This is really great to hear! Thank you for implementing this 🙏

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

No branches or pull requests

3 participants