PostgreSQL-For-Wordpress / postgresql-for-wordpress

A maintained fork of https://wordpress.org/plugins/postgresql-for-wordpress/
GNU General Public License v2.0
209 stars 68 forks source link

Fatal Error when browsing Site Health page #96

Closed SamTyurenkov closed 5 months ago

SamTyurenkov commented 5 months ago

environment: -wordpress 6.4.3 on roots bedrock -docker pg16.1, php8.1-fpm

[07-Feb-2024 14:43:18 UTC] PHP Warning:  Undefined property: PgSql\Connection::$client_info in /var/www/site/web/wp/wp-admin/includes/class-wp-debug-data.php on line 869
[07-Feb-2024 14:43:18 UTC] PHP Fatal error:  Uncaught Exception: Invalid or unsupported SQL statement. in /var/www/site/web/app/pg4wp/driver_pgsql_rewrite.php:26
Stack trace:
#0 /var/www/site/web/app/pg4wp/driver_pgsql_rewrite.php(40): createSQLRewriter('SHOW TABLE STAT...')
#1 /var/www/site/web/app/pg4wp/driver_pgsql.php(493): pg4wp_rewrite('SHOW TABLE STAT...')
#2 /var/www/site/web/app/pg4wp/core.php(34) : eval()'d code(2349): wpsqli_query(Object(PgSql\Connection), 'SHOW TABLE STAT...')
#3 /var/www/site/web/app/pg4wp/core.php(34) : eval()'d code(2263): wpdb2->_do_query('SHOW TABLE STAT...')
#4 /var/www/site/web/app/pg4wp/core.php(34) : eval()'d code(3146): wpdb2->query('SHOW TABLE STAT...')
#5 /var/www/site/web/wp/wp-admin/includes/class-wp-debug-data.php(1574): wpdb2->get_results('SHOW TABLE STAT...', 'ARRAY_A')
#6 /var/www/site/web/wp/wp-admin/includes/class-wp-debug-data.php(1594): WP_Debug_Data::get_database_size()
#7 /var/www/site/web/wp/wp-includes/rest-api/endpoints/class-wp-rest-site-health-controller.php(290): WP_Debug_Data::get_sizes()
#8 /var/www/site/web/wp/wp-includes/rest-api/class-wp-rest-server.php(1193): WP_REST_Site_Health_Controller->get_directory_sizes(Object(WP_REST_Request))
#9 /var/www/site/web/wp/wp-includes/rest-api/class-wp-rest-server.php(1041): WP_REST_Server->respond_to_request(Object(WP_REST_Request), '/wp-site-health...', Array, NULL)
#10 /var/www/site/web/wp/wp-includes/rest-api/class-wp-rest-server.php(431): WP_REST_Server->dispatch(Object(WP_REST_Request))
#11 /var/www/site/web/wp/wp-includes/rest-api.php(424): WP_REST_Server->serve_request('/wp-site-health...')
#12 /var/www/site/web/wp/wp-includes/class-wp-hook.php(324): rest_api_loaded(Object(WP))
#13 /var/www/site/web/wp/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters('ru', Array)
#14 /var/www/site/web/wp/wp-includes/plugin.php(565): WP_Hook->do_action(Array)
#15 /var/www/site/web/wp/wp-includes/class-wp.php(418): do_action_ref_array('parse_request', Array)
#16 /var/www/site/web/wp/wp-includes/class-wp.php(813): WP->parse_request('')
#17 /var/www/site/web/wp/wp-includes/functions.php(1336): WP->main('')
#18 /var/www/site/web/wp/wp-blog-header.php(16): wp()
#19 /var/www/site/web/index.php(7): require('/var/www/site/w...')
#20 {main}
  thrown in /var/www/site/web/app/pg4wp/driver_pgsql_rewrite.php on line 26

Full SQL command is SHOW TABLE STATUS

mattbucci commented 5 months ago

Can you post the full SQL command? It looks like it's https://dev.mysql.com/doc/refman/8.0/en/show-table-status.html which is indeed not handled.

Looks like we need to add a rewriter for that to emulate behavior. Per the docs it's similar to show tables which is already implemented but provides some additional info we need to emulate

SamTyurenkov commented 5 months ago

@mattbucci already not at pc, but its easily reproducable when visiting site health page in dashboard i think.

Maybe you can find the SQL in core files as per trace message: /wp-admin/includes/class-wp-debug-data.php(1574) and add the error_log just before it to get the SQL.

mattbucci commented 5 months ago

