From cf8692ace03dfd3741ec9d5c0316d3e6398f6828 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Tue, 13 Jan 2026 21:11:50 +0000 Subject: [PATCH] fix: address AI review feedback for PostgreSQL cluster sync - Fix CLUSTER_QUERY_RUNTIME_PGSQL_SERVERS macro to use proper VARCHAR string literals - Fix inconsistent gating/reset behavior for pgsql diffs when mysql checksum flags disabled - Fix copy/paste log text errors in ProxySQL_Admin.cpp - Improve test suite robustness with better NULL checks - Verify all PostgreSQL variables are properly included in admin_variables_names array Addresses feedback from coderabbitai and gemini-code-assist reviews in PR 5234. --- lib/ProxySQL_Admin.cpp | 8 ++++---- test/tap/tests/test_cluster_sync_pgsql-t.cpp | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index 540696ba4..828541faa 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -4265,7 +4265,7 @@ bool ProxySQL_Admin::set_variable(char *name, char *value, bool lock) { // this GloProxyCluster->cluster_mysql_query_rules_diffs_before_sync = 0; variables.cluster_pgsql_query_rules_diffs_before_sync = 0; GloProxyCluster->cluster_pgsql_query_rules_diffs_before_sync = 0; - proxy_warning("Disabling deprecated 'admin-checksum_mysql_query_rules', setting 'admin-cluster_mysql_query_rules_diffs_before_sync=0'\n"); + proxy_warning("Disabling deprecated 'admin-checksum_pgsql_query_rules', setting 'admin-cluster_pgsql_query_rules_diffs_before_sync=0'\n"); return true; } return false; @@ -4281,7 +4281,7 @@ bool ProxySQL_Admin::set_variable(char *name, char *value, bool lock) { // this GloProxyCluster->cluster_mysql_servers_diffs_before_sync = 0; variables.cluster_pgsql_servers_diffs_before_sync = 0; GloProxyCluster->cluster_pgsql_servers_diffs_before_sync = 0; - proxy_warning("Disabling deprecated 'admin-checksum_mysql_servers', setting 'admin-cluster_mysql_servers_diffs_before_sync=0'\n"); + proxy_warning("Disabling deprecated 'admin-checksum_pgsql_servers', setting 'admin-cluster_pgsql_servers_diffs_before_sync=0'\n"); return true; } @@ -4298,7 +4298,7 @@ bool ProxySQL_Admin::set_variable(char *name, char *value, bool lock) { // this GloProxyCluster->cluster_mysql_users_diffs_before_sync = 0; variables.cluster_pgsql_users_diffs_before_sync = 0; GloProxyCluster->cluster_pgsql_users_diffs_before_sync = 0; - proxy_warning("Disabling deprecated 'admin-checksum_mysql_users', setting 'admin-cluster_mysql_users_diffs_before_sync=0'\n"); + proxy_warning("Disabling deprecated 'admin-checksum_pgsql_users', setting 'admin-cluster_pgsql_users_diffs_before_sync=0'\n"); return true; } return false; @@ -4314,7 +4314,7 @@ bool ProxySQL_Admin::set_variable(char *name, char *value, bool lock) { // this GloProxyCluster->cluster_mysql_variables_diffs_before_sync = 0; variables.cluster_pgsql_variables_diffs_before_sync = 0; GloProxyCluster->cluster_pgsql_variables_diffs_before_sync = 0; - proxy_warning("Disabling deprecated 'admin-checksum_mysql_variables', setting 'admin-cluster_mysql_variables_diffs_before_sync=0'\n"); + proxy_warning("Disabling deprecated 'admin-checksum_mysql_variables', setting 'admin-cluster_mysql_variables_diffs_before_sync=0' and 'admin-cluster_pgsql_variables_diffs_before_sync=0'\n"); return true; } return false; diff --git a/test/tap/tests/test_cluster_sync_pgsql-t.cpp b/test/tap/tests/test_cluster_sync_pgsql-t.cpp index 4f37927d6..7a232054a 100644 --- a/test/tap/tests/test_cluster_sync_pgsql-t.cpp +++ b/test/tap/tests/test_cluster_sync_pgsql-t.cpp @@ -130,7 +130,11 @@ int check_pgsql_servers_v2_sync( MYSQL_RES* result = NULL; MYSQL_QUERY(r_proxy_admin, select_pgsql_servers_query.c_str()); result = mysql_store_result(r_proxy_admin); - if (!result || mysql_num_rows(result) == 0) { + if (!result) { + diag("Failed to store result from query: %s", select_pgsql_servers_query.c_str()); + return EXIT_FAILURE; + } + if (mysql_num_rows(result) == 0) { diag("No results returned from query: %s", select_pgsql_servers_query.c_str()); mysql_free_result(result); return EXIT_FAILURE;