WordPress 6.7 Compatibility: Avoid errors in the database where a TEXT type can't have a default value

[eason9487]

Changes proposed in this Pull Request:

There are database errors when running PHP Unit Tests with WordPress 6.7-RC3.

WordPress database error BLOB, TEXT, GEOMETRY or JSON column 'categories' can't have a default value for query
ALTER TABLE wptests_gla_attribute_mapping_rules ALTER COLUMN `categories` SET DEFAULT ''
made by include('phpvfscomposer:///home/runner/work/google-listings-and-ads/google-listings-and-ads/vendor/phpunit/phpunit/phpunit'),
PHPUnit\TextUI\Command::main,
PHPUnit\TextUI\Command->run,
PHPUnit\TextUI\TestRunner->run,
PHPUnit\Framework\TestSuite->run,
PHPUnit\Framework\TestSuite->run,
PHPUnit\Framework\TestSuite->run,
PHPUnit\Framework\TestCase->run,
PHPUnit\Framework\TestResult->run,
PHPUnit\Framework\TestCase->runBare,
Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\Integration\WPCOMProxyTest->setUp,
Automattic\WooCommerce\GoogleListingsAndAds\DB\Table->install,
Automattic\WooCommerce\GoogleListingsAndAds\Proxies\WP->db_delta,
dbDelta

Considering the null categories will be sanitized to '' before inserting it into database, it may not be an issue to change the categories column in the gla_attribute_mapping_rules table to:

• No default value
• Not allow null
• No migration for existing tables and data, as they should be compatible on this premise.

google-listings-and-ads/src/DB/Query/AttributeMappingRulesQuery.php

Lines 42 to 44 in 4535b71

	if ( $column === 'categories' && is_null( $value ) ) {
	return '';
	}

Screenshots:

📷 Before

📷 After

Detailed test instructions:

1. Alter the table if needed
   • Before fix

     ALTER TABLE `wp_gla_attribute_mapping_rules` MODIFY COLUMN `categories` text DEFAULT '';

   • After fix

     ALTER TABLE `wp_gla_attribute_mapping_rules` MODIFY COLUMN `categories` text NOT NULL;

2. Go to the plugin's Attributes page.
3. Create and update some attribute rules to see if its CRUD work well.
4. View the runs of PHP Unit Tests on GitHub Actions to confirm the errors no longer occur.
   • https://github.com/woocommerce/google-listings-and-ads/actions/runs/11773996882/job/32791822305?pr=2667

Changelog entry

Tweak - WordPress 6.7 Compatibility: Avoid errors in the database where a TEXT type can't have a default value.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.4%. Comparing base (0383264) to head (a29a031).
Report is 4 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##             develop   #2667      +/-   ##
============================================
+ Coverage       62.7%   65.4%    +2.8%     
- Complexity         0    4596    +4596     
============================================
  Files            319     474     +155     
  Lines           5074   19268   +14194     
  Branches        1231       0    -1231     
============================================
+ Hits            3180   12610    +9430     
- Misses          1721    6658    +4937     
+ Partials         173       0     -173     

Flag	Coverage Δ
js-unit-tests	?
php-unit-tests	65.4% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/DB/Table/AttributeMappingRulesTable.php	46.7% <ø> (ø)

... and 792 files with indirect coverage changes

[mikkamp]

Thanks for fixing this issue. Although can you clarify how you are running into the error?

I'm not able to reproduce it locally as I run a newer version of MariaDB which supports defaults for text columns. Checking on some of the GH action runs it also seems like it's not running into the issue.

Like in this run here: https://github.com/woocommerce/google-listings-and-ads/actions/runs/11701884828/job/32588901868

It's using WP6.7 RC3:

download https://wordpress.org/wordpress-6.7-RC3.tar.gz /tmp/wordpress.tar.gz

But it's also using a newer version of MySQL (which supports defaults for text columns):

mysql  Ver 8.0.39-0ubuntu0.22.04.1 for Linux on x86_64 ((Ubuntu))

So if the error is coming from MySQL I'm just wondering how this is specifically related to WP 6.7 and wouldn't occur in older versions of WP? I'm also not finding anything in WP 6.7 that made these checks stricter.

Anyways I am approving the PR as it would allow it to support older MySQL versions (not recommended but still supported).

I tested the PR with the change, and as you mentioned the categories column is always passed as an empty string so there are no compatibility problems.

[eason9487]

Hi @mikkamp, thanks for the review!

Although can you clarify how you are running into the error?

Originally, I haven't reproduced the issue locally either. After changing my local DB to MySQL 8.0.39, I can reproduce it.

So if the error is coming from MySQL I'm just wondering how this is specifically related to WP 6.7 and wouldn't occur in older versions of WP? I'm also not finding anything in WP 6.7 that made these checks stricter.

It was firstly noticed on GitHub Actions. After testing with different combinations of WP and WC versions, the error occurs when using WordPress 6.7-RC3. It won't make the testing fail but only output error messages: https://github.com/woocommerce/google-listings-and-ads/actions/runs/11701884828/job/32588901868#step:6:29

After further research, it appears that WordPress 6.7 changed wp-admin/includes/upgrade.ph to strict comparison, which might make this comparison produce a different result.

CREATE TABLE `wp_gla_attribute_mapping_rules` (
    `id` bigint(20) NOT NULL AUTO_INCREMENT,
    `attribute` varchar(255) NOT NULL,
    `source` varchar(100) NOT NULL,
    `category_condition_type` varchar(10) NOT NULL,
    `categories` text DEFAULT '',
    PRIMARY KEY `id` (`id`)
);

In the original loose comparison, the logical comparison matched from the DEFAULT '' statement in the above table creation SQL will be NULL != '', changing it to a strict comparison produces a different result, which is then later executed:

ALTER TABLE wp_gla_attribute_mapping_rules ALTER COLUMN `categories` SET DEFAULT '';

Therefore, I believe this PR could be categorized as a WordPress 6.7 compatibility adjustment.

[mikkamp]

Thanks for the clarification, that makes sense. I was looking for actual failures in the test runs, but I can see the output mixed in with the test results now.