I visited the site health dashboard as part of https://github.com/PostgreSQL-For-Wordpress/postgresql-for-wordpress/pull/91

I did not see this command executed

SamTyurenkov commented 5 months ago

@mattbucci I really wont be at that project until Monday, but did you visit both tabs on site health page? maybe the second tab generates that SQL image

mattbucci commented 5 months ago

yes I visited both tabs

SamTyurenkov commented 5 months ago

updated original message, with full stack trace and full SQL command, which is just "SHOW TABLE STATUS" without anything else. it is called by core method

image

mattbucci commented 5 months ago

finally able to take a look at this.

Not seeing the issue with SHOW TABLE STATUS, but getting another error:

SELECT TABLE_NAME AS 'table', TABLE_ROWS AS 'rows', SUM(data_length + index_length) as 'bytes' FROM information_schema.TABLES WHERE TABLE_SCHEMA = 'wordpress' AND TABLE_NAME IN ('wp_comments','wp_options','wp_posts','wp_terms','wp_users') GROUP BY TABLE_NAME;

This has lots of problems as information_schema looks nothing like this.

TABLE_SCHEMA is being set to wordpress which is not correct, it should be public, or even better extracted to a constant which can be set. wordpress is my db name. Additionally it should be lowercase, table_schema TABLE_ROWS, data_length, index_length all don't exist

it should look like this I think

SELECT 
    C.relname AS "table", 
    S.n_live_tup AS "rows", 
    pg_total_relation_size(C.oid) AS "bytes"
FROM 
    pg_class C 
LEFT JOIN 
    pg_namespace N ON (N.oid = C.relnamespace) 
INNER JOIN 
    pg_stat_user_tables S ON (S.relid = C.oid) 
WHERE 
    N.nspname = 'public' AND 
    C.relname IN ('wp_comments','wp_options','wp_posts','wp_terms','wp_users')
GROUP BY 
    C.relname, pg_total_relation_size(C.oid), S.n_live_tup;
mattbucci commented 5 months ago

That comes from should_suggest_persistent_object_cache

not really a point in trying to fix the schema, better to just detect this query and rewrite the whole thing from scratch in the select rewriter, this also doesn't get triggered by multisite installs, only single sites

        // With InnoDB the `TABLE_ROWS` are estimates, which are accurate enough and faster to retrieve than individual `COUNT()` queries.
        $results = $wpdb->get_results(
            $wpdb->prepare(
                // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- This query cannot use interpolation.
                "SELECT TABLE_NAME AS 'table', TABLE_ROWS AS 'rows', SUM(data_length + index_length) as 'bytes' FROM information_schema.TABLES WHERE TABLE_SCHEMA = %s AND TABLE_NAME IN ('$table_names') GROUP BY TABLE_NAME;",
                DB_NAME
            ),
            OBJECT_K
        );
mattbucci commented 5 months ago

Ok, so as far as SHOW TABLE STATUS goes, there's a lot to emulate.

The function you referenced only includes data_length and index_length. Those are straightforward enough. Here's the full set though image

Name: The name of the table.
Engine: The storage engine for the table (MyISAM, InnoDB, etc.).
Version: The version number of the table's .frm file.
Row_format: The row storage format (Compact or Redundant) for InnoDB tables or the row storage format (Fixed, Dynamic, or Compressed) for MyISAM tables.
Rows: The number of rows. Some storage engines, like InnoDB, provide only an approximation.
Avg_row_length: The average row length.
Data_length: The length (bytes) of the data file (how much space the actual data takes).
Max_data_length: The maximum length of data file for the table.
Index_length: The length (bytes) of the index file (how much space the indexed data takes).
Data_free: The number of allocated but unused bytes.
Auto_increment: The next autoincrement value.
Create_time: When the table was created.
Update_time: When the data file was last updated.
Check_time: When the table was last checked.
Table_collation: The table's character set and collation.
Checksum: The live checksum or the last calculated checksum.
Create_options: Extra options that were specified with CREATE TABLE.
Comment: Any comment that was included in the COMMENT option when the table was created.

Here's the basic emulated query

SELECT 
    relname AS Name,
    pg_relation_size(relid) AS Data_length,
    pg_indexes_size(relid) AS Index_length
FROM 
    pg_stat_user_tables
ORDER BY 
    pg_total_relation_size(relid) DESC;

I will add this for now, with dummy values for other columns, but we should aim to improve emulation here

mattbucci commented 5 months ago

There is now a PR available if you'd like to test it: https://github.com/PostgreSQL-For-Wordpress/postgresql-for-wordpress/pull/100/files