[#28944] YSQL: Set the default value of auto analyze flag based on user set cbo flags

Summary:
Users currently have to manage cbo and auto analyze flags separately and can get into bad combinations. We do not want to enable auto analyze when cbo is in legacy mode (or anything other than on) because ANALYZE can update the `reltuples` field in pg_class for relations and cause unpredictable behavior. We intend to have the ysql_yb_enable_cbo flag default to false in code and be explicitly enabled by YBA/yugabyted for new clusters in 2025.2.

This diff (intended for backport to 2025.2) changes the auto analyze flag defaults to match this situation.

1.0 If the user explicits sets ysql_enable_auto_analyze, that setting is always used directly.
2.0 The default value of the flag ysql_enable_auto_analyze in the code is `false`, however it can be automatically turned on to true in the following cases:
2.1 ysql_yb_enable_cbo PG runtime gflag is set to `on` (currently this flag defaults to `legacy_mode` in code)
2.2 ysql_pg_conf_csv contains `yb_enable_cbo=on` (currently this GUC defaults to `legacy_mode` in code)
2.3 (logic prior to this change): both the deprecated auto analyze flags are set explictly by the user `--ysql_enable_auto_analyze_service=true` && `--ysql_enable_table_mutation_counter=true`

In all other cases, the default value of the flag (false) is used unchanged. For special cases, including precedence order when both ysql_yb_enable_cbo and ysql_pg_conf_csv are set, see Notes below.

Notes
1.  When we change the default GUC yb_enable_cbo or PG flag ysql_yb_enable_cbo to "on" in code in the future, we will also need to update ysql_enable_auto_analyze default in the code to true and possibly invert this logic if we want to disable it for older clusters with cbo in legacy mode. Added a DCHECK to catch this case.
2. `yb_enable_cbo` can be configured on a per-db/role basis as any other GUC. However, that case isn't handled here. The user can set the GUC `yb_disable_auto_analyze` to match this GUC on the corresponding db/roles manually in such cases.
3. GUCs are case insensitive both in name and value.
4. Precedence order and repeated values
4.1 If ysql_yb_enable_cbo is set explicitly, its value is the one that is used in postgres config, so that is the value used to determine the auto analyze setting (set to true if cbo is on/true/yes/1, false otherwise). In this case, any setting for yb_enable_cbo in pg_conf_csv is not considered.
4.2 Repeated values for ysql_yb_enable_cbo like `ysql_yb_enable_cbo=on,ysql_yb_enable_cbo=off` are parsed by the gflags library to prefer the last setting.
4.3 ysql_pg_conf_csv can technically include multiple settings for `ysql_yb_enable_cbo` like `ysql_pg_conf_csv={yb_enaBle_cbo=off,yb_enAble_cbo=oN}`. In this case, the last value wins.
Jira: DB-18673

Test Plan:
Jenkins

```
./yb_build.sh --java-test TestPgRegressThirdPartyExtensionsHypopg
```

Backport-through: 2025.2

Tested the following combinations manually using yugabyted.

| **tserver flags** | **ysql_enable_auto_analyze** | **show yb_enable_cbo** |
| --tserver_flags not set | false (default) | legacy_mode |
| --tserver_flags=ysql_yb_enable_cbo=on | true | on |
| --tserver_flags=ysql_yb_enable_cbo=legacy_mode/off | false (default) | legacy_mode/off |
| --tserver_flags="ysql_pg_conf_csv={yb_enable_cbo=on/yes/true/1}" | true | on |
| --tserver_flags="ysql_pg_conf_csv={yb_enaBle_cbo=off,yb_enAble_cbo=oN}" | true | on |
| --tserver_flags="ysql_pg_conf_csv={yb_enable_cbo=legacy_mode}" | false (default) | legacy_mode |
| --tserver_flags="ysql_yb_enable_cbo=on,ysql_pg_conf_csv={yb_enable_cbo=off}" | true | on |
| --tserver_flags="ysql_yb_enable_cbo=off,ysql_pg_conf_csv={yb_enable_cbo=on}" | false (default) | off |
| --tserver_flags='"ysql_yb_enable_cbo=on,ysql_yb_enable_cbo=legacy_bnl_mode,ysql_pg_conf_csv={yb_enable_cbo=off,yb_enable_cbo=on}" | false (default) | legacy_bnl_mode

Test script

```
rm /tmp/out; cat /tmp/examples | while read flags; do echo $flags >> /tmp/out; bin/yugabyted stop && sleep 1 && bin/yugabyted start --listen 127.0.0.3 --tserver_flags="$flags" &&  grep 'server_main_util.*enable_auto_analyze' ~/var/logs/tserver/yb-tserver.INFO | tee -a /tmp/out && sleep 15 && bin/ysqlsh -h 127.0.0.3 -c "show yb_enable_cbo" | tee -a /tmp/out ; echo "---" >> /tmp/out;  done
```

```
 $ cat /tmp/examples
ysql_yb_enable_cbo=on
ysql_yb_enable_cbo=legacy_mode
ysql_pg_conf_csv={yb_enable_cbo=TRue}
ysql_pg_conf_csv={yb_enable_cbo=on}
ysql_pg_conf_csv={yb_enaBle_cbo=off,yb_enAble_cbo=oN}
ysql_pg_conf_csv={yb_enable_cbo=legacy_mode}
ysql_yb_enable_cbo=on,ysql_pg_conf_csv={yb_enable_cbo=off}
ysql_yb_enable_cbo=off,ysql_pg_conf_csv={yb_enable_cbo=on}
ysql_yb_enable_cbo=on,ysql_yb_enable_cbo=legacy_bnl_mode,ysql_pg_conf_csv={yb_enable_cbo=off,yb_enable_cbo=on}
```

Close: #28944

Reviewers: yguan, mtakahara, gkukreja, sanketh

Reviewed By: yguan, mtakahara

Subscribers: jason, hsunder, yql, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D47676

Author: iSignal

java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressThirdPartyExtensionsHypopg.java

@@ -22,6 +22,8 @@
 import java.util.Collections;
 import java.util.Map;
 
+import static org.yb.AssertionWrappers.*;
+
 @RunWith(value=YBTestRunner.class)
 public class TestPgRegressThirdPartyExtensionsHypopg extends BasePgRegressTest {
   @Override
@@ -40,6 +42,13 @@ public void schedule_old_cost_model() throws Exception {
     Map<String, String> flagMap = super.getTServerFlags();
     flagMap.put("ysql_pg_conf_csv", "yb_enable_cbo=OFF");
     restartClusterWithFlags(Collections.emptyMap(), flagMap);
+
+    String enable_auto_analyze = miniCluster.getClient().getFlag(
+        miniCluster.getTabletServers().keySet().iterator().next(),
+        "ysql_enable_auto_analyze");
+    assertTrue("ysql_enable_auto_analyze should remain at default value false",
+        enable_auto_analyze.equals("false"));
+
     runPgRegressTest(new File(TestUtils.getBuildRootDir(),
                               "postgres_build/third-party-extensions/hypopg"),
                      "yb_old_cost_model_schedule");
@@ -50,6 +59,13 @@ public void schedule() throws Exception {
     Map<String, String> flagMap = super.getTServerFlags();
     flagMap.put("ysql_pg_conf_csv", "yb_enable_cbo=ON");
     restartClusterWithFlags(Collections.emptyMap(), flagMap);
+
+    String enable_auto_analyze = miniCluster.getClient().getFlag(
+        miniCluster.getTabletServers().keySet().iterator().next(),
+        "ysql_enable_auto_analyze");
+    assertTrue("ysql_enable_auto_analyze should be auto set to true",
+        enable_auto_analyze.equals("true"));
+
     runPgRegressTest(new File(TestUtils.getBuildRootDir(),
                               "postgres_build/third-party-extensions/hypopg"),
                      "yb_schedule");

src/yb/tserver/server_main_util.cc

@@ -13,7 +13,10 @@
 
 #include "yb/tserver/server_main_util.h"
 
+#include <algorithm>
 #include <iostream>
+#include <boost/algorithm/string/trim.hpp>
+#include "yb/util/string_case.h"
 
 #if YB_ABSL_ENABLED
 #include "absl/debugging/symbolize.h"
@@ -31,6 +34,7 @@
 #include "yb/util/debug/trace_event.h"
 #include "yb/util/flags.h"
 #include "yb/util/mem_tracker.h"
+#include "yb/util/pg_util.h"
 #include "yb/util/size_literals.h"
 #include "yb/util/status.h"
 
@@ -49,6 +53,8 @@ DECLARE_int64(memory_limit_hard_bytes);
 DECLARE_bool(ysql_enable_auto_analyze);
 DECLARE_bool(ysql_enable_auto_analyze_service);
 DECLARE_bool(ysql_enable_table_mutation_counter);
+DECLARE_string(ysql_pg_conf_csv);
+DECLARE_string(ysql_yb_enable_cbo);
 
 namespace yb {
 
@@ -125,40 +131,106 @@ void AdjustMemoryLimitsIfNeeded(bool is_master) {
   AdjustMemoryLimits(values);
 }
 
-// If auto analyze service is used based on old deprecated flags,
-// adjust new flags' values based on the deprecated flags to make auto analyze
-// service behaves consistently.
-// When using deprecated flags, only setting both FLAGS_ysql_enable_auto_analyze
-// and FLAGS_ysql_enable_table_mutation_counter makes auto analyze service runable,
-// so ysql_enable_auto_analyze is set in the following precedence order
-//    1. Any explicit user set value for ysql_enable_auto_analyze.
-//    2. If not set explicitly,
-//          2.1 If ysql_enable_auto_analyze_service and ysql_enable_table_mutation_counter are set
-//              explicitly. Use their values to set value for ysql_enable_auto_analyze.
-//              ysql_enable_auto_analyze is true only if both ysql_enable_auto_analyze_service and
-//              ysql_enable_table_mutation_counter are true.
-//          2.2 If not both of ysql_enable_auto_analyze_service and
-//              ysql_enable_table_mutation_counter are set explicity,
-//              use ysql_enable_auto_analyze default value.
+// Return the lower case value of a GUC set in a CSV string (FLAGS_ysql_pg_conf_csv),
+// accounting for the following
+// 1. GUC keys and values are case insensitive
+// 2. A GUC can be repeated multiple times, the final value is what matters
+//    (consistent with WritePostgresConfig)
+std::optional<std::string> FindGUCValue(
+    const std::string& guc_key, const std::vector<std::string>& user_configs) {
+  std::optional<std::string> result;
+  for (const auto& config : user_configs) {
+    const auto lower_case_config = yb::ToLowerCase(config);
+    if (lower_case_config.find(guc_key) == std::string::npos) {
+      continue;
+    }
+    const auto pos = lower_case_config.find('=');
+    if (pos != std::string::npos) {
+      std::string config_key = boost::trim_copy(lower_case_config.substr(0, pos));
+      std::string config_value = boost::trim_copy(lower_case_config.substr(pos + 1));
+      if (config_key == guc_key) {
+        result = config_value;
+      }
+    }
+  }
+  return result;
+}
+
+// Auto analyze flag can have different defaults based on other flags like
+// older (deprecated) auto analyze flags or the cbo flags. In any case, an explict
+// user set flag value should be respected as-is.
 void AdjustAutoAnalyzeFlagIfNeeded() {
-  // Check if the flag is explictly set.
-  if (!google::GetCommandLineFlagInfoOrDie("ysql_enable_auto_analyze").is_default)
+  // If the auto analyze flag is explictly set, do not change it.
+  if (!google::GetCommandLineFlagInfoOrDie("ysql_enable_auto_analyze").is_default) {
+    LOG(INFO) << "Flag: ysql_enable_auto_analyze has been set to " << FLAGS_ysql_enable_auto_analyze
+              << " by explicit config in command line";
     return;
-  bool old_value_of_new_flag = FLAGS_ysql_enable_auto_analyze;
-  // check if the old flags are enabled.
-  bool auto_analyze_enabled_using_old_flags
-    = FLAGS_ysql_enable_auto_analyze_service && FLAGS_ysql_enable_table_mutation_counter;
+  }
+
+  // When we change ysql_yb_enable_cbo to default on, we need to update the logic here, so adding
+  // a DCHECK to catch this situation.
+  DCHECK(
+      !(google::GetCommandLineFlagInfoOrDie("ysql_yb_enable_cbo").is_default &&
+        FLAGS_ysql_yb_enable_cbo == "on"));
+
+  // If ysql_yb_enable_cbo was explicitly set by user, then ysql_enable_auto_analyze flag is set to
+  // true whenever it is on. The reasoning is that CBO in legacy mode
+  // can have more unpredictable behavior when ANALYZE is run, so we don't want to run ANALYZE
+  // automatically.
+  bool cbo_flag_on = false;
+  std::string cbo_reason = "";
+  const std::vector<std::string> cbo_on_values = {"on", "true", "1", "yes"};
+  if (!google::GetCommandLineFlagInfoOrDie("ysql_yb_enable_cbo").is_default) {
+    if (std::find(
+            cbo_on_values.begin(), cbo_on_values.end(),
+            yb::ToLowerCase(FLAGS_ysql_yb_enable_cbo)) != cbo_on_values.end()) {
+      cbo_flag_on = true;
+      cbo_reason = "ysql_yb_enable_cbo flag was explicitly set by user to '" +
+                   FLAGS_ysql_yb_enable_cbo + "'";
+    }
+  } else {
+    // If ysql_pg_conf_csv contains yb_enable_cbo=on, then ysql_enable_auto_analyze is set to true.
+    // Note that ysql_pg_conf_csv has lower precedence than user-set ysql_yb_enable_cbo.
+    std::vector<std::string> user_configs;
+    std::optional<std::string> cbo_guc_value;
+    if (!FLAGS_ysql_pg_conf_csv.empty() &&
+        ReadCSVValues(FLAGS_ysql_pg_conf_csv, &user_configs).ok() &&
+        (cbo_guc_value = FindGUCValue("yb_enable_cbo", user_configs)).has_value() &&
+        std::find(cbo_on_values.begin(), cbo_on_values.end(), cbo_guc_value.value()) !=
+            cbo_on_values.end()) {
+      cbo_flag_on = true;
+      cbo_reason = "yb_enable_cbo was set to " + cbo_guc_value.value() + " in ysql_pg_conf_csv";
+    }
+  }
+
+  if (cbo_flag_on) {
+    google::SetCommandLineOptionWithMode(
+      "ysql_enable_auto_analyze", "true", google::SET_FLAG_IF_DEFAULT);
+    LOG(INFO) << "Flag: ysql_yb_enable_auto_analyze was auto-set to "
+              << FLAGS_ysql_enable_auto_analyze << " because " << cbo_reason;
+    return;
+  }
+
+  // If ysql_enable_auto_analyze_service and ysql_enable_table_mutation_counter are set
+  // explicitly, use their values to set value for ysql_enable_auto_analyze.
+  // ysql_enable_auto_analyze is true only if both ysql_enable_auto_analyze_service and
+  // ysql_enable_table_mutation_counter are true.
+  const bool old_value_of_new_flag = FLAGS_ysql_enable_auto_analyze;
+  const bool auto_analyze_enabled_using_old_flags =
+      FLAGS_ysql_enable_auto_analyze_service && FLAGS_ysql_enable_table_mutation_counter;
   if (auto_analyze_enabled_using_old_flags) {
-    // set new flag default.
     google::SetCommandLineOptionWithMode("ysql_enable_auto_analyze", "true",
                                          google::SET_FLAG_IF_DEFAULT);
 
-    LOG(INFO) << "Flag: ysql_enable_auto_analyze was auto-set to "
-              << FLAGS_ysql_enable_auto_analyze
+    LOG(INFO) << "Flag: ysql_enable_auto_analyze was auto-set to " << FLAGS_ysql_enable_auto_analyze
               << " from its default value of " << old_value_of_new_flag
-              << ". Flags: ysql_enable_auto_analyze_service and "
-              << "ysql_enable_table_mutation_counter are deprecated.";
+              << "because it was enabled by the following deprecated flags"
+              << " - ysql_enable_auto_analyze_service and ysql_enable_table_mutation_counter.";
+    return;
   }
+
+  LOG(INFO) << "Not changing ysql_enable_auto_analyze from its default value "
+            << FLAGS_ysql_enable_auto_analyze;
 }
 
 }  // anonymous namespace

src/yb/tserver/tserver_flags.cc

@@ -27,6 +27,7 @@ DEFINE_UNKNOWN_uint64(tserver_master_replication_factor, 0,
     "name and port through tserver_master_addrs for masters auto-discovery when running on "
     "Kubernetes.");
 
-DEFINE_RUNTIME_bool(ysql_enable_auto_analyze, true,
+DEFINE_RUNTIME_bool(
+    ysql_enable_auto_analyze, false,
     "Enable Auto Analyze to automatically trigger ANALYZE for updating table statistics of tables "
     "which have changed more than a configurable threshold.");

src/yb/util/pg_util.cc

@@ -14,7 +14,10 @@
 #include "yb/util/pg_util.h"
 
 #include <algorithm>
+
+#include <regex>
 #include <string>
+#include <boost/algorithm/string/replace.hpp>
 
 #include "yb/util/logging.h"
 
@@ -111,4 +114,38 @@ std::string PgDeriveSocketLockFile(const HostPort& host_port) {
       PgDeriveSocketDir(host_port), Format(".s.PGSQL.$0.lock", host_port.port()));
 }
 
+Status ReadCSVValues(const std::string& csv, std::vector<std::string>* lines) {
+  // Function reads CSV string in the following format:
+  // - fields are divided with comma (,)
+  // - fields with comma (,) or double-quote (") are quoted with double-quote (")
+  // - pair of double-quote ("") in quoted field represents single double-quote (")
+  //
+  // Examples:
+  // 1,"two, 2","three ""3""", four , -> ['1', 'two, 2', 'three "3"', ' four ', '']
+  // 1,"two                           -> Malformed CSV (quoted field 'two' is not closed)
+  // 1, "two"                         -> Malformed CSV (quoted field 'two' has leading spaces)
+  // 1,two "2"                        -> Malformed CSV (field with " must be quoted)
+  // 1,"tw"o"                         -> Malformed CSV (no separator after quoted field 'tw')
+
+  const std::regex exp(R"(^(?:([^,"]+)|(?:"((?:[^"]|(?:""))*)\"))(?:(?:,)|(?:$)))");
+  auto i = csv.begin();
+  const auto end = csv.end();
+  std::smatch match;
+  while (i != end && std::regex_search(i, end, match, exp)) {
+    // Replace pair of double-quote ("") with single double-quote (") in quoted field.
+    if (match[2].length() > 0) {
+      lines->emplace_back(match[2].first, match[2].second);
+      boost::algorithm::replace_all(lines->back(), "\"\"", "\"");
+    } else {
+      lines->emplace_back(match[1].first, match[1].second);
+    }
+    i += match.length();
+  }
+  SCHECK(i == end, InvalidArgument, Format("Malformed CSV '$0'", csv));
+  if (!csv.empty() && csv.back() == ',') {
+    lines->emplace_back();
+  }
+  return Status::OK();
+}
+
 } // namespace yb

