Index: main/trunk/greenstone2/perllib/gssql.pm
===================================================================
--- main/trunk/greenstone2/perllib/gssql.pm	(revision 32581)
+++ main/trunk/greenstone2/perllib/gssql.pm	(revision 32582)
@@ -33,17 +33,5 @@
 use DBI; # the central package for this module used by GreenstoneSQL Plugout and Plugin
 
-#$SIG{INT}  = sub { die "Caught a sigint $!" };
-#$SIG{TERM}  = sub { die "Caught a sigterm $!" };
-#$SIG{KILL}  = sub { die "Caught a sigkill $!" };
-
-
-$SIG{INT}  = \&finish_signal_handler;
-$SIG{TERM}  = \&finish_signal_handler;
-$SIG{KILL}  = \&finish_signal_handler;
-
-sub finish_signal_handler {
-    my ($sig) = @_; # one of INT|KILL|TERM
-    die "Caught a $sig signal $!"; # will call destructor
-}
+
 ##############################
 
@@ -75,4 +63,29 @@
 # - db_name (which is the GS3 sitename)
 
+
+
+$SIG{INT}  = \&finish_signal_handler;
+$SIG{TERM}  = \&finish_signal_handler;
+$SIG{KILL}  = \&finish_signal_handler;
+
+sub finish_signal_handler {
+    my ($sig) = @_; # one of INT|KILL|TERM
+
+    if ($_dbh_instance) { # database handle (note, using singleton) still active.
+	
+	# TODO: If autocommit wasn't set, then this is a cancel operation.
+	# If we've not disconnected from the sql db yet and if we've not committed
+	# transactions yet, then cancel means we do a rollback here
+	
+	if($_dbh_instance->{AutoCommit} == 0) {
+	    print STDERR "   User cancelled: rolling back SQL database transaction.\n";
+	    $_dbh_instance->rollback(); # will warn on failure, nothing more we can/want to do,
+	}
+    }
+
+    
+    die "Caught a $sig signal $!"; # die() will always call destructor (sub DESTROY)
+}
+
 sub new
 {  
@@ -111,10 +124,38 @@
 # We want to ensure we've closed the db connection in such cases.
 # "Itâs common to call die when handling SIGINT and SIGTERM. die is useful because it will ensure that Perl stops correctly: for example Perl will execute a destructor method if present when die is called, but the destructor method will not be called if a SIGINT or SIGTERM is received and no signal handler calls die."
+#
 # https://perldoc.perl.org/perlobj.html#Destructors
+#
+# https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#disconnect
+# "Disconnects the database from the database handle. disconnect is typically only used before exitin# g the program. The handle is of little use after disconnecting.
+#
+# The transaction behaviour of the disconnect method is, sadly, undefined. Some database systems (such as Oracle and Ingres) will automatically commit any outstanding changes, but others (such as Informix) will rollback any outstanding changes. Applications not using AutoCommit should explicitly call commit or rollback before calling disconnect.
+#
+# The database is automatically disconnected by the DESTROY method if still connected when there are no longer any references to the handle. The DESTROY method for each driver should implicitly call rollback to undo any uncommitted changes. This is vital behaviour to ensure that incomplete transactions don't get committed simply because Perl calls DESTROY on every object before exiting. Also, do not rely on the order of object destruction during "global destruction", as it is undefined.
+#
+# Generally, if you want your changes to be committed or rolled back when you disconnect, then you should explicitly call "commit" or "rollback" before disconnecting.
+#
+# If you disconnect from a database while you still have active statement handles (e.g., SELECT statement handles that may have more data to fetch), you will get a warning. The warning may indicate that a fetch loop terminated early, perhaps due to an uncaught error. To avoid the warning call the finish method on the active handles."
+#
 sub DESTROY {
     my $self = shift;
 
     if (${^GLOBAL_PHASE} eq 'DESTRUCT') {
-	if ($_dbh_instance) {
+	
+	if ($_dbh_instance) { # database handle still active. Use singleton handle!
+
+	    # THIS CODE HAS MOVED TO finish_signal_handler() WHERE IT BELONGS
+	    # If autocommit wasn't set, then this is a cancel operation.
+	    # If we've not disconnected from the sql db yet and if we've not committed
+	    # transactions yet, then cancel means we do a rollback here
+
+	    # if($_dbh_instance->{AutoCommit} == 0) {
+		
+	    # 	$_dbh_instance->rollback(); # will warn on failure, nothing more we can/want to do,
+            #     # don't do a die() here: possibility of infinite loop and we still want to disconnect
+	    # }
+
+	    # Either way, we're now finally ready to disconnect as is required for premature
+	    # termination too
 	    print STDERR "XXXXXXXX Global Destruct: Disconnecting from database\n";
 	    $_dbh_instance->disconnect or warn $_dbh_instance->errstr;
@@ -182,5 +223,16 @@
     #my $self= shift (@_); # singleton method doesn't use self, but callers don't need to know that
     my ($params_map) = @_;
-
+    
+    if($params_map->{'verbosity'}) {
+	if(!defined $params_map->{'autocommit'}) {
+	    print STDERR "  Autocommit parameter not defined\n";
+	}
+	if($params_map->{'autocommit'}) {
+	    print STDERR "   SQL DB UNDO SUPPORT OFF.\n";
+	} else {
+	    print STDERR "   SQL DB UNDO SUPPORT ON.\n";
+	}
+    }
+    
     return $_dbh_instance if($_dbh_instance);
 
@@ -217,4 +269,8 @@
 	print STDERR "\nAssuming the mysql server has been started with: --character_set_server=utf8mb4\n" if $db_driver eq "mysql";
     }
+
+    # DBI AutoCommit connection param is on/1 by default, so if a value for this is not defined
+    # as a method parameter to _get_connection_instance, then fallback to the default of on/1
+    my $autocommit = (defined $params_map->{'autocommit'}) ? $params_map->{'autocommit'} : 1;
     
     my $dbh = DBI->connect("$connect_str", $db_user, $db_pwd,
@@ -223,5 +279,5 @@
 			       PrintError => 1, # on by default, but being explicit
 			       RaiseError => 0, # off by default, but being explicit
-			       AutoCommit => 1, # on by default, but being explicit
+			       AutoCommit => $autocommit,
 			       mysql_enable_utf8mb4 => 1 # tells MySQL to use UTF-8 for communication and tells DBD::mysql to decode the data, see https://stackoverflow.com/questions/46727362/perl-mysql-utf8mb4-issue-possible-bug 
 			   });
@@ -273,8 +329,37 @@
     my $self= shift (@_);
 
+    # TODO: if AutoCommit was off, meaning transactions were on/enabled,
+    # then here is where we commit our one long transaction.
+    # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
+    my $rc = 1;    
+    
     $ref_count--;
     if($ref_count == 0) {
+	# Only commit transaction when we're about to disconnect, not before
+	# If autocommit was on, then we'd have committed after every db operation, so nothing to do
+	$rc = $self->do_commit_if_on();
+	
 	$self->force_disconnect_from_db();
-    }    
+    }
+
+    return $rc;
+}
+
+sub do_commit_if_on {
+    my $self= shift (@_);
+    my $dbh = $self->{'db_handle'};
+    
+    my $rc = 1; # return code: everything went fine, regardless of whether we needed to commit
+                # (AutoCommit on or off)
+
+    # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#commit
+    if($dbh->{AutoCommit} == 0) {
+	print STDERR "   Committing transaction to SQL database now.\n" if $self->{'verbosity'};
+	$rc = $dbh->commit() or warn("SQL DB COMMIT FAILED: " . $dbh->errstr); # important problem
+	                                                        # worth embellishing error message
+    }
+    # If autocommit was on, then we'd have committed after every db operation, so nothing to do
+
+    return $rc;
 }
 
@@ -429,4 +514,7 @@
 	$dbh->do("drop table $table");# || warn("@@@ Couldn't delete $table");
     }
