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

Fix default null values and NULL value update edge case #94

Merged
merged 6 commits into from
Apr 15, 2024

Conversation

bgrgicak
Copy link
Collaborator

@bgrgicak bgrgicak commented Apr 11, 2024

This PR allows SQL to set DEFAULT NULL which currently isn't supported because the code is built as DEFAULT (empty value instead of NULL).

It also adds support for updating a field to NULL that is NOT NULL and doesn't have a DEFAULT value.
The result of the update should be an empty value ('' for text and 0 for numbers).

Testing instructions

  • confirm that all tests pass

@bgrgicak bgrgicak force-pushed the fix/default-and-not-null-edge-cases branch from 2d9bf95 to 99b0c6c Compare April 15, 2024 08:01
Comment on lines 1139 to 1143
if ('text' === $field->sqlite_data_type) {
$definition .= ' DEFAULT \'\'';
} else {
$definition .= ' DEFAULT 0';
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@adamziel what do you think about this solution?
Instead of rewriting update queries with COALESCE we could always set a default.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This will allow both inserting NULL values and updating existing rows to NULL values. MySQL rejects inserting NULL values and only allows updating. I think it's fine for now, let's just create a new issue to track rejecting inserts in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#97

@bgrgicak bgrgicak marked this pull request as ready for review April 15, 2024 09:39
@bgrgicak
Copy link
Collaborator Author

This PR should fix an issue with WPDB support where WordPress updates meta_value to NULL and results in ''. (meta_value is NOT NULL and doesn't have a default value)

@aristath
Copy link
Member

Released v2.1.9 with this patch 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants