From 2b77698c9dfd6210dae4dfd5e0ef6747504af9e5 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Tue, 31 Dec 2024 12:07:18 -0500
Subject: [PATCH] Exit `sqlplus` on error with error code 4

Change the default error code from Oracle `sqlplus` from `SQL.SQLCODE`
to `4`, because otherwise the exit code is returned `modulo 256`,
meaning it can end up `0` for success for an SQL error code like
`ORA-01792`. Selected `4` as the replacement to match the behavior of
Exasol and because `FAILURE` maps to exit code `1`, which has in the
past been more akin to a warning from Sqitch. Resolves #831.
---
 Changes                         |  7 +++++++
 lib/App/Sqitch/Engine/oracle.pm |  2 +-
 t/oracle.t                      | 13 ++++++-------
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Changes b/Changes
index d92818d7..cce015f3 100644
--- a/Changes
+++ b/Changes
@@ -23,6 +23,13 @@ Revision history for Perl extension App::Sqitch
        underlying error loading the engine package and the stack trace remain
        available by using triple verbosity (`-vvv`). Thanks to Martin Fischer
        for the report (#838).
+     - Changed the default error code from Oracle `sqlplus` from `SQL.SQLCODE`
+       to `4`, because otherwise the exit code is returned `modulo 256`,
+       meaning it can end up `0` for success for an SQL error code like
+       `ORA-01792`. Selected `4` as the replacement to match the behavior of
+       Exasol and because `FAILURE` maps to exit code `1`, which has in the
+       past been more akin to a warning from Sqitch. Thanks to @vectro for the
+       report (#831).
 
 1.4.1  2024-02-04T16:35:32Z
      - Removed the quoting of the role and warehouse identifiers that was
diff --git a/lib/App/Sqitch/Engine/oracle.pm b/lib/App/Sqitch/Engine/oracle.pm
index 89c462aa..24d4029f 100644
--- a/lib/App/Sqitch/Engine/oracle.pm
+++ b/lib/App/Sqitch/Engine/oracle.pm
@@ -734,7 +734,7 @@ sub _script {
     return join "\n" => (
         'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
         'WHENEVER OSERROR EXIT 9;',
-        'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+        'WHENEVER SQLERROR EXIT 4;',
         (map {; (my $v = $vars{$_}) =~ s/"/""/g; qq{DEFINE $_="$v"} } sort keys %vars),
         "connect $conn",
         $self->_registry_variable,
diff --git a/t/oracle.t b/t/oracle.t
index 8f7b3585..4407cdc9 100644
--- a/t/oracle.t
+++ b/t/oracle.t
@@ -197,7 +197,7 @@ is_deeply [$ora->sqlplus], [$client, @std_opts],
 is $ora->_script, join( "\n" => (
     'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
     'WHENEVER OSERROR EXIT 9;',
-    'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+    'WHENEVER SQLERROR EXIT 4;',
     'connect ',
     $ora->_registry_variable,
 ) ), '_script should work';
@@ -215,7 +215,7 @@ isa_ok $ora = $CLASS->new(
 is $ora->_script, join( "\n" => (
     'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
     'WHENEVER OSERROR EXIT 9;',
-    'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+    'WHENEVER SQLERROR EXIT 4;',
     'connect fred/"derf"@"blah"',
     $ora->_registry_variable,
 ) ), '_script should assemble connection string';
@@ -233,7 +233,7 @@ isa_ok $ora = $CLASS->new(
 is $ora->_script('@foo'), join( "\n" => (
     'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
     'WHENEVER OSERROR EXIT 9;',
-    'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+    'WHENEVER SQLERROR EXIT 4;',
     'connect fred/"derf"@//there/"blah"',
     $ora->_registry_variable,
     '@foo',
@@ -256,7 +256,7 @@ ok $ora->set_variables(foo => 'baz', whu => 'hi there', yo => q{"stellar"}),
 is $ora->_script, join( "\n" => (
     'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
     'WHENEVER OSERROR EXIT 9;',
-    'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+    'WHENEVER SQLERROR EXIT 4;',
     'DEFINE foo="baz"',
     'DEFINE whu="hi there"',
     'DEFINE yo="""stellar"""',
@@ -278,7 +278,7 @@ isa_ok $ora = $CLASS->new(
 is $ora->_script('@foo'), join( "\n" => (
     'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
     'WHENEVER OSERROR EXIT 9;',
-    'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+    'WHENEVER SQLERROR EXIT 4;',
     'connect /@"secure_user_tns.tpg"',
     $ora->_registry_variable,
     '@foo',
@@ -298,13 +298,12 @@ isa_ok $ora = $CLASS->new(
 is $ora->_script('@foo'), join( "\n" => (
     'SET ECHO OFF NEWP 0 SPA 0 PAGES 0 FEED OFF HEAD OFF TRIMS ON TAB OFF VERIFY OFF',
     'WHENEVER OSERROR EXIT 9;',
-    'WHENEVER SQLERROR EXIT SQL.SQLCODE;',
+    'WHENEVER SQLERROR EXIT 4;',
     'connect /@"wallet_tns_name"',
     $ora->_registry_variable,
     '@foo',
 ) ), '_script should assemble connection string with double-slash and dbname';
 
-
 ##############################################################################
 # Test other configs for the destination.
 $target = App::Sqitch::Target->new(sqitch => $sqitch);