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.