Catch script_hash unique constraint violations

Originally added in a04be11 and updated in 4d63e2c, the unique violations have been working fine in most situations, but were confusing when hit, as they returned as raw SQL errors. So modify `log_deploy_change` to catch database exceptions, check for unique constraint violations, and return the more useful error message `Cannot log change "{change}": The deploy script is not unique`. Requires adding a new method to each engine, `_unique_error`, which checks error codes from the DBI to determine whether the most recent error was a unique violation. The default implementation always returns false for the database engines that don't enforce unique constraints (Exasol and Snowflake). Add a test to DBIEngineTest to trigger the unique violation. This required fixing some tests that previously always passed when they shouldn't (thanks to the use of `try` without `catch`) and fixing them to testing things properly. Also requires a new parameter to `DBIEngineTest->run()` to disable the unique constraint test for engines that don't enforce that constraint (Exasol and Snowflake). While at it, always enable primary key and unique constraints on Vertica. They previously were enforced only by database-level configuration, and unique constraints only on Vertica 7.2 and later with that configuration. Enforcement will only be enabled by default for new registry databases. Drop support for versions of Vertica prior to 7.2, which don't support the syntax to enable constraints. Also drop support for versions of SQLite prior to 3.8.6, when unique constraint enforcement was added to SQLite. Resolves #630.
sqitchers · Jul 1, 2023 · 9b4327a · 9b4327a
1 parent c6ccc30
commit 9b4327a
Show file tree

Hide file tree

Showing 23 changed files with 200 additions and 106 deletions.
diff --git a/.github/workflows/sqlite.yml b/.github/workflows/sqlite.yml
@@ -12,7 +12,7 @@ jobs:
     strategy:
       matrix:
         # https://sqlite.org/chronology.html
-        sqlite: [3.40.1, 3.39.4, 3.38.5, 3.37.2, 3.36.0, 3.35.5, 3.34.1, 3.33.0, 3.32.3, 3.31.1, 3.30.1, 3.29.0, 3.28.0, 3.27.2, 3.26.0, 3.25.3, 3.24.0, 3.23.1, 3.22.0, 3.21.0, 3.20.1, 3.19.3, 3.18.0, 3.17.0, 3.16.2, 3.15.2, 3.14.2, 3.13.0, 3.12.2, 3.11.1, 3.10.2, 3.9.2, 3.8.11.1, 3.7.17]
+        sqlite: [3.40.1, 3.39.4, 3.38.5, 3.37.2, 3.36.0, 3.35.5, 3.34.1, 3.33.0, 3.32.3, 3.31.1, 3.30.1, 3.29.0, 3.28.0, 3.27.2, 3.26.0, 3.25.3, 3.24.0, 3.23.1, 3.22.0, 3.21.0, 3.20.1, 3.19.3, 3.18.0, 3.17.0, 3.16.2, 3.15.2, 3.14.2, 3.13.0, 3.12.2, 3.11.1, 3.10.2, 3.9.2, 3.8.11.1]
     name: 💡 SQLite ${{ matrix.sqlite }}
     runs-on: ubuntu-latest
     steps:

diff --git a/.github/workflows/vertica.yml b/.github/workflows/vertica.yml
@@ -20,7 +20,6 @@ jobs:
          - { label: '8.1',  version: 8.1.1-0,  image: cjonesy/docker-vertica, db: docker }
          - { label: '8.0',  version: 8.0.0-0,  image: cjonesy/docker-vertica, db: docker }
          - { label: '7.2',  version: 7.2.3-18, image: cjonesy/docker-vertica, db: docker }
-         - { label: '7.1',  version: 7.1.2-21, image: cjonesy/docker-vertica, db: docker }
     name: 🔺 Vertica ${{ matrix.label }}
     runs-on: ubuntu-latest
     services:

