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

Stop using the deprecated json_array type internally #2782

Merged
merged 3 commits into from
Jul 22, 2017
Merged

Stop using the deprecated json_array type internally #2782

merged 3 commits into from
Jul 22, 2017

Conversation

lcobucci
Copy link
Member

No description provided.

@lcobucci lcobucci added this to the 2.6 milestone Jul 19, 2017
@lcobucci lcobucci self-assigned this Jul 19, 2017
@lcobucci lcobucci requested review from Ocramius and deeky666 July 19, 2017 20:10
@lcobucci lcobucci force-pushed the fix/stop-using-deprecated-types branch from 3f56f92 to c611df2 Compare July 20, 2017 08:32
@lcobucci lcobucci force-pushed the fix/stop-using-deprecated-types branch from c611df2 to 0b50739 Compare July 20, 2017 08:37
Ocramius
Ocramius approved these changes Jul 21, 2017
View reviewed changes
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.

@lcobucci any user-side change required here? Parch seems good otherwise.

@lcobucci
Copy link
Member Author

@Ocramius I think it shouldn't required but TBQH I'm not sure.

@Ocramius
Copy link
Member

YOLO, 🚢

@Ocramius Ocramius merged commit 6fb7a4b into doctrine:master Jul 22, 2017
@lcobucci lcobucci deleted the fix/stop-using-deprecated-types branch July 22, 2017 17:22
@Ocramius Ocramius changed the title Stop using deprecated "json_array" type Stop using the deprecated json_array type internally Jul 22, 2017
@bendavies
Copy link
Contributor

bendavies commented Aug 14, 2017 •
edited
Loading

@Ocramius @lcobucci
there is a BC break/bug here.

changing the default doctrineTypeMapping from json_array to json causes unfixable schema comparison.

i have an old column using json_array and this PR results in a schema diff (postgres) of:

ALTER TABLE settings ALTER parameters TYPE JSON
ALTER TABLE settings ALTER parameters DROP DEFAULT

Applying the diff does nothing. the same diff persists afterwards.

Only way for the user to fix this is to change the column type from json_array to json

@lcobucci
Copy link
Member Author

@bendavies that's sad news, sorry for causing this issue.

I'll be checking the options to fix the problem caused by 31d0128.

Thanks for letting us know of that

@bendavies
Copy link
Contributor

Thanks @lcobucci!

@lcobucci lcobucci mentioned this pull request Aug 29, 2017
Open
@stof
Copy link
Member

stof commented Sep 7, 2017

this requires changing which type requires SQL comment hint. currently, json_array says it does not require a SQL comment hint when native json type is supported in the platform, because platforms would map the native type to it. But as you changed it, you have to change the comment hint requirement here

lcobucci added a commit to lcobucci/dbal that referenced this pull request Sep 11, 2017
@lcobucci
Add test to ensure json_array can still be used
2d29a77 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
@lcobucci lcobucci mentioned this pull request Sep 11, 2017
Merged
@lcobucci
Copy link
Member Author

@bendavies @stof I think I've managed to fix the reported BC-break on #2855, could you please check (it's still WIP, though)?

lcobucci added a commit to lcobucci/dbal that referenced this pull request Sep 11, 2017
@lcobucci
Add test to ensure json_array can still be used
47918e6 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
lcobucci added a commit to lcobucci/dbal that referenced this pull request Sep 11, 2017
@lcobucci
Add test to ensure json_array can still be used
94f6786 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
kopaygorodsky added a commit to kopaygorodsky/JMSJobQueueBundle that referenced this pull request Sep 20, 2017
@kopaygorodsky
updated json_array to json in Job entity
d3c51b8 
According to BC-break in doctrine/dbal
doctrine/dbal#2782 (comment)
@kopaygorodsky kopaygorodsky mentioned this pull request Sep 20, 2017
Closed
lcobucci added a commit to lcobucci/dbal that referenced this pull request Oct 25, 2017
@lcobucci
Add test to ensure json_array can still be used
42f7b2e 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
lcobucci added a commit to lcobucci/dbal that referenced this pull request Nov 18, 2017
@lcobucci
Add test to ensure json_array can still be used
743a93a 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
lcobucci added a commit to lcobucci/dbal that referenced this pull request Nov 18, 2017
@lcobucci
Add test to ensure json_array can still be used
6f43e26 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
lcobucci added a commit to lcobucci/dbal that referenced this pull request Nov 18, 2017
@lcobucci
Add test to ensure json_array can still be used
78c12e2 
On 31d0128 we made JsonType the default
one, which caused some unexpected results with the comparator.

This tests validates that if users have a JSON column but are using the
JsonArrayType we'll just add a comment to the column.

Reference: doctrine#2782
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Ocramius Ocramius Ocramius approved these changes

@deeky666 deeky666 Awaiting requested review from deeky666

Assignees

@lcobucci lcobucci

Labels
Improvement Platforms Schema Definition Schema Introspection Type Mapping Types
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@lcobucci @Ocramius @bendavies @stof