From a5efcb9b27fb577e32b7ed0ad7c13dc3d1045417 Mon Sep 17 00:00:00 2001
From: timothy-e <timothy.elgersma@gmail.com>
Date: Tue, 14 Jan 2025 18:24:54 +0000
Subject: [PATCH] [#25607] YSQL: Check pushdown is disabled before major
 version upgrade

Summary:
When pushdown is enabled during upgrade, queries can fail with cryptic errors like `ERROR:  did not find '}' at end of input node`.

Until #24730 - //Support for operator pushdown during upgrade// is done, we should more explicitly block upgrades when pushdown is disabled.
Jira: DB-14855

Test Plan:
```
./yb_build.sh release --cxx-test integration-tests_pg15_upgrade-test --gtest_filter Pg15UpgradeTest.CheckPushdownIsDisabled
```

Reviewers: hsunder

Reviewed By: hsunder

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D41200
---
 src/postgres/src/bin/pg_upgrade/check.c       | 37 +++++++++++++++++++
 .../upgrade-tests/pg15_upgrade-test.cc        | 12 ++++++
 2 files changed, 49 insertions(+)

diff --git a/src/postgres/src/bin/pg_upgrade/check.c b/src/postgres/src/bin/pg_upgrade/check.c
index c895109c7762..fc76955c5422 100644
--- a/src/postgres/src/bin/pg_upgrade/check.c
+++ b/src/postgres/src/bin/pg_upgrade/check.c
@@ -34,6 +34,8 @@ static void check_for_new_tablespace_dir(ClusterInfo *new_cluster);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 static char *get_canonical_locale_name(int category, const char *locale);
 
+static void yb_check_pushdown_is_disabled(ClusterInfo *cluster);
+
 /*
  * fix_path_separator
  * For non-Windows, just return the argument.
@@ -176,6 +178,14 @@ check_and_dump_old_cluster(bool live_check)
 	if (!is_yugabyte_enabled() && GET_MAJOR_VERSION(old_cluster.major_version) <= 903)
 		old_9_3_check_for_line_data_type_usage(&old_cluster);
 
+	/*
+	 * Yugabyte does not support expression pushdown during major upgrades.
+	 * Only check this when we are ready to actually upgrade the cluster,
+	 * because users may want to run this check long before the upgrade.
+	 */
+	if (is_yugabyte_enabled() && !user_opts.check)
+		yb_check_pushdown_is_disabled(&old_cluster);
+
 	/*
 	 * While not a check option, we do this now because this is the only time
 	 * the old server is running.
@@ -1538,3 +1548,30 @@ get_canonical_locale_name(int category, const char *locale)
 
 	return res;
 }
+
+/*
+ *	yb_check_pushdown_is_disabled()
+ *
+ *	Check we are the install user, and that the new cluster
+ *	has no other users.
+ */
+static void
+yb_check_pushdown_is_disabled(ClusterInfo *cluster)
+{
+	PGresult   *res;
+	PGconn	   *conn = connectToServer(cluster, "template1");
+
+	prep_status("Checking expression pushdown is disabled");
+
+	res = executeQueryOrDie(conn, "SHOW yb_enable_expression_pushdown");
+
+	if (strncmp(PQgetvalue(res, 0, 0), "off", 3))
+		pg_fatal("Expression pushdown (ysql_yb_enable_expression_pushdown) must "
+				 "be disabled during ysql major upgrade. See GH issue #24730\n");
+
+	PQclear(res);
+
+	PQfinish(conn);
+
+	check_ok();
+}
diff --git a/src/yb/integration-tests/upgrade-tests/pg15_upgrade-test.cc b/src/yb/integration-tests/upgrade-tests/pg15_upgrade-test.cc
index 49327069028b..b85aa6b47a9e 100644
--- a/src/yb/integration-tests/upgrade-tests/pg15_upgrade-test.cc
+++ b/src/yb/integration-tests/upgrade-tests/pg15_upgrade-test.cc
@@ -1400,6 +1400,18 @@ TEST_F(Pg15UpgradeTest, YbGinIndex) {
   }
 }
 
+TEST_F(Pg15UpgradeTest, CheckPushdownIsDisabled) {
+  // Whether or not pushdown is enabled, pg_upgrade --check will not error.
+  ASSERT_OK(cluster_->AddAndSetExtraFlag("ysql_yb_enable_expression_pushdown", "false"));
+  ASSERT_OK(ValidateUpgradeCompatibility());
+
+  ASSERT_OK(cluster_->AddAndSetExtraFlag("ysql_yb_enable_expression_pushdown", "true"));
+  ASSERT_OK(ValidateUpgradeCompatibility());
+
+  // However, when we actually run the YSQL upgrade, pg_upgrade will error if pushdown is enabled.
+  ASSERT_NOK(UpgradeClusterToMixedMode());
+}
+
 TEST_F(Pg15UpgradeSequenceTest, Sequences) {
   ASSERT_OK(ExecuteStatement(Format("CREATE SEQUENCE $0", kSequencePg11)));