Skip to content

Enforce no notnull for boolean to store false#24055

Merged
MorrisJobke merged 4 commits into
masterfrom
bugfix/noid/enfore-no-notnull-for-boolean-to-store-false
Apr 1, 2021
Merged

Enforce no notnull for boolean to store false#24055
MorrisJobke merged 4 commits into
masterfrom
bugfix/noid/enfore-no-notnull-for-boolean-to-store-false

Conversation

@nickvergessen

@nickvergessen nickvergessen commented Nov 11, 2020

Copy link
Copy Markdown
Member

Fails until #24053 is in

Fix #24083

@rullzer

rullzer commented Nov 11, 2020

Copy link
Copy Markdown
Member

I get this is just how oracle works. But when you hae a boolean field that you can't store false is just somewhat insane.

@nickvergessen

Copy link
Copy Markdown
Member Author

But when you have a boolean field that you can't store false is just somewhat insane.

Well oracle has no boolean fields, doctrine maps it to an integer with length 1 byte. "You" manually tell it to not allow 0/null, so that's what we do wrapped by doctrine.

@nickvergessen nickvergessen changed the title Enfore no notnull for boolean to store false Enforce no notnull for boolean to store false Nov 11, 2020
@faily-bot

This comment has been minimized.

@rullzer

rullzer commented Nov 11, 2020

Copy link
Copy Markdown
Member

But when you have a boolean field that you can't store false is just somewhat insane.

Well oracle has no boolean fields, doctrine maps it to an integer with length 1 byte. "You" manually tell it to not allow 0/null, so that's what we do wrapped by doctrine.

That is somehow more confusing. If doctrine maps it takes an int. Null and 0 are not the same. So if you have a mandatory int field. You can't set it to 0?

@rullzer

rullzer commented Mar 30, 2021

Copy link
Copy Markdown
Member

Conflicts. BUta lso CI is not happy

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the bugfix/noid/enfore-no-notnull-for-boolean-to-store-false branch from 9ccd7f7 to f9d4fa2 Compare March 31, 2021 08:21
@nickvergessen

Copy link
Copy Markdown
Member Author

Rebased, lets see what CI says

@juliusknorr

juliusknorr commented Apr 1, 2021

Copy link
Copy Markdown
Member

Hm haven't seen those tests failing before so I'd assume that might be a related failure, but I cannot see how from the affected code: integration/sharing_features/sharing-v1-video-verification.feature I've retriggered drone once so let's see

@nickvergessen

Copy link
Copy Markdown
Member Author

yeah, talk does not install anymore :(
I will make a PR

@nickvergessen

Copy link
Copy Markdown
Member Author

Ref nextcloud/spreed#5445

@MorrisJobke MorrisJobke merged commit 5fb909f into master Apr 1, 2021
@MorrisJobke MorrisJobke deleted the bugfix/noid/enfore-no-notnull-for-boolean-to-store-false branch April 1, 2021 16:30
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Apr 7, 2021
@ChristophWurst

Copy link
Copy Markdown
Member

this should definitely go into our upgrade docs

@nickvergessen

Copy link
Copy Markdown
Member Author

Added to #26407 (comment)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve error message for not existing column

5 participants