-
Notifications
You must be signed in to change notification settings - Fork 891
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add CI workflow to check for incorrect catalog updates
When adding or dropping columns to timescaledb catalog tables, the right way to do this is to drop and recreate the table with the desired definition, instead of doing ALTER TABLE .. ADD/DROP COLUMN. This is required to ensure consistent attribute numbers across versions. This workflow will fail with an error if it detects such an incorrect modification of a catalog table. Fixes #6049
- Loading branch information
Showing
2 changed files
with
110 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
name: Check for unsafe catalog updates in latest-dev | ||
"on": | ||
pull_request: | ||
types: [opened, synchronize, reopened, edited] | ||
jobs: | ||
# Check if the PR has updated latest-dev. If a catalog update is required, but latest-dev | ||
# is unchanged, this will be caught in the update tests so we don't need to double check here. | ||
# Otherwise, read latest-dev contents to determine if there are column added or dropped from | ||
# catalog tables. A "safe" modification should rebuild the table to ensure consistent attribute | ||
# numbers across versions, instead of simply doing ALTER TABLE ... ADD/DROP COLUMN | ||
check_latest_dev_correctly_updated: | ||
name: Check that latest-dev.sql was properly updated by PR | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout source | ||
uses: actions/checkout@v3 | ||
with: | ||
ref: ${{ github.event.pull_request.head.sha }} | ||
fetch-depth: 0 | ||
- name: Check if latest-dev was modified | ||
shell: bash --norc --noprofile {0} | ||
id: check_updated | ||
run: | | ||
file="sql/updates/latest-dev.sql" | ||
updated=0 | ||
# diff=$(git diff --name-status origin/main | grep "${file}") | ||
diff=$(git --no-pager diff --name-only HEAD $(git merge-base HEAD ${{ github.event.pull_request.base.sha }}) | grep "${file}") | ||
echo "diff is ${diff}" | ||
if [[ ${diff} != "" ]]; then | ||
updated=1 | ||
fi | ||
echo "updated is ${updated}" | ||
echo "latestdev_updated=$(($updated))" >> $GITHUB_OUTPUT | ||
- name: Check latest-dev contents | ||
shell: bash --norc --noprofile {0} | ||
env: | ||
BODY: ${{ github.event.pull_request.body }} | ||
run: | | ||
if [[ ${{ steps.check_updated.outputs.latestdev_updated }} == 0 ]]; then | ||
exit 0; # did not update latest-dev, if anything should have been updated, it will be caught in the upgrade test | ||
fi | ||
echo "$BODY" | egrep -qsi '^disable-check:.*\<catalog-updates\>' | ||
if [[ $? -ne 0 ]]; then | ||
if ! python scripts/check_latest-dev_updates.py "sql/updates/latest-dev.sql"; then | ||
echo | ||
echo "Incorrect modification of catalog tables detected" | ||
echo | ||
echo "To disable the catalog updates check, add this trailer to pull request message:" | ||
echo | ||
echo "Disable-check: catalog-updates" | ||
echo | ||
echo "Trailers follow RFC2822 conventions, so no whitespace" | ||
echo "before field name and the check is case-insensitive for" | ||
echo "both the field name and the field body." | ||
exit 1 | ||
fi | ||
fi | ||
exit 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
#!/usr/bin/env python3 | ||
|
||
import sys | ||
import re | ||
|
||
|
||
# also need a function to separate comments from actual commands | ||
def remove_comments(lines): | ||
code_lines = [] | ||
for line in lines: | ||
if not line.startswith("--"): | ||
code_lines.append(line) | ||
return "\n".join(code_lines) | ||
|
||
|
||
def unsafe_catalog_modification(s): | ||
# catalog tables reside in _timescaledb_catalog, _timescaledb_internal and _timescaledb_config | ||
# it is unsafe to simply ADD or DROP columns from catalog tables | ||
matches = re.search( | ||
r"ALTER\s*TABLE\s*(_timescaledb_catalog|_timescaledb_internal|_timescaledb_config)[\s\S]+(DROP|ADD)\s*COLUMN", | ||
s, | ||
flags=re.IGNORECASE, | ||
) | ||
if matches: | ||
return True | ||
return False | ||
|
||
|
||
def main(): | ||
# Open latest-dev.sql | ||
latest_dev = sys.argv[1] | ||
unsafe_modification = False | ||
contents = "" | ||
|
||
with open(latest_dev, "r", encoding="utf-8") as ldev: | ||
# remove comments, we don't want to confuse them with actual commands | ||
lines = ldev.readlines() | ||
contents = remove_comments(lines) | ||
unsafe_modification = unsafe_catalog_modification(contents) | ||
if unsafe_modification: | ||
print( | ||
"""ERROR: Attempting to alter timescaledb catalog tables without rebuilding the table.""" | ||
+ """\nRebuilding catalog tables is required to ensure consistent attribute numbers across versions.""" | ||
) | ||
sys.exit(1) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() | ||
sys.exit(0) |