diff --git a/Changes b/Changes
@@ -50,6 +50,17 @@ Revision history for Perl extension App::Sqitch
        (#668)!
      - Fixed Snowflake and MySQL to properly raise errors on session query
        failures immediately after connection.
+     - Fixed the handling of unique violations for deploy script hash
+       uniqueness so that it no longer returns a database error but properly
+       reports the issue in a more human-friendly error message. Thanks to
+       Stefan Badenhorst for the reminder (#630).
+     - Updated the registry SQL scripts for Vertica to always enable primary
+       key and unique constraints. Unique constraints are now enabled for
+       all database engines except Exasol and Snowflake.
+     - Dropped support for Vertica 7.1, as unique constraint enforcement was
+       not added until Vertica 7.2.
+     - Increased minimum SQLite versions to 3.8.6, when unique constraint
+       enforcement was added.
 
 1.3.1  2022-10-01T18:49:30Z
      - Fixed a bug introduced in v1.3.0 where the Postgres engine would

diff --git a/README.md b/README.md
@@ -14,12 +14,12 @@ App/Sqitch version v1.3.2-dev
 *   [PostgreSQL] 8.4+
 *   [YugabyteDB] 2.6+
 *   [CockroachDB] 21+
-*   [SQLite][lite] 3.7.11+
+*   [SQLite][lite] 3.8.6+
 *   [MySQL][my] 5.1+
 *   [MariaDB] 10.0+
 *   [Oracle][orcl] 10g+,
 *   [Firebird][bird] 2.0+
-*   [Vertica][vert] 6.0+
+*   [Vertica][vert] 7.2+
 *   [Exasol][exa] 6.0+
 *   [Snowflake][flake]
 
@@ -181,15 +181,15 @@ SOFTWARE.
   [MySQL]:     https://github.com/sqitchers/sqitch/actions/workflows/mysql.yml/badge.svg
   [🐬]:        https://github.com/sqitchers/sqitch/actions/workflows/mysql.yml "Tested with MySQL 5.5–8 and MariaDB 10.0–10.6"
   [SQLite]:    https://github.com/sqitchers/sqitch/actions/workflows/sqlite.yml/badge.svg
-  [💡]:        https://github.com/sqitchers/sqitch/actions/workflows/sqlite.yml "Tested with SQLite 3.7–3.36"
+  [💡]:        https://github.com/sqitchers/sqitch/actions/workflows/sqlite.yml "Tested with SQLite 3.8.6–3.40"
   [Debian]:    https://img.shields.io/debian/v/sqitch?label=%F0%9F%8D%A5%20Debian
   [🍥]:        https://packages.debian.org/stable/sqitch "Latest version on Debian"
   [Postgres]:  https://github.com/sqitchers/sqitch/actions/workflows/pg.yml/badge.svg
   [🐘]:        https://github.com/sqitchers/sqitch/actions/workflows/pg.yml "Tested with PostgreSQL 9.3–14"
   [Yugabyte]:  https://github.com/sqitchers/sqitch/actions/workflows/yugabyte.yml/badge.svg
   [💫]:        https://github.com/sqitchers/sqitch/actions/workflows/yugabyte.yml "Tested with YugabyteDB 2.6–2.13"
   [Vertica]:   https://github.com/sqitchers/sqitch/actions/workflows/vertica.yml/badge.svg
-  [🔺]:        https://github.com/sqitchers/sqitch/actions/workflows/vertica.yml "Tested with Vertica 7.1–11.0"
+  [🔺]:        https://github.com/sqitchers/sqitch/actions/workflows/vertica.yml "Tested with Vertica 7.2–12.0"
   [Cockroach]: https://github.com/sqitchers/sqitch/actions/workflows/cockroach.yml/badge.svg
   [🪳]:        https://github.com/sqitchers/sqitch/actions/workflows/cockroach.yml "Tested with CockroachDB v21-22"
 

diff --git a/lib/App/Sqitch/Engine/Upgrade/vertica-1.0.sql b/lib/App/Sqitch/Engine/Upgrade/vertica-1.0.sql
@@ -1,5 +1,5 @@
 CREATE TABLE :"registry".releases (
-    version         FLOAT          PRIMARY KEY,
+    version         FLOAT          PRIMARY KEY ENABLED,
     installed_at    TIMESTAMPTZ    NOT NULL DEFAULT clock_timestamp(),
     installer_name  VARCHAR(1024)  NOT NULL,
     installer_email VARCHAR(1024)  NOT NULL
@@ -10,6 +10,6 @@ COMMENT ON TABLE  :"registry".releases IS 'Sqitch registry releases.';
 -- Add the script_hash column to the changes table. Copy change_id for now.
 ALTER TABLE :"registry".changes ADD COLUMN script_hash CHAR(40);
 UPDATE :"registry".changes SET script_hash = change_id;
-ALTER TABLE :"registry".changes ADD UNIQUE(script_hash);
+ALTER TABLE :"registry".changes ADD UNIQUE(script_hash) ENABLED;
 
 COMMENT ON SCHEMA :"registry" IS 'Sqitch database deployment metadata v1.0.';
diff --git a/lib/App/Sqitch/Engine/Upgrade/vertica-1.1.sql b/lib/App/Sqitch/Engine/Upgrade/vertica-1.1.sql
@@ -1,3 +1,3 @@
 ALTER TABLE :"registry".changes DROP CONSTRAINT c_unique;
-ALTER TABLE :"registry".changes ADD UNIQUE(project, script_hash);
+ALTER TABLE :"registry".changes ADD UNIQUE(project, script_hash) ENABLED;
 COMMENT ON SCHEMA :"registry" IS 'Sqitch database deployment metadata v1.1.';
diff --git a/lib/App/Sqitch/Engine/exasol.pm b/lib/App/Sqitch/Engine/exasol.pm
@@ -468,6 +468,11 @@ sub _no_column_error  {
     return $DBI::errstr && $DBI::errstr =~ /object \w+ not found/m;
 }
 
+sub _unique_error  {
+    # Unique constraints not supported by Exasol
+    return 0;
+}
+
 sub _script {
     my $self = shift;
     my $uri  = $self->uri;

diff --git a/lib/App/Sqitch/Engine/exasol.sql b/lib/App/Sqitch/Engine/exasol.sql
@@ -42,7 +42,7 @@ CREATE TABLE &registry..changes (
     planned_at      TIMESTAMP WITH LOCAL TIME ZONE NOT NULL,
     planner_name    VARCHAR2(512 CHAR)       NOT NULL,
     planner_email   VARCHAR2(512 CHAR)       NOT NULL
-    -- UNIQUE(project, script_hash)
+    -- UNIQUE(project, script_hash) -- not supported in EXASOL
 );
 
 COMMENT ON TABLE  &registry..changes                 IS 'Tracks the changes currently deployed to the database.';

diff --git a/lib/App/Sqitch/Engine/firebird.pm b/lib/App/Sqitch/Engine/firebird.pm
@@ -294,6 +294,10 @@ sub _no_column_error  {
     return $DBI::errstr && $DBI::errstr =~ /^-Column unknown/m;
 }
 
+sub _unique_error  {
+    return $DBI::errstr && $DBI::errstr =~ /no 2 table rows can have duplicate column values$/m;
+}
+
 sub _regex_op { 'SIMILAR TO' }               # NOT good match for
                                              # REGEXP :(
 
@@ -812,23 +816,31 @@ sub log_deploy_change {
         planner_email
         committed_at
     ));
-    $dbh->do(qq{
-        INSERT INTO changes (
-            $cols
-        )
-        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, $ts)
-    }, undef,
-        $id,
-        $change->script_hash,
-        $name,
-        $proj,
-        $change->note,
-        $user,
-        $email,
-        $self->_char2ts( $change->timestamp ),
-        $change->planner_name,
-        $change->planner_email,
-    );
+    try {
+        $dbh->do(qq{
+            INSERT INTO changes (
+                $cols
+            )
+            VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, $ts)
+        }, undef,
+            $id,
+            $change->script_hash,
+            $name,
+            $proj,
+            $change->note,
+            $user,
+            $email,
+            $self->_char2ts( $change->timestamp ),
+            $change->planner_name,
+            $change->planner_email,
+        );
+    } catch {
+        hurl engine => __x(
+            'Cannot log change "{change}": The deploy script is not unique',
+            change => $name,
+        ) if $self->_unique_error;
+        die $_;
+    };
 
     if ( my @deps = $change->dependencies ) {
         foreach my $dep (@deps) {

diff --git a/lib/App/Sqitch/Engine/mysql.pm b/lib/App/Sqitch/Engine/mysql.pm
@@ -358,6 +358,10 @@ sub _no_column_error  {
     return $DBI::state && $DBI::state eq '42S22' && $DBI::err == '1054'; # ER_BAD_FIELD_ERROR
 }
 
+sub _unique_error  {
+    return $DBI::state && $DBI::state eq '23000' && $DBI::err == '1062'; # ER_DUP_ENTRY
+}
+
 sub _regex_op { 'REGEXP' }
 
 sub _limit_default { '18446744073709551615' }

diff --git a/lib/App/Sqitch/Engine/oracle.pm b/lib/App/Sqitch/Engine/oracle.pm
@@ -695,6 +695,10 @@ sub _no_column_error  {
     return $DBI::err && $DBI::err == 904; # ORA-00904: invalid identifier
 }
 
+sub _unique_error  {
+    return $DBI::err && $DBI::err == 1; # ORA-00001: unique constraint violated
+}
+
 sub _script {
     my $self = shift;
     my $uri  = $self->uri;

diff --git a/lib/App/Sqitch/Engine/pg.pm b/lib/App/Sqitch/Engine/pg.pm
@@ -462,10 +462,15 @@ sub _no_table_error  {
     return 1;
 }
 
+# https://www.postgresql.org/docs/current/errcodes-appendix.html
 sub _no_column_error  {
     return $DBI::state && $DBI::state eq '42703'; # undefined_column
 }
 
+sub _unique_error {
+    return $DBI::state && $DBI::state eq '23505'; # unique_violation
+}
+
 sub _in_expr {
     my ($self, $vals) = @_;
     return '= ANY(?)', $vals;

diff --git a/lib/App/Sqitch/Engine/snowflake.pm b/lib/App/Sqitch/Engine/snowflake.pm
@@ -316,6 +316,13 @@ sub _no_column_error  {
     return $DBI::state && $DBI::state eq '42703'; # ERRCODE_UNDEFINED_COLUMN
 }
 
+sub _unique_error  {
+    # https://docs.snowflake.com/en/sql-reference/constraints-overview
+    # Snowflake supports defining and maintaining constraints, but does not
+    # enforce them, except for NOT NULL constraints, which are always enforced.
+    return 0;
+}
+
 sub _ts2char_format {
     # The colon has to be inside the quotation marks, because otherwise it
     # generates wayward single quotation marks. Bug report:

diff --git a/lib/App/Sqitch/Engine/sqlite.pm b/lib/App/Sqitch/Engine/sqlite.pm
@@ -95,9 +95,9 @@ has dbh => (
         # Make sure we support this version.
         my @v = split /[.]/ => $dbh->{sqlite_version};
         hurl sqlite => __x(
-            'Sqitch requires SQLite 3.7.11 or later; DBD::SQLite was built with {version}',
+            'Sqitch requires SQLite 3.8.6 or later; DBD::SQLite was built with {version}',
             version => $dbh->{sqlite_version}
-        ) unless $v[0] > 3 || ($v[0] == 3 && ($v[1] > 7 || ($v[1] == 7 && $v[2] >= 11)));
+        ) unless $v[0] > 3 || ($v[0] == 3 && ($v[1] > 8 || ($v[1] == 8 && $v[2] >= 6)));
 
         return $dbh;
     }
@@ -175,6 +175,10 @@ sub _no_column_error  {
     return $DBI::errstr && $DBI::errstr =~ /^\Qno such column:/;
 }
 
+sub _unique_error  {
+    return $DBI::errstr && $DBI::errstr =~ /^\QUNIQUE constraint failed:/;
+}
+
 sub _regex_op { 'REGEXP' }
 
 sub _limit_default { -1 }

diff --git a/lib/App/Sqitch/Engine/vertica.pm b/lib/App/Sqitch/Engine/vertica.pm
@@ -162,9 +162,6 @@ sub _run_registry_file {
     );
     (my $sql = scalar $file->slurp) =~ s{:"registry"}{$schema}g;
 
-    # No LONG VARCHAR before Vertica 7.
-    $sql =~ s/LONG //g if $maj < 7;
-
     # Write out the temporary file.
     require File::Temp;
     my $fh = File::Temp->new;
@@ -183,6 +180,10 @@ sub _no_column_error  {
     return $DBI::state && $DBI::state eq '42703'; # ERRCODE_UNDEFINED_COLUMN
 }
 
+sub _unique_error  {
+    return $DBI::state && $DBI::state eq '23505'; # ERRCODE_UNIQUE_VIOLATION
+}
+
 sub _dt($) {
     require App::Sqitch::DateTime;
     return App::Sqitch::DateTime->new(split /:/ => shift);

diff --git a/lib/App/Sqitch/Engine/vertica.sql b/lib/App/Sqitch/Engine/vertica.sql
@@ -3,27 +3,27 @@ CREATE SCHEMA :"registry";
 COMMENT ON SCHEMA :"registry" IS 'Sqitch database deployment metadata v1.1.';
 
 CREATE TABLE :"registry".releases (
-    version         FLOAT          PRIMARY KEY,
+    version         FLOAT          PRIMARY KEY ENABLED,
     installed_at    TIMESTAMPTZ    NOT NULL DEFAULT clock_timestamp(),
     installer_name  VARCHAR(1024)  NOT NULL,
     installer_email VARCHAR(1024)  NOT NULL
 );
 
-COMMENT ON TABLE  :"registry".releases                 IS 'Sqitch registry releases.';
+COMMENT ON TABLE  :"registry".releases IS 'Sqitch registry releases.';
 
 CREATE TABLE :"registry".projects (
-    project         VARCHAR(1024) PRIMARY KEY ENCODING AUTO,
-    uri             VARCHAR(1024) NULL UNIQUE,
+    project         VARCHAR(1024) PRIMARY KEY ENABLED ENCODING AUTO,
+    uri             VARCHAR(1024) NULL UNIQUE ENABLED,
     created_at      TIMESTAMPTZ   NOT NULL DEFAULT clock_timestamp(),
     creator_name    VARCHAR(1024) NOT NULL,
     creator_email   VARCHAR(1024) NOT NULL
 );
 
-COMMENT ON TABLE :"registry".projects                 IS 'Sqitch projects deployed to this database.';
+COMMENT ON TABLE :"registry".projects IS 'Sqitch projects deployed to this database.';
 
 CREATE TABLE :"registry".changes (
-    change_id       CHAR(40)       PRIMARY KEY ENCODING AUTO,
-    script_hash     CHAR(40)           NULL UNIQUE,
+    change_id       CHAR(40)       PRIMARY KEY ENABLED ENCODING AUTO,
+    script_hash     CHAR(40)           NULL UNIQUE ENABLED,
     change          VARCHAR(1024)  NOT NULL,
     project         VARCHAR(1024)  NOT NULL REFERENCES :"registry".projects(project),
     note            VARCHAR(65000) NOT NULL DEFAULT '',
@@ -35,10 +35,10 @@ CREATE TABLE :"registry".changes (
     planner_email   VARCHAR(1024)  NOT NULL
 );
 
-COMMENT ON TABLE :"registry".changes                  IS 'Tracks the changes currently deployed to the database.';
+COMMENT ON TABLE :"registry".changes IS 'Tracks the changes currently deployed to the database.';
 
 CREATE TABLE :"registry".tags (
-    tag_id          CHAR(40)       PRIMARY KEY ENCODING AUTO,
+    tag_id          CHAR(40)       PRIMARY KEY ENABLED ENCODING AUTO,
     tag             VARCHAR(1024)  NOT NULL,
     project         VARCHAR(1024)  NOT NULL REFERENCES :"registry".projects(project),
     change_id       CHAR(40)       NOT NULL REFERENCES :"registry".changes(change_id),
@@ -49,20 +49,20 @@ CREATE TABLE :"registry".tags (
     planned_at      TIMESTAMPTZ    NOT NULL,
     planner_name    VARCHAR(1024)  NOT NULL,
     planner_email   VARCHAR(1024)  NOT NULL,
-    UNIQUE(project, tag)
+    UNIQUE(project, tag) ENABLED
 );
 
-COMMENT ON TABLE :"registry".tags                  IS 'Tracks the tags currently applied to the database.';
+COMMENT ON TABLE :"registry".tags IS 'Tracks the tags currently applied to the database.';
 
 CREATE TABLE :"registry".dependencies (
     change_id       CHAR(40)      NOT NULL REFERENCES :"registry".changes(change_id),
     type            VARCHAR(8)    NOT NULL ENCODING AUTO,
     dependency      VARCHAR(2048) NOT NULL,
     dependency_id   CHAR(40)      NULL REFERENCES :"registry".changes(change_id),
-    PRIMARY KEY (change_id, dependency)
+    PRIMARY KEY (change_id, dependency) ENABLED
 );
 
-COMMENT ON TABLE :"registry".dependencies                IS 'Tracks the currently satisfied dependencies.';
+COMMENT ON TABLE :"registry".dependencies IS 'Tracks the currently satisfied dependencies.';
 
 CREATE TABLE :"registry".events (
     event           VARCHAR(6)     NOT NULL ENCODING AUTO,
@@ -79,7 +79,7 @@ CREATE TABLE :"registry".events (
     planned_at      TIMESTAMPTZ    NOT NULL,
     planner_name    VARCHAR(1024)  NOT NULL,
     planner_email   VARCHAR(1024)  NOT NULL,
-    PRIMARY KEY (change_id, committed_at)
+    PRIMARY KEY (change_id, committed_at) ENABLED
 );
 
-COMMENT ON TABLE :"registry".events                  IS 'Contains full history of all deployment events.';
+COMMENT ON TABLE :"registry".events  IS 'Contains full history of all deployment events.';