Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI checks to check for missing downgrade updates #6661

Merged
merged 1 commit into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ jobs:
- run:
name: 'Check if all GUCs are sorted alphabetically'
command: ci/check_gucs_are_alphabetically_sorted.sh
- run:
name: 'Check for missing downgrade scripts'
command: ci/check_migration_files.sh

check-sql-snapshots:
docker:
Expand Down
8 changes: 8 additions & 0 deletions ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,14 @@ actually run in CI. This is most commonly forgotten for newly added CI tests
that the developer only ran locally. It also checks that all CI scripts have a
section in this `README.md` file and that they include `ci/ci_helpers.sh`.

## `check_migration_files.sh`

A branch that touches a set of upgrade scripts is also expected to touch
corresponding downgrade scripts as well. If this script fails, read the output
and make sure you update the downgrade scripts in the printed list. If you
really don't need a downgrade to run any SQL. You can write a comment in the
file explaining why a downgrade step is not necessary.

## `disallow_c_comments_in_migrations.sh`

We do not use C-style comments in migration files as the stripped
Expand Down
33 changes: 33 additions & 0 deletions ci/check_migration_files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#! /bin/bash

set -euo pipefail
# shellcheck disable=SC1091
source ci/ci_helpers.sh

# This file checks for the existence of downgrade scripts for every upgrade script that is changed in the branch.

# create list of migration files for upgrades
upgrade_files=$(git diff --name-only origin/main | { grep "src/backend/distributed/sql/citus--.*sql" || exit 0 ; })
downgrade_files=$(git diff --name-only origin/main | { grep "src/backend/distributed/sql/downgrades/citus--.*sql" || exit 0 ; })
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only doing this check for citus upgrade files, not for columnar ones.

ret_value=0

for file in $upgrade_files
do
# There should always be 2 matches, and no need to avoid splitting here
# shellcheck disable=SC2207
versions=($(grep --only-matching --extended-regexp "[0-9]+\.[0-9]+[-.][0-9]+" <<< "$file"))

from_version=${versions[0]};
to_version=${versions[1]};

downgrade_migration_file="src/backend/distributed/sql/downgrades/citus--$to_version--$from_version.sql"

# check for the existence of migration scripts
if [[ $(grep --line-regexp --count downgrade_migration_file <<< "$downgrade_files") == 0 ]]
then
echo "$file is updated, but $downgrade_migration_file is not updated in branch"
ret_value=1
fi
done

exit $ret_value;