From 2987242d4fb6e2246402909d1cd58e68ee35df96 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 14 Dec 2025 05:42:38 +0000 Subject: [PATCH] Fix cache_empty_result=0 not caching non-empty resultsets (issue #5248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `cache_empty_result` field in query rules has three possible values: • -1: Use global setting (`query_cache_stores_empty_result`) • 0: Do NOT cache empty resultsets, but cache non-empty resultsets • 1: Always cache resultsets (both empty and non-empty) Previously, when `cache_empty_result` was set to 0, nothing was cached at all, even for non-empty resultsets. This prevented users from disabling caching for empty resultsets while still allowing caching of non-empty resultsets on a per-rule basis. Changes: 1. Modified caching logic in MySQL_Session.cpp and PgSQL_Session.cpp to add the condition `(qpo->cache_empty_result == 0 && MyRS->num_rows)` (MySQL) and `(qpo->cache_empty_result == 0 && num_rows)` (PgSQL) to allow caching when cache_empty_result=0 AND result has rows. 2. Added comprehensive Doxygen documentation in query_processor.h explaining the semantics of cache_empty_result values. 3. Updated Query_Processor.cpp with inline comments explaining the three possible values. Now when cache_empty_result is set to 0: - Empty resultsets (0 rows) are NOT cached - Non-empty resultsets (>0 rows) ARE cached - This matches the intended per-rule behavior described in issue #5248. Fixes: https://github.com/sysown/proxysql/issues/5248 --- include/query_processor.h | 20 ++++++++++++++++++++ lib/MySQL_Session.cpp | 17 ++++++++++++++++- lib/PgSQL_Session.cpp | 19 +++++++++++++++++-- lib/Query_Processor.cpp | 4 ++++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/include/query_processor.h b/include/query_processor.h index d9a4bd075..bdcf1e6cd 100644 --- a/include/query_processor.h +++ b/include/query_processor.h @@ -104,6 +104,16 @@ typedef struct _Query_Processor_rule_t { char *replace_pattern; int destination_hostgroup; int cache_ttl; + /** @brief Controls caching of empty resultsets for query rules + * + * Values: + * - -1: Use global setting (query_cache_stores_empty_result) + * - 0: Do NOT cache empty resultsets, but cache non-empty resultsets + * - 1: Always cache resultsets (both empty and non-empty) + * + * Previously, setting cache_empty_result to 0 would prevent ALL caching, + * even for non-empty resultsets. This was fixed in issue #5248. + */ int cache_empty_result; int cache_timeout; int reconnect; @@ -139,6 +149,16 @@ class Query_Processor_Output { int mirror_flagOUT; int next_query_flagIN; int cache_ttl; + /** @brief Controls caching of empty resultsets for query rule output + * + * Values: + * - -1: Use global setting (query_cache_stores_empty_result) + * - 0: Do NOT cache empty resultsets, but cache non-empty resultsets + * - 1: Always cache resultsets (both empty and non-empty) + * + * Previously, setting cache_empty_result to 0 would prevent ALL caching, + * even for non-empty resultsets. This was fixed in issue #5248. + */ int cache_empty_result; int cache_timeout; int reconnect; diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index 4b9367ad2..f3634456e 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -7490,10 +7490,25 @@ void MySQL_Session::MySQL_Result_to_MySQL_wire(MYSQL *mysql, MySQL_ResultSet *My if (transfer_started==false) { // we have all the resultset when MySQL_Result_to_MySQL_wire was called if (qpo && qpo->cache_ttl>0 && com_field_list==false) { // the resultset should be cached if (mysql_errno(mysql)==0 && - (mysql_warning_count(mysql)==0 || + (mysql_warning_count(mysql)==0 || mysql_thread___query_cache_handle_warnings==1)) { // no errors + /** + * @brief Check if the query result should be cached based on cache_empty_result setting + * + * The cache_empty_result field in query rule has three possible values: + * - 1: Always cache the result, regardless of whether it's empty or not + * - 0: Cache only non-empty results (num_rows > 0). Empty resultsets are not cached. + * - -1: Use global setting (thread->variables.query_cache_stores_empty_result) + * OR cache if result is non-empty (num_rows > 0) + * + * Previously, when cache_empty_result was set to 0, nothing was cached at all. + * This fix adds support for caching non-empty results when cache_empty_result=0. + * + * @see Issue #5248: Setting cache_empty_result to "0" on individual mysql_query_rules doesn't work + */ if ( (qpo->cache_empty_result==1) + || (qpo->cache_empty_result == 0 && MyRS->num_rows) || ( (qpo->cache_empty_result == -1) && diff --git a/lib/PgSQL_Session.cpp b/lib/PgSQL_Session.cpp index b589c5209..6104bda1c 100644 --- a/lib/PgSQL_Session.cpp +++ b/lib/PgSQL_Session.cpp @@ -4809,11 +4809,26 @@ void PgSQL_Session::PgSQL_Result_to_PgSQL_wire(PgSQL_Connection* _conn, PgSQL_Da if (qpo && qpo->cache_ttl > 0 && is_tuple == true) { // the resultset should be cached if (_conn->is_error_present() == false && - (/* check warnings count here*/ true || + (/* check warnings count here*/ true || pgsql_thread___query_cache_handle_warnings == 1)) { // no errors + /** + * @brief Check if the query result should be cached based on cache_empty_result setting + * + * The cache_empty_result field in query rule has three possible values: + * - 1: Always cache the result, regardless of whether it's empty or not + * - 0: Cache only non-empty results (num_rows > 0). Empty resultsets are not cached. + * - -1: Use global setting (thread->variables.query_cache_stores_empty_result) + * OR cache if result is non-empty (num_rows > 0) + * + * Previously, when cache_empty_result was set to 0, nothing was cached at all. + * This fix adds support for caching non-empty results when cache_empty_result=0. + * + * @see Issue #5248: Setting cache_empty_result to "0" on individual mysql_query_rules doesn't work + */ if ( - (qpo->cache_empty_result == 1) || + (qpo->cache_empty_result == 1) || + (qpo->cache_empty_result == 0 && num_rows) || ( (qpo->cache_empty_result == -1) && (thread->variables.query_cache_stores_empty_result || num_rows) diff --git a/lib/Query_Processor.cpp b/lib/Query_Processor.cpp index c044d0390..685b42199 100644 --- a/lib/Query_Processor.cpp +++ b/lib/Query_Processor.cpp @@ -1559,6 +1559,10 @@ __internal_loop: } if (qr->cache_empty_result >= 0) { // Note: negative value means this rule doesn't change + // cache_empty_result values: + // -1: Use global setting (query_cache_stores_empty_result) + // 0: Do NOT cache empty resultsets, but cache non-empty resultsets + // 1: Always cache resultsets (both empty and non-empty) proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 5, "query rule %d has set cache_empty_result: %d. Query with empty result will%s hit the cache\n", qr->rule_id, qr->cache_empty_result, (qr->cache_empty_result == 0 ? " NOT" : "" )); ret->cache_empty_result=qr->cache_empty_result; }