+
+    # TODO Q: commit here, so that future select statements work?
+    # See https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#Transactions
 }
 
Index: main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32581)
+++ main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32582)
@@ -137,4 +137,10 @@
         'desc' => "{GreenstoneSQLPlug.process_mode.all}" } ];
 
+my $rollback_on_cancel_list =
+    [ { 'name' => "true",
+        'desc' => "{GreenstoneSQLPlug.rollback_on_cancel}" },      
+      { 'name' => "false",
+        'desc' => "{GreenstoneSQLPlug.rollbacl_on_cancel}" } ];
+
 my $arguments =
     [ { 'name' => "process_exp",
@@ -149,4 +155,11 @@
 	'deft' => "all",
 	'reqd' => "no"},
+      { 'name' => "rollback_on_cancel", 
+	'desc' => "{GreenstoneSQLPlug.rollback_on_cancel}",
+	'type' => "enum",
+	'list' => $rollback_on_cancel_list,
+	'deft' => "true", # TODO Q: what's the better default? If "true", any memory concerns?
+	'reqd' => "no",
+	'hiddengli' => "no"},
       { 'name' => "db_driver", 
 	'desc' => "{GreenstoneSQLPlug.db_driver}",
@@ -204,4 +217,6 @@
 }
 
+###### Called during import.pl
+
 # This is called once if removeold is set with import.pl. Most plugins will do
 # nothing but if a plugin does any stuff outside of creating doc obj, then 
@@ -230,7 +245,13 @@
 	$gs_sql->ensure_fulltxt_table_exists();
     }
