Fix repair in update scripts

The commit fixes two bugs in the repair scripts that could prevent an update in rare circumstances. For the 1.7.1--1.7.2 repair script: if there were several missing dimension slices in different hypertables with the same column name, the repair script would be confused on what constraint had what type and generate an error. For the 2.0.0-rc1--2.0.0-rc2 repair script: if a partition constraint was broken, it would generate an error rather than repairing the dimension slices because BIGINT_MIN would be cast to a double float and then an attempt would be made to cast it back to bigint, causing an overflow error. This commit also creates an update repair test that breaks a few tables for pre-2.0 versions to ensure that the repair script actually fixes them. The integrity check for the update tests already contain a check that dimension slices are valid, so there is no need to add a test for that. This commit adds an extra dimension in the workflow to test updates with repair and run that separately. It also changes the update test scripts to by default run without repair tests and add the additional option `-r` for running repair tests in addition to the normal tests. Fixes timescale#2824
svenklemm · Jan 28, 2021 · c716325 · c716325
1 parent 0e86bbe
commit c716325
Show file tree

Hide file tree

Showing 11 changed files with 318 additions and 62 deletions.
diff --git a/.github/workflows/update-test.yaml b/.github/workflows/update-test.yaml
@@ -9,16 +9,19 @@ on:
   pull_request:
 jobs:
   update_test:
-    name: Update test PG${{ matrix.pg }}
+    name: Update test PG${{ matrix.pg }} ${{ matrix.kind }}
     runs-on: 'ubuntu-18.04'
     strategy:
       matrix:
         pg: ["11.10","12.5"]
+        opt: ["", "-r"]
         include:
           - pg: 11.10
             pg_major: 11
           - pg: 12.5
             pg_major: 12
+          - opt: "-r"
+            kind: "with repair test"
       fail-fast: false
     env:
       PG_VERSION: ${{ matrix.pg }}
@@ -27,9 +30,9 @@ jobs:
     - name: Checkout TimescaleDB
       uses: actions/checkout@v2
 
-    - name: Update tests ${{ matrix.pg }}
+    - name: Update tests ${{ matrix.pg }} ${{ matrix.kind }}
       run: |
-        ./scripts/test_updates_pg${{ matrix.pg_major}}.sh
+        ./scripts/test_updates_pg${{ matrix.pg_major}}.sh ${{ matrix.opt }}
 
     - name: Update diff
       if: failure()
@@ -52,4 +55,3 @@ jobs:
         SLACK_TITLE: Update test PG${{ matrix.pg }} ${{ job.status }}
         SLACK_MESSAGE: ${{ github.event.head_commit.message }}
       uses: rtCamp/action-slack-notify@v2.0.2
