Index: main/trunk/greenstone2/perllib/gssql.pm
===================================================================
--- main/trunk/greenstone2/perllib/gssql.pm	(revision 32585)
+++ main/trunk/greenstone2/perllib/gssql.pm	(revision 32588)
@@ -33,16 +33,40 @@
 use DBI; # the central package for this module used by GreenstoneSQL Plugout and Plugin
 
+#################
+# Database functions that use the perl DBI module (with the DBD driver module for mysql)
+# https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm
+# https://metacpan.org/pod/DBD::mysql
+#################
 
 ##############################
 
-# TODO: add infrastructure for db_port, AutoCommit etc
+# TODO Q: If disconnect is automatically called when object destroyed, what does that mean
+# for our file-global handle object, is disconnect only called at end of perl process?
+# Does that mean we don't need to explicitly call disconnect in gssql object's destroy during
+# the GLOBAL destruction phase?
+# https://perldoc.perl.org/perlobj.html#Destructors
+
+#+ TODO: add infrastructure for db_port, AutoCommit etc
 # For port, see https://stackoverflow.com/questions/2248665/perl-script-to-connect-to-mysql-server-port-3307
 
 # + TODO: remove unnecessary warn() since PrintError is active
-# https://perldoc.perl.org/perlobj.html#Destructors
 
 # TODO: drop table if exists and create table if exists are available in MySQL. Use those cmds
 # instead of always first checking for existence ourselves? Only when subclassing to specific
-# mysql class.
+# mysql class?
+
+
+# + TODO Q: What on cancelling a build: delete table? But what if it was a rebuild and the rebuild is cancelled (not the original build)?
+# Do we create a copy of the orig database as backup, then start populating current db, and if cancelled, delete current db and RENAME backup table to current? 
+# https://stackoverflow.com/questions/3280006/duplicating-a-mysql-table-indexes-and-data
+# BUT what if the table is HUGE? (Think of a collection with millions of docs.) Huge overhead in copying?
+# The alternative is we just quit on cancel, but then: cancel could leave the table in a partial committed state, with no way of rolling back.
+# Unless they do a full rebuild, which will recreate the table from scratch?
+# SOLUTION-> rollback transaction on error, see https://www.effectiveperlprogramming.com/2010/07/set-custom-dbi-error-handlers/
+# In that case, should set AutoCommit to off on connection, and remember to commit at end.
+
+# + TODO: Consider AutoCommit status (and Autocommit off allowing commit or rollback for GS coll build cancel) later
+
+
 ##############################
 
@@ -64,6 +88,10 @@
 # - db_name (which is the GS3 sitename, or "greenstone2" for GS2)
 
-
-
+# Database access related functions
+# http://g2pc1.bu.edu/~qzpeng/manual/MySQL%20Commands.htm
+# https://www.guru99.com/insert-into.html
+
+# Add signal handlers to cleanup and disconnect from db on sudden termination, incl cancel build
+# https://www.perl.com/article/37/2013/8/18/Catch-and-Handle-Signals-in-Perl/
 $SIG{INT}  = \&finish_signal_handler;
 $SIG{TERM}  = \&finish_signal_handler;
@@ -75,5 +103,5 @@
     if ($_dbh_instance) { # database handle (note, using singleton) still active.
 	
-	# TODO: If autocommit wasn't set, then this is a cancel operation.
+	# 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
@@ -126,4 +154,5 @@
 #
 # Useful: https://perldoc.perl.org/perlobj.html#Destructors
+# For more on when destroy is called, see https://www.perlmonks.org/?node_id=1020920
 #
 # https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#disconnect
@@ -169,27 +198,10 @@
     # "Always include a call to $self->SUPER::DESTROY in our destructors (even if we don't yet have any base/parent classes). (p. 145)"
     # Superclass and destroy, call to SUPER: https://www.perlmonks.org/?node_id=879920
-    # also covers multiple-inheritance (MI)    
+    # discussion also covers multiple-inheritance (MI)    
     $self->SUPER::DESTROY if $self->can("SUPER::DESTROY");
 
 }
 
-#################################
-
-# Database access related functions
-# http://g2pc1.bu.edu/~qzpeng/manual/MySQL%20Commands.htm
-# https://www.guru99.com/insert-into.html
-
-# TODO Q: What on cancelling a build: delete table? But what if it was a rebuild and the rebuild is cancelled (not the original build)?
-# Do we create a copy of the orig database as backup, then start populating current db, and if cancelled, delete current db and RENAME backup table to current? 
-# https://stackoverflow.com/questions/3280006/duplicating-a-mysql-table-indexes-and-data
-# BUT what if the table is HUGE? (Think of a collection with millions of docs.) Huge overhead in copying?
-# The alternative is we just quit on cancel, but then: cancel could leave the table in a partial committed state, with no way of rolling back.
-# Unless they do a full rebuild, which will recreate the table from scratch?
-# SOLUTION-> rollback transaction on error, see https://www.effectiveperlprogramming.com/2010/07/set-custom-dbi-error-handlers/
-# But then should set AutoCommit to off on connection, and remember to commit every time
-
-#################
-# Database functions that use the perl DBI module (with the DBD driver module for mysql)
-#################
+
 
 ################### BASIC DB OPERATIONS ##################
@@ -198,6 +210,4 @@
 # NOTE: FULLTEXT is a reserved keyword in (My)SQL. So we can't name a table or any of its columns "fulltext".
 # https://dev.mysql.com/doc/refman/5.5/en/keywords.html
-
-# TODO: Consider AutoCommit status (and Autocommit off allowing commit or rollback for GS coll build cancel) later
 
 
@@ -279,4 +289,5 @@
     # 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
+    # More: https://www.oreilly.com/library/view/programming-the-perl/1565926994/re44.html
     my $autocommit = (defined $params_map->{'autocommit'}) ? $params_map->{'autocommit'} : 1;
     
