Defining filter pre_get_table_charset causes errors

[haoran]

In wp-db.php::get_table_charset(), a filter is defined, pre_get_table_charset.

However, if this filter is defined, wp-db.php::get_col_charset() throws errors. This is because the latter assumes that it can find cached results cached in:

$this->table_charset[];

The assumption seems to be that if you add the pre_get_table_charset filter, you will also define the pre_get_col_charset filter.

Code reference: ​https://github.com/WordPress/WordPress/blob/master/wp-includes/wp-db.php#L2437

Note: defining this filter lets you hard-code the charset of tables, rather than doing a database lookup every time. This can be done quite safely because the most recent upgrade forced our DB tables to be utf8mb4.

[pento]

Thanks for the bug report @haoran!

We can definitely get this fixed up. :-)

[cjbj]

I can confirm this bug. We discussed it in more detail on wpse: ​http://wordpress.stackexchange.com/q/247109/75495

[birgire]

In 38921.diff​ there's a suggestion how to set $this->table_charset, when the the table charset is short-curcuited with the pre_get_table_charset filter.

There's an existing test for the wpdb::get_table_charset() method, with the pre_get_table_charset filter, in (Source):

Tests_DB::test_pre_get_table_charset_filter()

but it doesn't test it with the wpdb::get_col_charset() method as well.

38921.diff​ extends that test with a new one in:

Tests_DB_Charset::test_get_table_and_column_charset_with_pre_get_table_charset_filter()

[kovshenin]

Yeah, definitely a bug.

I have a table with some ascii columns and a couple of utf8mb4 columns, and the default wpdb behavior treats my entire table as ascii, so I get the CONVERT(CONVERT()) bullet when inserting into my utf8mb4 column via strip_invalid_text(). That pre_get_table_charset filter's definitely a life saver for me and did exactly what I expected it to do, though I was a bit sad about a whole bunch of warnings from get_col_charset().

A workaround is to filter pre_get_col_charset as well.

The patch from @birgire looks good though.

Change how column meta is pulled so it uses a get_cols_meta and get_col_meta function.

This is to consolidate the database request so it has a single source of truth.
It previously used a cached values array throughout the code which relied on get_table_charset being run to prime the cache. The priming of the cache was a side effect of that function.
If the pre_get_table_charset was set then the cache would never be primed and cache checks would throw errors.
We now always get the values from the functions which can be cached but not rely on the cache being primed.

Trac ticket: https://core.trac.wordpress.org/ticket/38921
and https://core.trac.wordpress.org/ticket/59836

[matt-h]

I've created a patch for this ​https://github.com/WordPress/wordpress-develop/pull/10646