-
diff --git a/scripts/test_update_from_tag.sh b/scripts/test_update_from_tag.sh
@@ -83,7 +83,7 @@ docker_pgcmd() {
     local database=${3:-single}
     echo "executing pgcmd on database $database"
     set +e
-    docker_exec $1 "psql -h localhost -U postgres -d $database -v VERBOSITY=verbose -c \"$2\""
+    docker_exec $1 "psql -h localhost -U postgres -d $database -v TEST_REPAIR=${TEST_REPAIR} -v VERBOSITY=verbose -c \"$2\""
     if [ $? -ne 0 ]; then
       docker_logs $1
       exit 1
@@ -93,7 +93,7 @@ docker_pgcmd() {
 
 docker_pgscript() {
     local database=${3:-postgres}
-    docker_exec $1 "psql -h localhost -U postgres -d $database -v ON_ERROR_STOP=1 -f $2"
+    docker_exec $1 "psql -h localhost -U postgres -d $database -v TEST_REPAIR=${TEST_REPAIR} -v ON_ERROR_STOP=1 -f $2"
 }
 
 docker_pgtest() {
@@ -204,12 +204,30 @@ docker_pgcmd ${CONTAINER_UPDATED} "ALTER EXTENSION timescaledb UPDATE" "postgres
 # version that doesn't support multi-node to a multi-node capable
 # version.
 if [[ "${TEST_VERSION}" > "v6" ]] || [[ "${TEST_VERSION}" = "v6" ]]; then
-	echo "Executing post update scritps"
+ 	echo "Executing post update scripts"
 	docker_pgscript ${CONTAINER_UPDATED} /src/test/sql/updates/post.update.sql "single"
+	if [[ "${TEST_REPAIR}" = "true" ]]; then
+	    echo "Executing post update repair script"
+	    docker_pgscript ${CONTAINER_UPDATED} /src/test/sql/updates/post.repair.sql "single"
+	fi
 fi
 
+
+# Check that there is nothing wrong before taking a backup
+echo "Checking that there are no missing dimension slices"
+docker_pgscript ${CONTAINER_UPDATED} /src/test/sql/updates/setup.check.sql
+
 echo "Executing setup script on clean"
 docker_pgscript ${CONTAINER_CLEAN_RERUN} /src/test/sql/updates/setup.${TEST_VERSION}.sql
+if [[ "${TEST_VERSION}" > "v6" ]] || [[ "${TEST_VERSION}" = "v6" ]]; then
+    if [[ "${TEST_REPAIR}" = "true" ]]; then
+	# We need to run the post repair script to make sure that the
+	# constraint is on the clean rerun as well since the setup
+	# script can remove it.
+	echo "Executing post update repair script on clean"
+	docker_pgscript ${CONTAINER_CLEAN_RERUN} /src/test/sql/updates/post.repair.sql "single"
+    fi
+fi
 
 docker_exec ${CONTAINER_UPDATED} "pg_dump -h localhost -U postgres -Fc single > /tmp/single.sql"
 docker_exec ${CONTAINER_UPDATED} "pg_dump -h localhost -U postgres -Fc dn1 > /tmp/dn1.sql"
@@ -224,13 +242,13 @@ docker cp ${TEST_TMPDIR}/dn1.sql ${CONTAINER_CLEAN_RESTORE}:/tmp/dn1.sql
 docker_exec ${CONTAINER_CLEAN_RESTORE} "createdb -h localhost -U postgres single"
 docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE single SET timescaledb.restoring='on'"
 docker_exec ${CONTAINER_CLEAN_RESTORE} "pg_restore -h localhost -U postgres -d single /tmp/single.sql"
-docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE single SET timescaledb.restoring='off'"
+docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE single RESET timescaledb.restoring"
 
 # Restore dn1
 docker_exec ${CONTAINER_CLEAN_RESTORE} "createdb -h localhost -U postgres dn1"
 docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE dn1 SET timescaledb.restoring='on'"
 docker_exec ${CONTAINER_CLEAN_RESTORE} "pg_restore -h localhost -U postgres -d dn1 /tmp/dn1.sql"
-docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE dn1 SET timescaledb.restoring='off'"
+docker_pgcmd ${CONTAINER_CLEAN_RESTORE} "ALTER DATABASE dn1 RESET timescaledb.restoring"
 
 echo "Comparing upgraded ($UPDATE_FROM_TAG -> $UPDATE_TO_TAG) with clean install ($UPDATE_TO_TAG)"
 docker_pgdiff_all /src/test/sql/updates/post.${TEST_VERSION}.sql "single"
diff --git a/scripts/test_updates.sh b/scripts/test_updates.sh
@@ -13,16 +13,19 @@ UPDATE_TO_IMAGE=${UPDATE_TO_IMAGE:-update_test}
 UPDATE_TO_TAG=${UPDATE_TO_TAG:-${GIT_ID}}
 PG_VERSION=${PG_VERSION:-11.0}
 
+# This will propagate to the test_update_from_tags.sh script
+export TEST_REPAIR
+
 FAILED_TEST=
 KEEP_TEMP_DIRS=false
 TEST_UPDATE_FROM_TAGS_EXTRA_ARGS=
-
+TEST_REPAIR=false
 FAIL_COUNT=0
 
 # Declare a hash table to keep test names keyed by pid
 declare -A tests
 
-while getopts "cd" opt;
+while getopts "cdr" opt;
 do
     case $opt in
         c)
@@ -34,6 +37,10 @@ do
             KEEP_TEMP_DIRS=true
             TEST_UPDATE_FROM_TAGS_EXTRA_ARGS="-d"
             ;;
+	r)
+	    echo "Breaking dimension slices to test repair part"
+	    TEST_REPAIR=true
+	    ;;
     esac
 done
 

diff --git a/scripts/test_updates_pg11.sh b/scripts/test_updates_pg11.sh
@@ -8,7 +8,7 @@ SCRIPT_DIR=$(dirname $0)
 TAGS="1.1.0-pg11 1.1.1-pg11 1.2.0-pg11 1.2.1-pg11 1.2.2-pg11"
 TEST_VERSION="v2"
 
-TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh
+TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh "$@"
 EXIT_CODE=$?
 if [ $EXIT_CODE -ne 0 ]; then
   exit $EXIT_CODE
@@ -17,7 +17,7 @@ fi
 TAGS="1.3.0-pg11 1.3.1-pg11 1.3.2-pg11 1.4.0-pg11 1.4.1-pg11 1.4.2-pg11"
 TEST_VERSION="v4"
 
-TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh
+TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh "$@"
 EXIT_CODE=$?
 if [ $EXIT_CODE -ne 0 ]; then
   exit $EXIT_CODE
@@ -26,7 +26,7 @@ fi
 TAGS="1.5.0-pg11 1.5.1-pg11 1.6.0-pg11 1.6.1-pg11"
 TEST_VERSION="v5"
 
-TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh
+TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh "$@"
 EXIT_CODE=$?
 if [ $EXIT_CODE -ne 0 ]; then
   exit $EXIT_CODE
@@ -35,7 +35,7 @@ fi
 TAGS="1.7.0-pg11 1.7.1-pg11 1.7.2-pg11 1.7.3-pg11 1.7.4-pg11 2.0.0-rc1-pg11 2.0.0-rc2-pg11 2.0.0-rc3-pg11 2.0.0-rc4-pg11 2.0.0-pg11"
 TEST_VERSION="v6"
 
-TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh
+TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh "$@"
 EXIT_CODE=$?
 if [ $EXIT_CODE -ne 0 ]; then
   exit $EXIT_CODE

diff --git a/scripts/test_updates_pg12.sh b/scripts/test_updates_pg12.sh
@@ -9,7 +9,7 @@ echo $SCRIPT_DIR
 TAGS="1.7.0-pg12 1.7.1-pg12 1.7.2-pg12 1.7.3-pg12 1.7.4-pg12 2.0.0-rc1-pg12 2.0.0-rc2-pg12 2.0.0-rc3-pg12 2.0.0-rc4-pg12 2.0.0-pg12"
 TEST_VERSION="v6"
 
-TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh
+TAGS=$TAGS TEST_VERSION=$TEST_VERSION bash ${SCRIPT_DIR}/test_updates.sh "$@"
 EXIT_CODE=$?
 if [ $EXIT_CODE -ne 0 ]; then
   exit $EXIT_CODE

diff --git a/sql/updates/1.7.1--1.7.2.sql b/sql/updates/1.7.1--1.7.2.sql
@@ -14,61 +14,70 @@ RETURNS BIGINT AS '@MODULE_PATHNAME@', 'ts_time_to_internal' LANGUAGE C VOLATILE
 INSERT INTO _timescaledb_catalog.dimension_slice
 WITH
    -- All dimension slices that are mentioned in the chunk_constraint
-   -- table but are missing from the dimension_slice table.
+   -- table but are missing from the dimension_slice table. There can
+   -- be duplicates since several chunk constraints can refer to one
+   -- dimension slice.
    missing_slices AS (
-      SELECT dimension_slice_id,
-      	     constraint_name,
-	     attname AS column_name,
-	     pg_get_expr(conbin, conrelid) AS constraint_expr
+      SELECT DISTINCT ch.hypertable_id,
+             di.id as dimension_id,
+             dimension_slice_id,
+             constraint_name,
+             di.column_type,
+             attname AS column_name,
+             pg_get_expr(conbin, conrelid) AS constraint_expr
       FROM _timescaledb_catalog.chunk_constraint cc
       JOIN _timescaledb_catalog.chunk ch ON cc.chunk_id = ch.id
       JOIN pg_constraint ON conname = constraint_name
       JOIN pg_namespace ns ON connamespace = ns.oid AND ns.nspname = ch.schema_name
       JOIN pg_attribute ON attnum = conkey[1] AND attrelid = conrelid
+      JOIN _timescaledb_catalog.dimension di
+           ON ch.hypertable_id = di.hypertable_id AND di.column_name = attname
       WHERE
-	 dimension_slice_id NOT IN (SELECT id FROM _timescaledb_catalog.dimension_slice)
+         dimension_slice_id NOT IN (SELECT id FROM _timescaledb_catalog.dimension_slice)
    ),
 
   -- Unparsed range start and end for each dimension slice id that
   -- is missing.
    unparsed_missing_slices AS (
-      SELECT dimension_slice_id,
+      SELECT dimension_id,
+             dimension_slice_id,
              constraint_name,
-	     column_name,
-	     (SELECT SUBSTRING(constraint_expr, $$>=\s*'?([\d\s:+-]+)'?$$)) AS range_start,
-	     (SELECT SUBSTRING(constraint_expr, $$<\s*'?([\d\s:+-]+)'?$$)) AS range_end
-	FROM missing_slices
+             column_type,
+             column_name,
+             (SELECT SUBSTRING(constraint_expr, $$>=\s*'?([\d\s:+-]+)'?$$)) AS range_start,
+             (SELECT SUBSTRING(constraint_expr, $$<\s*'?([\d\s:+-]+)'?$$)) AS range_end
+        FROM missing_slices
    )
 SELECT dimension_slice_id,
-       di.id AS dimension_id,
+       dimension_id,
        CASE
-       WHEN di.column_type IN ('smallint'::regtype, 'bigint'::regtype, 'integer'::regtype) THEN
-       	    CASE
-	    WHEN range_start IS NULL
-	    THEN -9223372036854775808
-	    ELSE _timescaledb_internal.time_to_internal(range_start::bigint)
-	    END
-       WHEN di.column_type = 'timestamptz'::regtype THEN
-       	    _timescaledb_internal.time_to_internal(range_start::timestamptz)
-       WHEN di.column_type = 'timestamp'::regtype THEN
-       	    _timescaledb_internal.time_to_internal(range_start::timestamp)
-       WHEN di.column_type = 'date'::regtype THEN
-       	    _timescaledb_internal.time_to_internal(range_start::date)
+       WHEN column_type IN ('smallint'::regtype, 'bigint'::regtype, 'integer'::regtype) THEN
+            CASE
+            WHEN range_start IS NULL
+            THEN -9223372036854775808
+            ELSE _timescaledb_internal.time_to_internal(range_start::bigint)
+            END
+       WHEN column_type = 'timestamptz'::regtype THEN
+            _timescaledb_internal.time_to_internal(range_start::timestamptz)
+       WHEN column_type = 'timestamp'::regtype THEN
+            _timescaledb_internal.time_to_internal(range_start::timestamp)
+       WHEN column_type = 'date'::regtype THEN
+            _timescaledb_internal.time_to_internal(range_start::date)
        ELSE
-	    NULL
+            NULL
        END AS range_start,
-       CASE 
-       WHEN di.column_type IN ('smallint'::regtype, 'bigint'::regtype, 'integer'::regtype) THEN
-       	    CASE WHEN range_end IS NULL
-	    THEN 9223372036854775807
-	    ELSE _timescaledb_internal.time_to_internal(range_end::bigint)
-	    END
-       WHEN di.column_type = 'timestamptz'::regtype THEN
-       	    _timescaledb_internal.time_to_internal(range_end::timestamptz)
-       WHEN di.column_type = 'timestamp'::regtype THEN
-       	    _timescaledb_internal.time_to_internal(range_end::timestamp)
-       WHEN di.column_type = 'date'::regtype THEN
-       	    _timescaledb_internal.time_to_internal(range_end::date)
+       CASE
+       WHEN column_type IN ('smallint'::regtype, 'bigint'::regtype, 'integer'::regtype) THEN
+            CASE WHEN range_end IS NULL
+            THEN 9223372036854775807
+            ELSE _timescaledb_internal.time_to_internal(range_end::bigint)
+            END
+       WHEN column_type = 'timestamptz'::regtype THEN
+            _timescaledb_internal.time_to_internal(range_end::timestamptz)
+       WHEN column_type = 'timestamp'::regtype THEN
+            _timescaledb_internal.time_to_internal(range_end::timestamp)
+       WHEN column_type = 'date'::regtype THEN
+            _timescaledb_internal.time_to_internal(range_end::date)
        ELSE NULL
        END AS range_end
-  FROM unparsed_missing_slices JOIN _timescaledb_catalog.dimension di USING (column_name);
+  FROM unparsed_missing_slices;
diff --git a/sql/updates/2.0.0-rc1--2.0.0-rc2.sql b/sql/updates/2.0.0-rc1--2.0.0-rc2.sql
@@ -137,28 +137,28 @@ SELECT DISTINCT
        dimension_id,
        CASE
        WHEN column_type = 'timestamptz'::regtype THEN
-            EXTRACT(EPOCH FROM range_start::timestamptz) * 1000000
+            EXTRACT(EPOCH FROM range_start::timestamptz)::bigint * 1000000
        WHEN column_type = 'timestamp'::regtype THEN
-            EXTRACT(EPOCH FROM range_start::timestamp) * 1000000
+            EXTRACT(EPOCH FROM range_start::timestamp)::bigint * 1000000
        WHEN column_type = 'date'::regtype THEN
-            EXTRACT(EPOCH FROM range_start::date) * 1000000
+            EXTRACT(EPOCH FROM range_start::date)::bigint * 1000000
        ELSE
             CASE
             WHEN range_start IS NULL
-            THEN -9223372036854775808
+            THEN (-9223372036854775808)::bigint
             ELSE range_start::bigint
             END
        END AS range_start,
        CASE
        WHEN column_type = 'timestamptz'::regtype THEN
-            EXTRACT(EPOCH FROM range_end::timestamptz) * 1000000
+            EXTRACT(EPOCH FROM range_end::timestamptz)::bigint * 1000000
        WHEN column_type = 'timestamp'::regtype THEN
-            EXTRACT(EPOCH FROM range_end::timestamp) * 1000000
+            EXTRACT(EPOCH FROM range_end::timestamp)::bigint * 1000000
        WHEN column_type = 'date'::regtype THEN
-            EXTRACT(EPOCH FROM range_end::date) * 1000000
+            EXTRACT(EPOCH FROM range_end::date)::bigint * 1000000
        ELSE
             CASE WHEN range_end IS NULL
-            THEN 9223372036854775807
+            THEN 9223372036854775807::bigint
             ELSE range_end::bigint
             END
        END AS range_end

diff --git a/test/sql/updates/post.repair.sql b/test/sql/updates/post.repair.sql
@@ -0,0 +1,10 @@
+-- This file and its contents are licensed under the Apache License 2.0.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-APACHE for a copy of the license.
+
+-- Re-add the dropped foreign key constraint that was dropped for
+-- repair testing.
+ALTER TABLE _timescaledb_catalog.chunk_constraint
+    ADD CONSTRAINT chunk_constraint_dimension_slice_id_fkey
+    FOREIGN KEY (dimension_slice_id) REFERENCES _timescaledb_catalog.dimension_slice (id);
+
diff --git a/test/sql/updates/setup.check.sql b/test/sql/updates/setup.check.sql
@@ -0,0 +1,22 @@
+-- This file and its contents are licensed under the Apache License 2.0.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-APACHE for a copy of the license.
+
+\echo **** Missing dimension slices ****
+SELECT hypertable_id,
+       (
+	   SELECT format('%I.%I', schema_name, table_name)::regclass
+	   FROM _timescaledb_catalog.hypertable ht
+	   WHERE ht.id = ch.hypertable_id
+       ) AS hypertable,
+       chunk_id,
+       dimension_slice_id,
+       constraint_name,
+       attname AS column_name,
+       pg_get_expr(conbin, conrelid) AS constraint_expr
+FROM _timescaledb_catalog.chunk_constraint cc
+JOIN _timescaledb_catalog.chunk ch ON cc.chunk_id = ch.id
+JOIN pg_constraint ON conname = constraint_name
+JOIN pg_namespace ns ON connamespace = ns.oid AND ns.nspname = ch.schema_name
+JOIN pg_attribute ON attnum = conkey[1] AND attrelid = conrelid
+WHERE dimension_slice_id NOT IN (SELECT id FROM _timescaledb_catalog.dimension_slice);