Make WordPress Core

Opened 2 years ago

Last modified 5 days ago

#59836 new defect (bug)

Using the pre_get_table_charset filter ensures wpdb->col_meta never gets populated

Reported by: c0ntax's profile c0ntax Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.3.3
Component: Database Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

I have a non-core table with a binary column that I use for selects and deletes (it's an efficient way to store UUIDs). I've noticed that whenever I attempt to do select from it, WP does a relatively expensive SHOW FULL COLUMNS FROM query, which seems to be uncachable. I've been attempting to use the pre_get_table_charset filter to get around this, but this then caused problems when get_col_charset() runs in the same call. It seems that inside get_table_charset() there is the line $this->col_meta[ $tablekey ] = $columns; which appears to be the only place that col_meta is ever populated. But it wont be populated for that table if you use the hook, so if something like this then runs:

 // If still no column information, return the table charset. if ( empty( $this->col_meta[ $tablekey ] ) ) { return $this->table_charset[ $tablekey ]; } 

as it does in get_col_charset() then things just break. col_meta for that table is not populated and neither is table_charset. I'm too stupid to work out the best way to fix this I'm afraid.

Change History (4)

#1 in reply to: ↑ description @c0ntax
2 years ago

The following in get_col_length() also seems to make the same assumption that running get_table_charset() will ensure that col_meta is populated:

 if ( empty( $this->col_meta[ $tablekey ] ) ) { // This primes column information for us. $table_charset = $this->get_table_charset( $table ); if ( is_wp_error( $table_charset ) ) { return $table_charset; } } if ( empty( $this->col_meta[ $tablekey ][ $columnkey ] ) ) { return false; } 

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 years ago

#3 @matt-h
5 days ago

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

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.

#4 @matt-h
5 days ago

  • Keywords has-patch has-unit-tests added
Note: See TracTickets for help on using tickets.