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

Do not use quotes to represent a boolean default #5108

Merged
merged 1 commit into from
Dec 12, 2021

Conversation

morozov
Copy link
Member

@morozov morozov commented Dec 11, 2021

Q A
Type improvement
BC Break no

Representing boolean default as a string literal may cause false-positive diffs during platform-aware schema comparison if the platform doesn't support boolean columns natively (IBM DB2, Oracle, MySQL) and doesn't implement any hacks for introspection of boolean columns:

'tinyint' => 'boolean',

'pls_integer' => 'boolean',

See #5107 (comment) for more details.

The existing assertions that expect SQL like TINYINT(1) DEFAULT '0' are obviously wrong and were likely implemented by copy/pasting the actual value as expected.

According to the functional tests, there are no cases where a boolean default would have to be represented as a string literal. All supported platforms use integer 0 and 1 to represent boolean values. PostgreSQL supports booleans literals true/false.

Note that the documentation for PostgreSQL 9.3 is the last version that mentions the support for quoted 'true' and 'false' as valid boolean literals.

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

The tests with a string default value still work. If there's no test using a boolean default value DEFAULT true, we might need one.

@morozov
Copy link
Member Author

morozov commented Dec 12, 2021

If there's no test using a boolean default value DEFAULT true, we might need one.

I checked that the following test passes with true as well and generates the expected table DDL:

public function testBooleanDefault(callable $comparatorFactory): void
{
$table = new Table('ddc2843_bools');
$table->addColumn('id', 'integer');
$table->addColumn('checked', 'boolean', ['default' => false]);

We can update the test separately so it doesn't block merging the patch.

@morozov morozov merged commit cca4836 into doctrine:3.3.x Dec 12, 2021
@morozov morozov deleted the boolean-default-as-number branch December 12, 2021 17:06
@morozov morozov added this to the 3.3.0 milestone Jun 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants