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

fix applying column options on foreign key columns #6830

Merged
merged 2 commits into from
Nov 20, 2018
Merged

fix applying column options on foreign key columns #6830

merged 2 commits into from
Nov 20, 2018

Conversation

Tobion
Copy link
Contributor

@Tobion Tobion commented Nov 19, 2017

Fixes doctrine/dbal#2811

Since the collation is not a standard option, see doctrine/dbal#2919, it must explicitly be set as platform option on a join column when the referenced column also has a collation set.

@mrxotey
Copy link

mrxotey commented Nov 20, 2017

Hallo @Tobion,
I have the same issue with join column datatypes. I have my own solving PR: #6833. Could you review it?

Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please rebase this branch to get the latest changes of master and add tests covering the issue?

@lcobucci
Copy link
Member

lcobucci commented Nov 27, 2017

As mentioned on #6833, @mrxotey's work can be helpful here

@Tobion
Copy link
Contributor Author

Tobion commented Dec 26, 2017

@lcobucci I rebased and incorporated the changes of #6833

@Tobion Tobion changed the title fix applying collation on foreign key columns fix applying column options on foreign key columns Dec 26, 2017
lib/Doctrine/ORM/Tools/SchemaTool.php Outdated Show resolved Hide resolved
lib/Doctrine/ORM/Tools/SchemaTool.php Outdated Show resolved Hide resolved
tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php Outdated Show resolved Hide resolved
tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php Outdated Show resolved Hide resolved
lib/Doctrine/ORM/Tools/SchemaTool.php Outdated Show resolved Hide resolved
tests/Doctrine/Tests/ORM/Tools/SchemaToolTest.php Outdated Show resolved Hide resolved
@Tobion
Copy link
Contributor Author

Tobion commented Jan 1, 2018

@Majkl578 I did your proposed changes

@Tobion
Copy link
Contributor Author

Tobion commented Jan 5, 2018

This now conflicts with master. @Majkl578 can you merge it into 2.x or rebase it?

@Tobion
Copy link
Contributor Author

Tobion commented Feb 11, 2018

@Majkl578 @Ocramius ping

@yoshz
Copy link
Contributor

yoshz commented Mar 1, 2018

Still anyone working on this PR?

BTW travis failed because getcomposer.org wasn't reachable temporary.

@lcobucci
Copy link
Member

lcobucci commented Mar 1, 2018

Still anyone working on this PR?

We do need to rebase to sync stuff

@Tobion Tobion changed the base branch from master to 2.6 March 30, 2018 16:33
@Tobion
Copy link
Contributor Author

Tobion commented Mar 30, 2018

Rebased and changed base branch. Please merge.

Ocramius
Ocramius previously approved these changes Mar 30, 2018
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! @Tobion I suppose we'll need to forward-port it, right?

@Ocramius Ocramius assigned morozov and unassigned Tobion Mar 30, 2018
@Tobion
Copy link
Contributor Author

Tobion commented Mar 30, 2018

Yes I don't think it has been fixed in master yet.

@morozov
Copy link
Member

morozov commented Mar 30, 2018

@Ocramius you really wanted to assign this to me?

@Ocramius
Copy link
Member

Ocramius commented Mar 31, 2018

@morozov yeah, up to you to decide to apply to stable or just 2.7. I can't do backporting at the moment (no computer)

@morozov
Copy link
Member

morozov commented Mar 31, 2018

@Ocramius I have no idea what the implications of back-porting this to 2.6 are. I don't use ORM. I can merge the ticket though.

@Ocramius
Copy link
Member

@morozov I mistook it for a DBAL issue, sorry! 😱

@Ocramius Ocramius assigned Ocramius and unassigned morozov Mar 31, 2018
@Tobion
Copy link
Contributor Author

Tobion commented Apr 27, 2018

@Ocramius do you need anything here?

@Tobion
Copy link
Contributor Author

Tobion commented May 17, 2018

@Ocramius ping

@Tobion
Copy link
Contributor Author

Tobion commented Oct 28, 2018

@Ocramius can you merge this?

Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tobion I've rebased and applied some minor fixes, LGTM

