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

Ignore columnDefinition in schema comparison #5224

Merged
merged 2 commits into from
Feb 2, 2022
Merged

Ignore columnDefinition in schema comparison #5224

merged 2 commits into from
Feb 2, 2022

Conversation

bcremer
Copy link
Contributor

@bcremer bcremer commented Jan 31, 2022 •
edited by greg0ire
Loading

Q A
Type improvement
BC Break no
Fixes #5223

Summary

Support schema validation when using columnDefinition.

Closes #5234

@bcremer bcremer mentioned this pull request Jan 31, 2022
Closed
@bcremer bcremer force-pushed the schema-validator-with-customDefinition branch from d003fd6 to b799815 Compare January 31, 2022 12:53
@bcremer bcremer force-pushed the schema-validator-with-customDefinition branch from b799815 to a49798b Compare February 2, 2022 07:10
@bcremer bcremer changed the title WIP: Support columnDefinition in schema comparison for MySQL Platform WIP: Ignore columnDefinition in schema comparison Feb 2, 2022
@bcremer bcremer force-pushed the schema-validator-with-customDefinition branch from a49798b to 626eb67 Compare February 2, 2022 07:13
@Ocramius
Copy link
Member

Ocramius commented Feb 2, 2022 •
edited
Loading

@bcremer could you cherry-pick my test from #5234 into this? Would like to see if this is a possible solution :)

@Ocramius
Copy link
Member

Ocramius commented Feb 2, 2022

Locally, I verified that the test @ e760ef8 (from #5234) works with this patch on SQLite

@Ocramius Ocramius added this to the 3.3.2 milestone Feb 2, 2022
@Ocramius Ocramius linked an issue Feb 2, 2022 that may be closed by this pull request
Schema diff changes since version 3.3.1 #5223
Closed
@Ocramius Ocramius mentioned this pull request Feb 2, 2022
Closed
@greg0ire greg0ire requested a review from morozov February 2, 2022 17:21
@greg0ire
Copy link
Member

greg0ire commented Feb 2, 2022

Is this still WIP?

@morozov
Copy link
Member

morozov commented Feb 2, 2022

Without digging too much into the details, the approach here looks more straightforward and correct than the one in #5234. So I assume if the test from there passes with this fix, this is what we need.

@Ocramius @greg0ire
Failing test case: comparator yields invalid diffs when `Column#$_col…
1d40ffe 
…umnDefinition` is set

When a `Column` is compared against another `Column` with a `$_columnDefinition`, it yields invalid
results, because it generates a diff, but does not declare that the `columnDefinition` is the changed
property.

Before `3.3.1`, this behavior worked fine, but doctrine#5220 introduced
a regression that led to the schema comparison changing behavior here.
@greg0ire greg0ire changed the title WIP: Ignore columnDefinition in schema comparison Ignore columnDefinition in schema comparison Feb 2, 2022
dmaicher
dmaicher approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@dmaicher dmaicher left a comment

Choose a reason for hiding this comment

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

This works for me and schema diff because of custom columnDefinition are gone 👍

@greg0ire
Copy link
Member

greg0ire commented Feb 2, 2022

Blocked by #5239

@greg0ire greg0ire closed this Feb 2, 2022
auto-merge was automatically disabled February 2, 2022 19:55

Pull request was closed

@greg0ire greg0ire reopened this Feb 2, 2022
@greg0ire greg0ire merged commit d9918cf into doctrine:3.3.x Feb 2, 2022
@morozov morozov mentioned this pull request Feb 4, 2022
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dmaicher dmaicher dmaicher approved these changes

@greg0ire greg0ire greg0ire approved these changes

@morozov morozov morozov approved these changes

Assignees
No one assigned
Labels
Bug Columns Schema Comparison Schema Definition Schema
Projects
None yet
Milestone
3.3.2
Development

Successfully merging this pull request may close these issues.

Schema diff changes since version 3.3.1
5 participants
@bcremer @Ocramius @greg0ire @morozov @dmaicher