Skip to content

Commit

Permalink
Add check for column types we use in our catalog.
Browse files Browse the repository at this point in the history
We have to be careful with the column types we use in our catalog to only allow types
that are safe to use in catalog tables and not cause problems during extension upgrade,
pg_upgrade or dump/restore.
  • Loading branch information
svenklemm committed May 28, 2024
1 parent cec6b37 commit 70bdbaf
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 13 deletions.
23 changes: 10 additions & 13 deletions .github/workflows/catalog-updates-check.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: Check for unsafe catalog updates
"on":
pull_request:
types: [opened, synchronize, reopened, edited]
jobs:
check_catalog_correctly_updated:
name: Check updates to latest-dev and reverse-dev are properly handled by PR
Expand All @@ -10,24 +9,22 @@ jobs:
- name: Checkout source
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 0
- name: Install pglast
run: |
python -m pip install pglast
- name: Check sql file contents
run: |
find sql -name '*.sql' -not -path 'sql/updates/*' -not -path 'sql/compat.sql' | xargs -IFILE python scripts/check_updates_ast.py FILE
- name: Check latest-dev contents
shell: bash {0}
id: check_latestdev
run: |
if ! python scripts/check_updates_ast.py "sql/updates/latest-dev.sql"; then
exit 1
fi
exit 0
python scripts/check_updates_ast.py "sql/updates/latest-dev.sql"
# To allow fixing previous mistakes we run the check against reverse-dev but don't
# fail it on errors.
- name: Check reverse-dev contents
if: always()
shell: bash {0}
run: |
if ! python scripts/check_updates_ast.py "sql/updates/reverse-dev.sql"; then
exit 1
fi
exit 0
python scripts/check_updates_ast.py "sql/updates/reverse-dev.sql" || true
37 changes: 37 additions & 0 deletions scripts/check_updates_ast.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from pglast import parse_sql
from pglast.ast import ColumnDef
from pglast.visitors import Visitor
from pglast import enums
import sys
Expand Down Expand Up @@ -70,6 +71,42 @@ def visit_CreateStmt(self, ancestors, node): # pylint: disable=unused-argument
f"ERROR: Attempting to CREATE TABLE IF NOT EXISTS {node.relation.relname}"
)

# We have to be careful with the column types we use in our catalog to only allow types
# that are safe to use in catalog tables and not cause problems during extension upgrade,
# pg_upgrade or dump/restore.
if node.tableElts is not None:
for coldef in node.tableElts:
if isinstance(coldef, ColumnDef):
if coldef.typeName.arrayBounds is not None:
if coldef.typeName.names[-1].sval not in [
"bool",
"text",
]:
self.errors += 1
print(
f"ERROR: Attempting to CREATE TABLE {node.relation.relname} with blocked array type {coldef.typeName.names[-1].sval}"
)
else:
if coldef.typeName.names[-1].sval not in [
"bool",
"int2",
"int4",
"int8",
"interval",
"jsonb",
"name",
"regclass",
"regtype",
"regrole",
"serial",
"text",
"timestamptz",
]:
self.errors += 1
print(
f"ERROR: Attempting to CREATE TABLE {node.relation.relname} with blocked type {coldef.typeName.names[-1].sval}"
)

# CREATE SCHEMA IF NOT EXISTS ..
def visit_CreateSchemaStmt(
self, ancestors, node
Expand Down

0 comments on commit 70bdbaf

Please sign in to comment.