Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fatal Error when browsing Site Health page #96

Closed
SamTyurenkov opened this issue Feb 2, 2024 · 10 comments · Fixed by #100
Closed

Fatal Error when browsing Site Health page #96

SamTyurenkov opened this issue Feb 2, 2024 · 10 comments · Fixed by #100

Comments

@SamTyurenkov
Copy link

SamTyurenkov commented Feb 2, 2024

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
Copy link
Collaborator

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
Copy link
Author

@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
Copy link
Collaborator

I visited the site health dashboard as part of #91

I did not see this command executed

@SamTyurenkov
Copy link
Author

@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
Copy link
Collaborator

yes I visited both tabs

@SamTyurenkov
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Collaborator

mattbucci commented Feb 21, 2024

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
Copy link
Collaborator

mattbucci commented Feb 21, 2024

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants