Add actionable hints to update script checker

svenklemm · Jan 31, 2025 · 65b8151 · 65b8151
1 parent 64e5ffc
commit 65b8151
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 41 deletions.
diff --git a/.github/workflows/catalog-updates-check.yaml b/.github/workflows/catalog-updates-check.yaml
@@ -16,11 +16,11 @@ jobs:
 
       - 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
+          find sql -name '*.sql' -not -path 'sql/updates/*' -not -path 'sql/compat.sql' | xargs -IFILE python scripts/check_updates.py FILE
 
       - name: Check latest-dev contents
         run: |
-          python scripts/check_updates_ast.py --latest "sql/updates/latest-dev.sql"
+          python scripts/check_updates.py --latest "sql/updates/latest-dev.sql"
 
       - name: Check for idempotency in SQL scripts
         if: always()
@@ -32,5 +32,5 @@ jobs:
       - name: Check reverse-dev contents
         if: always()
         run: |
-          python scripts/check_updates_ast.py "sql/updates/reverse-dev.sql" || true
+          python scripts/check_updates.py "sql/updates/reverse-dev.sql" || true
 
diff --git a/scripts/check_updates_ast.py → scripts/check_updates.py b/scripts/check_updates_ast.py → scripts/check_updates.py
@@ -28,6 +28,13 @@ def __init__(self):
         ]
         super().__init__()
 
+    def error(self, msg, hint=None):
+        self.errors += 1
+        print(msg)
+        if hint:
+            print(hint)
+        print()
+
     # ALTER TABLE _timescaledb_catalog.<tablename> ADD/DROP COLUMN
     def visit_AlterTableStmt(self, ancestors, node):  # pylint: disable=unused-argument
         if (
@@ -41,46 +48,45 @@ def visit_AlterTableStmt(self, ancestors, node):  # pylint: disable=unused-argum
                     enums.AlterTableType.AT_AddColumn,
                     enums.AlterTableType.AT_DropColumn,
                 ):
-                    self.errors += 1
                     if cmd.subtype == enums.AlterTableType.AT_AddColumn:
+                        subcmd = "ADD"
                         column = cmd.def_.colname
-                        print(
-                            f"ERROR: Attempting to ADD COLUMN {column} to catalog table {schema}.{table}"
-                        )
                     else:
+                        subcmd = "DROP"
                         column = cmd.name
-                        print(
-                            f"ERROR: Attempting to DROP COLUMN {column} from catalog table {schema}.{table}"
-                        )
+
+                    self.error(
+                        f"Attempting to {subcmd} COLUMN {column} to catalog table {schema}.{table}",
+                        "Tables need to be rebuilt in update script to ensure consistent attribute numbers",
+                    )
 
     # ALTER TABLE _timescaledb_catalog.<tablename> RENAME TO
     def visit_RenameStmt(self, ancestors, node):  # pylint: disable=unused-argument
         if (
             node.renameType == enums.ObjectType.OBJECT_TABLE
             and node.relation.schemaname in self.catalog_schemata
         ):
-            self.errors += 1
-            print(
-                f"ERROR: Attempting to RENAME catalog table {node.relation.schemaname}.{node.relation.relname}"
+            self.error(
+                f"Attempting to RENAME catalog table {node.relation.schemaname}.{node.relation.relname}",
+                "Catalog tables should be rebuilt in update scripts to ensure consistent naming for dependent objects",
             )
 
     # CREATE TEMP | TEMPORARY TABLE ..
     # CREATE TABLE IF NOT EXISTS ..
     def visit_CreateStmt(self, ancestors, node):  # pylint: disable=unused-argument
         if node.relation.relpersistence == "t":
-            self.errors += 1
             schema = (
                 node.relation.schemaname + "."
                 if node.relation.schemaname is not None
                 else ""
             )
-            print(
-                f"ERROR: Attempting to CREATE TEMPORARY TABLE {schema}{node.relation.relname}"
+            self.error(
+                f"Attempting to CREATE TEMPORARY TABLE {schema}{node.relation.relname}"
+                "Creating temporary tables is blocked in pg_extwlist context"
             )
         if node.if_not_exists:
-            self.errors += 1
-            print(
-                f"ERROR: Attempting to CREATE TABLE IF NOT EXISTS {node.relation.relname}"
+            self.error(
+                f"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
@@ -94,9 +100,8 @@ def visit_CreateStmt(self, ancestors, node):  # pylint: disable=unused-argument
                             "bool",
                             "text",
                         ]:
-                            self.errors += 1
-                            print(
-                                f"ERROR: Attempting to CREATE TABLE {node.relation.relname} with blocked array type {coldef.typeName.names[-1].sval}"
+                            self.error(
+                                f"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 [
@@ -114,44 +119,39 @@ def visit_CreateStmt(self, ancestors, node):  # pylint: disable=unused-argument
                             "text",
                             "timestamptz",
                         ]:
-                            self.errors += 1
-                            print(
-                                f"ERROR: Attempting to CREATE TABLE {node.relation.relname} with blocked type {coldef.typeName.names[-1].sval}"
+                            self.error(
+                                f"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
     ):  # pylint: disable=unused-argument
         if node.if_not_exists:
-            self.errors += 1
-            print(f"ERROR: Attempting to CREATE SCHEMA IF NOT EXISTS {node.schemaname}")
+            self.error(f"Attempting to CREATE SCHEMA IF NOT EXISTS {node.schemaname}")
 
     # CREATE MATERIALIZED VIEW IF NOT EXISTS ..
     def visit_CreateTableAsStmt(
         self, ancestors, node
     ):  # pylint: disable=unused-argument
         if node.if_not_exists:
-            self.errors += 1
-            print(
-                f"ERROR: Attempting to CREATE MATERIALIZED VIEW IF NOT EXISTS {node.into.rel.relname}"
+            self.error(
+                f"Attempting to CREATE MATERIALIZED VIEW IF NOT EXISTS {node.into.rel.relname}"
             )
 
     # CREATE FOREIGN TABLE IF NOT EXISTS ..
     def visit_CreateForeignTableStmt(
         self, ancestors, node
     ):  # pylint: disable=unused-argument
         if node.base.if_not_exists:
-            self.errors += 1
-            print(
-                f"ERROR: Attempting to CREATE FOREIGN TABLE IF NOT EXISTS {node.base.relation.relname}"
+            self.error(
+                f"Attempting to CREATE FOREIGN TABLE IF NOT EXISTS {node.base.relation.relname}"
             )
 
     # CREATE INDEX IF NOT EXISTS ..
     def visit_IndexStmt(self, ancestors, node):  # pylint: disable=unused-argument
         if node.if_not_exists:
-            self.errors += 1
-            print(f"ERROR: Attempting to CREATE INDEX IF NOT EXISTS {node.idxname}")
+            self.error(f"Attempting to CREATE INDEX IF NOT EXISTS {node.idxname}")
 
     # CREATE FUNCTION / PROCEDURE _timescaledb_internal...
     def visit_CreateFunctionStmt(
@@ -170,17 +170,17 @@ def visit_CreateFunctionStmt(
                 and node.returnType.names[0].sval
                 not in ["table_am_handler", "index_am_handler"]
             ):
-                self.errors += 1
                 functype = "procedure" if node.is_procedure else "function"
-                print(
-                    f"ERROR: Attempting to create {functype} {node.funcname[1].sval} with language 'c'"
+                self.error(
+                    f"Attempting to create {functype} {node.funcname[-1].sval} with language 'c'",
+                    "latest-dev should link C functions to ts_update_placeholder",
                 )
 
         if len(node.funcname) == 2 and node.funcname[0].sval == "_timescaledb_internal":
-            self.errors += 1
             functype = "procedure" if node.is_procedure else "function"
-            print(
-                f"ERROR: Attempting to create {functype} {node.funcname[1].sval} in the internal schema"
+            self.error(
+                f"Attempting to create {functype} {node.funcname[1].sval} in the internal schema",
+                "_timescaledb_functions should be used as schema for internal functions",
             )