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

Allow setting column options like charset and collation on JoinTable, JoinColumn and InverseJoinColumn #9655

Merged
merged 1 commit into from
May 2, 2022
Merged

Allow setting column options like charset and collation on JoinTable, JoinColumn and InverseJoinColumn #9655

merged 1 commit into from
May 2, 2022

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Apr 12, 2022 •
edited
Loading

Alternative approach for #9636

This makes it possible to set custom options on the following:

  • JoinTable
  • JoinColumn
  • InverseJoinColumn

/cc @greg0ire

@ruudk ruudk force-pushed the join-column-collation branch 4 times, most recently from d358864 to b8d0c9f Compare April 12, 2022 07:12
@ruudk ruudk mentioned this pull request Apr 12, 2022
Closed
@ruudk ruudk force-pushed the join-column-collation branch from b8d0c9f to a509d95 Compare April 12, 2022 07:18
@ruudk ruudk force-pushed the join-column-collation branch from a509d95 to ceecdbb Compare April 14, 2022 08:31
@ruudk ruudk force-pushed the join-column-collation branch 6 times, most recently from 0f01e47 to 0126457 Compare April 14, 2022 09:24
@ruudk ruudk changed the title JoinColumn > Allow setting column options like charset and collation Allow setting column options like charset and collation on JoinTable, JoinColumn and InverseJoinColumn Apr 14, 2022
@ruudk ruudk force-pushed the join-column-collation branch from 0126457 to 51e29ff Compare April 14, 2022 13:32
@beberlei
Copy link
Member

This is the way, consistent with the rest. Thanks for the continued work on thi

@ruudk
Copy link
Contributor Author

ruudk commented Apr 14, 2022

Thanks!

I'm currently testing this thoroughly in our large monolith with 250+ entities and various collations. Will report back when I know 100% everything is working.

@ruudk ruudk force-pushed the join-column-collation branch from 51e29ff to 88a6a4f Compare April 15, 2022 07:15
@ruudk
Copy link
Contributor Author

ruudk commented Apr 15, 2022

I finished my testing. I was finally able to escape the diff hell and now get:
Screenshot 2022-04-15 at 09 16 03@2x
🎉

I tested the following:

  • import production table structure with mixed charset/collations and run bin/console doctrine:schema:update --dump-sql and assert no differences
  • drop local database, create database, then run bin/console doctrine:schema:update --force to apply current metadata to database, then run bin/console doctrine:schema:update --dump-sql and assert no differences

@beberlei @greg0ire The PR is now ready for review. Please let me know if I need to add extra tests. Thanks!

@ruudk ruudk requested a review from greg0ire April 15, 2022 07:22
@ruudk
Copy link
Contributor Author

ruudk commented Apr 20, 2022

@derrabus @beberlei @greg0ire could you have a look at this please? Too bad this missed the 2.12 release 😢

@greg0ire greg0ire requested review from beberlei and derrabus April 20, 2022 05:48
@greg0ire greg0ire changed the base branch from 2.12.x to 2.13.x April 20, 2022 05:50
@greg0ire
Copy link
Member

Too bad indeed 😞

@ruudk ruudk force-pushed the join-column-collation branch from 88a6a4f to f3d6d4b Compare April 26, 2022 09:57
@ruudk
Copy link
Contributor Author

ruudk commented Apr 26, 2022

It's ready for review again.

@greg0ire
Copy link
Member

@ruudk the build is failing 🙈

@ruudk
Allow setting column options like charset and collation everywhere
1226b61 
This makes it possible to set custom options on the following:
* `JoinTable`
* `JoinColumn`
* `InverseJoinColumn`
@ruudk ruudk force-pushed the join-column-collation branch from f3d6d4b to 1226b61 Compare April 28, 2022 07:06
@ruudk
Copy link
Contributor Author

ruudk commented Apr 28, 2022

@greg0ire Fixed now.

@derrabus derrabus merged commit d7d6b9d into doctrine:2.13.x May 2, 2022
@ruudk
Copy link
Contributor Author

ruudk commented May 2, 2022

Thanks

Should this be added to the https://github.com/doctrine/orm/milestone/110 milestone ?

@derrabus derrabus added this to the 2.13.0 milestone May 2, 2022
@derrabus
Copy link
Member

derrabus commented May 2, 2022

Yes!

derrabus added a commit to derrabus/orm that referenced this pull request May 5, 2022
@derrabus
Merge branch '2.13.x' into 3.0.x
ee83302 
* 2.13.x:
  Allow doctrine/deprecations 1.0 (doctrine#9723)
  Allow setting column options like `charset` and `collation` everywhere (doctrine#9655)
  Fix psalm annotation
@greg0ire greg0ire mentioned this pull request Jul 7, 2022
Merged
@greg0ire greg0ire mentioned this pull request May 25, 2023
Merged
@torbentschechne
Copy link

torbentschechne commented Aug 24, 2023 •
edited
Loading

Hey there,
would be nice if this change would have been reflected on the documentation page:

https://www.doctrine-project.org/projects/doctrine-orm/en/2.16/reference/annotations-reference.html#annref_joincolumn

There is no information, that the options parameter can be used on the @JoinColumn. I had a lot of trouble because of this in the past days...

@greg0ire
Copy link
Member

Same goes for the XML driver I suppose… would you like to send a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

@greg0ire greg0ire greg0ire approved these changes

@beberlei beberlei Awaiting requested review from beberlei

Assignees
No one assigned
Labels
New Feature
Projects
None yet
Milestone
2.13.0
Development

Successfully merging this pull request may close these issues.
5 participants
@ruudk @beberlei @greg0ire @derrabus @torbentschechne