-}
-
-# This is called per document for docs that have been deleted from the 
+
+    # UNNECESSARY
+    # The removeold related DB transaction (deleting collection tables) is complete
+    # Don't let GS SQL PlugIN interfere with GS SQL PlugOUT's database transactions
+    # during import.pl hereafter. Finish up.
+    #$gs_sql->do_commit_if_on();
+}
+
+# This is called during import.pl per document for docs that have been deleted from the 
 # collection. Most plugins will do nothing
 # but if a plugin does any stuff outside of creating doc obj, then it may need
@@ -273,4 +294,5 @@
 }
 
+#### Called during buildcol
 
 sub xml_start_tag {
@@ -395,4 +417,5 @@
 }
 
+#### Called during buildcol and import
 
 # GS SQL Plugin::init() (and deinit()) is called by import.pl and also by buildcol.pl
@@ -429,4 +452,7 @@
 	'verbosity' => $self->{'verbosity'} || 0
 			   });
+    
+    # if autocommit is set, there's no rollback support
+    my $autocommit = ($self->{'rollback_on_cancel'} eq "false") ? 1 : 0;
 
     # try connecting to the mysql db, die if that fails
@@ -435,5 +461,6 @@
 	'db_client_user' => $self->{'db_client_user'},
 	'db_client_pwd' => $self->{'db_client_pwd'},
-	'db_host' => $self->{'db_host'}
+	'db_host' => $self->{'db_host'},
+	'autocommit' => $autocommit
 			       })
 	)
Index: main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm	(revision 32581)
+++ main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm	(revision 32582)
@@ -38,4 +38,6 @@
 use DBI; # the central package for this plugout
 
+
+# This entire class is called only during import.pl
 
 # TODO: SIGTERM rollback and disconnect?
@@ -69,4 +71,10 @@
         'desc' => "{GreenstoneSQLPlug.process_mode.all}" } ];
 
+my $rollback_on_cancel_list =
+    [ { 'name' => "true",
+        'desc' => "{GreenstoneSQLPlug.rollback_on_cancel}" },      
+      { 'name' => "false",
+        'desc' => "{GreenstoneSQLPlug.rollbacl_on_cancel}" } ];
+
 # The following are the saveas.options:
 my $arguments = [ 
@@ -76,4 +84,11 @@
       'list' => $process_mode_list,
       'deft' => "all",
+      'reqd' => "no",
+      'hiddengli' => "no"},
+    { 'name' => "rollback_on_cancel", 
+      'desc' => "{GreenstoneSQLPlug.rollback_on_cancel}",
+      'type' => "enum",
+      'list' => $rollback_on_cancel_list,
+      'deft' => "true", # TODO Q: what's the better default? If "true", any memory concerns?
       'reqd' => "no",
       'hiddengli' => "no"},
@@ -146,7 +161,11 @@
 	'collection_name' => $ENV{'GSDLCOLLECTION'},
 	'verbosity' => $self->{'verbosity'} || 0
+	
     };
 
     my $gs_sql = new gssql($db_params);
+
+    # if autocommit is set, there's no rollback support
+    my $autocommit = ($self->{'rollback_on_cancel'} eq "false") ? 1 : 0;
     
     # try connecting to the mysql db, die if that fails
@@ -156,5 +175,6 @@
 	'db_client_user' => $self->{'db_client_user'},
 	'db_client_pwd' => $self->{'db_client_pwd'},
-	'db_host' => $self->{'db_host'}
+	'db_host' => $self->{'db_host'},
+	'autocommit' => $autocommit
 			       })
 	)
@@ -164,4 +184,6 @@
 	die("Could not connect to db. Can't proceed.\n");
     }
+
+    #die("@@@@ TEST. Connected successfully. Testing gssql::destructor.\n"); # WORKS
     
     my $db_name = $self->{'site_name'} || "greenstone2"; # one database per GS3 site, for GS2 the db is called greenstone2
Index: main/trunk/greenstone2/perllib/strings.properties
===================================================================
--- main/trunk/greenstone2/perllib/strings.properties	(revision 32581)
+++ main/trunk/greenstone2/perllib/strings.properties	(revision 32582)
@@ -1460,4 +1460,6 @@
 GreenstoneSQLPlug.db_host:The hostname on which the (My)SQL database server is running, 127.0.0.1 by default. Other values to try include localhost.
 
+GreenstoneSQLPlug.rollback_on_cancel:Support for undo on cancel. Set to true to support rollbacks on cancel. Transactions are then only committed to the database at the end of import and buildcol. Set to false if you do not want undo support, in which case SQL statements are autocommitted to the database.
+
 #
 # Perl module strings
