Skip to content

Commit

Permalink
[#25607] YSQL: Check pushdown is disabled before major version upgrade
Browse files Browse the repository at this point in the history
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
  • Loading branch information
timothy-e authored and vpatibandla-yb committed Jan 16, 2025
1 parent 66bf744 commit 4acfd0c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
37 changes: 37 additions & 0 deletions src/postgres/src/bin/pg_upgrade/check.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}
12 changes: 12 additions & 0 deletions src/yb/integration-tests/upgrade-tests/pg15_upgrade-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)));

Expand Down

0 comments on commit 4acfd0c

Please sign in to comment.