src/yb/util/pg_util.h

@@ -14,13 +14,18 @@
 #pragma once
 
 #include <string>
+#include <vector>
 
 #include "yb/util/net/net_util.h"
 
 namespace yb {
 
+class Status;
+
 std::string PgDeriveSocketDir(const HostPort& host_port);
 
 std::string PgDeriveSocketLockFile(const HostPort& host_port);
 
+Status ReadCSVValues(const std::string& csv, std::vector<std::string>* lines);
+
 } // namespace yb

src/yb/yql/pgwrapper/pg_wrapper.cc

@@ -475,40 +475,6 @@ void MergeSharedPreloadLibraries(const string& src, vector<string>* defaults) {
   defaults->insert(defaults->end(), new_items.begin(), new_items.end());
 }
 
-Status ReadCSVValues(const string& csv, vector<string>* lines) {
-  // Function reads CSV string in the following format:
-  // - fields are divided with comma (,)
-  // - fields with comma (,) or double-quote (") are quoted with double-quote (")
-  // - pair of double-quote ("") in quoted field represents single double-quote (")
-  //
-  // Examples:
-  // 1,"two, 2","three ""3""", four , -> ['1', 'two, 2', 'three "3"', ' four ', '']
-  // 1,"two                           -> Malformed CSV (quoted field 'two' is not closed)
-  // 1, "two"                         -> Malformed CSV (quoted field 'two' has leading spaces)
-  // 1,two "2"                        -> Malformed CSV (field with " must be quoted)
-  // 1,"tw"o"                         -> Malformed CSV (no separator after quoted field 'tw')
-
-  const std::regex exp(R"(^(?:([^,"]+)|(?:"((?:[^"]|(?:""))*)\"))(?:(?:,)|(?:$)))");
-  auto i = csv.begin();
-  const auto end = csv.end();
-  std::smatch match;
-  while (i != end && std::regex_search(i, end, match, exp)) {
-    // Replace pair of double-quote ("") with single double-quote (") in quoted field.
-    if (match[2].length() > 0) {
-      lines->emplace_back(match[2].first, match[2].second);
-      boost::algorithm::replace_all(lines->back(), "\"\"", "\"");
-    } else {
-      lines->emplace_back(match[1].first, match[1].second);
-    }
-    i += match.length();
-  }
-  SCHECK(i == end, InvalidArgument, Format("Malformed CSV '$0'", csv));
-  if (!csv.empty() && csv.back() == ',') {
-    lines->emplace_back();
-  }
-  return Status::OK();
-}
-
 // Make sure that the parameter values do not contain '\n' since each line is a separate parameter.
 Status ValidateConfValuesBasic(const vector<string>& lines) {
   for (const string& parameter : lines) {