@lcobucci lcobucci assigned lcobucci and unassigned Ocramius Nov 20, 2018
@lcobucci lcobucci added this to the 2.6.3 milestone Nov 20, 2018
@lcobucci lcobucci merged commit 41ff526 into doctrine:2.6 Nov 20, 2018
@Tobion Tobion deleted the fix-collation-foreign-key branch November 21, 2018 01:34
ixarlie added a commit to surexdirect/doctrine2 that referenced this pull request Jul 10, 2019
v2.6.3

[![Build Status](https://travis-ci.org/doctrine/doctrine2.svg?branch=v2.6.3)](https://travis-ci.org/doctrine/doctrine2)

This release provides fixes for many things, specially:

- Regression in commit order calculation
- BC-break in `EntityManager#find()` using optimistic lock outside of
  transaction
- PHP 7.3 compatibility issues

--------------------------------------------

- Total issues resolved: **8**
- Total pull requests resolved: **26**
- Total contributors: **26**

Documentation
-------------

 - [7472: fix incorrect phpdoc typehint](doctrine#7472) thanks to @seferov
 - [7465: Fixes tiny typo in the 'Working with DateTime instances' documentation](doctrine#7465) thanks to @unguul
 - [7444: Fixed URLs of doctrine-mapping.xsd in docs](doctrine#7444) thanks to @Naitsirch
 - [7441: $hydrationMode throughout can be a string as well as int (for custom modes)](doctrine#7441) thanks to @asgrim
 - [7435: Fix a typo on Documentation](doctrine#7435) thanks to @oguzdumanoglu
 - [7434: Removed FAQ paragraph stating public variables are disallowed](doctrine#7434) thanks to @Naitsirch and @flaushi
 - [7423: Update association-mapping.rst](doctrine#7423) thanks to @ThomasLandauer
 - [7421: JIRA to Github issues on Limitations and Known Issues](doctrine#7421) thanks to @seferov
 - [7412: Some formatting improvements](doctrine#7412) thanks to @ThomasLandauer
 - [7411: Autoload error when following the Getting Started Guide](doctrine#7411) thanks to @ThomasLandauer
 - [7401: &doctrine#91;docs&doctrine#93; Fix docblock in `inheritance-mapping.rst`](doctrine#7401) thanks to @bobdenotter
 - [7397: Update getting-started.rst](doctrine#7397) thanks to @eibt
 - [7394: Class 'Doctrine\Common\Persistence\Mapping\Driver\AnnotationDriver' not found](doctrine#7394) thanks to @ekosynth
 - [7378: Typo fix](doctrine#7378) thanks to @BenMorel
 - [7377: Fix query andX doctype](doctrine#7377) thanks to @sserbin
 - [7374: Deprecation message in documentation for YAML](doctrine#7374) thanks to @SenseException and @iltar
 - [7360: Document getPartialReference() properly](doctrine#7360) thanks to @lcobucci

Bug
---

 - [7471: Fix parameter value processing for objects with unloaded metadata](doctrine#7471) thanks to @alcaeus
 - [7367: Fix for BC break in 2.6.2 when calling EM::find() with LockMode::OPTIMISTIC outside of a TX](doctrine#7367) thanks to @timdev
 - [7328: Handle removed parameters by tree walker in Paginator](doctrine#7328) thanks to @plfort
 - [7325: Make code php 7.3 lint-compatible](doctrine#7325) thanks to @paxal
 - [7317: &doctrine#91;XML&doctrine#93; Fix default value of many-to-many order-by to ASC](doctrine#7317) thanks to @alexdenvir
 - [7260: Fix the handling of circular references in the commit order calculator](doctrine#7260) thanks to @stof
 - [6830: fix applying column options on foreign key columns](doctrine#6830) thanks to @Tobion

Improvement
-----------

 - [7428: CI: Test against PHP 7.3](doctrine#7428) thanks to @Majkl578
 - [7363: Fix compatibility with phan](doctrine#7363) thanks to @philippe-unitiz
 - [7345: Correct DOMDocument constructor in test](doctrine#7345) thanks to @guilliamxavier
 - [7307: Fix remaining usages of deprecated ClassLoader and Inflector from doctrine/common](doctrine#7307) thanks to @Majkl578 and @simonwelsh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants