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

BUGFIX: Copyrightnotice migration for postgres needs default value #2342

Merged
merged 1 commit into from Jan 13, 2019

Conversation

Projects
None yet
3 participants
@daniellienert
Copy link
Member

daniellienert commented Jan 12, 2019

Resolves: #2333

@daniellienert daniellienert force-pushed the daniellienert:bugfix/postgres-migration-for-copyrightnotice branch from 8252422 to 71d12aa Jan 12, 2019

@daniellienert daniellienert requested a review from kdambekalns Jan 12, 2019

@daniellienert daniellienert force-pushed the daniellienert:bugfix/postgres-migration-for-copyrightnotice branch from 71d12aa to b7ace8f Jan 12, 2019

@mstruebing

This comment has been minimized.

Copy link
Contributor

mstruebing commented Jan 12, 2019

Does that field need to be NOT NULL at all?

@daniellienert

This comment has been minimized.

Copy link
Member Author

daniellienert commented Jan 13, 2019

@mstruebing - a very good question!
As I am quite unexperienced with postgres, I orientated at already existing fields like "caption" which are added just like I did.

$this->addSql("ALTER TABLE typo3_media_domain_model_asset ADD caption TEXT NOT NULL");

I am absolutely fine with dropping the NOT NULL

@kdambekalns

This comment has been minimized.

Copy link
Member

kdambekalns commented Jan 13, 2019

This is actually only partly related to PostgreSQL, in the sense that NOT NULL with MySQL only means you cannot insert NULL, but leaving out a field fills it with an empty string for character fields.

Technically it rather depends on the model. And Asset.php initializes the field with an empty string, so NOT NULL with an empty string default seems in fact correct. And yes, the same should be the case for caption, to be honest.

Now, confusingly exactly that "empty string default" is the result of NOT NULL on MySQL. Go figure.

@daniellienert

This comment has been minimized.

Copy link
Member Author

daniellienert commented Jan 13, 2019

@daniellienert daniellienert merged commit eb9ef7a into neos:4.2 Jan 13, 2019

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@daniellienert daniellienert deleted the daniellienert:bugfix/postgres-migration-for-copyrightnotice branch Jan 13, 2019

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