Index: /main/trunk/greenstone2/perllib/gssql.pm
===================================================================
--- /main/trunk/greenstone2/perllib/gssql.pm	(revision 32579)
+++ /main/trunk/greenstone2/perllib/gssql.pm	(revision 32580)
@@ -33,4 +33,16 @@
 use DBI; # the central package for this module used by GreenstoneSQL Plugout and Plugin
 
+##############################
+
+# 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?
+##############################
+
 # singleton connection
 my $_dbh_instance = undef; # calls undef() function. See https://perlmaven.com/undef-and-defined-in-perl
@@ -49,13 +61,4 @@
 # Parameterise (one or more methods may use them):
 # - db_name (which is the GS3 sitename)
-
-# 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?
 
 sub new
@@ -160,14 +163,21 @@
     my $db_driver = $params_map->{'db_driver'} || "mysql";
     my $db_user = $params_map->{'db_client_user'} || "root";
-    my $db_pwd = $params_map->{'db_client_pwd'}; # even if undef, we'll see a sensible error message
-                                           # when connect fails
+    my $db_pwd = $params_map->{'db_client_pwd'}; # even if undef and password was necessary,
+                                     # we'll see a sensible error message when connect fails
     my $db_host = $params_map->{'db_host'} || "127.0.0.1";
-    # localhost doesn't work for us, but 127.0.0.1 works
-    # https://metacpan.org/pod/DBD::mysql
-    # "The hostname, if not specified or specified as '' or 'localhost', will default to a MySQL server
-    # running on the local machine using the default for the UNIX socket. To connect to a MySQL server
-    # on the local machine via TCP, you must specify the loopback IP address (127.0.0.1) as the host."
+        # localhost doesn't work for us, but 127.0.0.1 works
+        # https://metacpan.org/pod/DBD::mysql
+        # "The hostname, if not specified or specified as '' or 'localhost', will default to a MySQL server
+        # running on the local machine using the default for the UNIX socket. To connect to a MySQL server
+        # on the local machine via TCP, you must specify the loopback IP address (127.0.0.1) as the host."
+    my $db_port = $params_map->{'db_port'}; # leave as undef if unspecified,
+                 # as our tests never used port anyway (must have internally
+                 # defaulted to whatever default port is used for MySQL)
+
+    
     #my $connect_str = "dbi:$db_driver:database=$db_name;host=$db_host";
-    my $connect_str = "dbi:$db_driver:host=$db_host"; # don't provide db - allows checking the db exists later when loading the db
+    # But don't provide db now - this allows checking the db exists later when loading the db
+    my $connect_str = "dbi:$db_driver:host=$db_host";
+    $connect_str .= ";port=$db_port" if $db_port;
 
     if($params_map->{'verbosity'}) {
@@ -375,24 +385,26 @@
 }
 
+
+sub delete_collection_tables {
+    my $self= shift (@_);
+    my $dbh = $self->{'db_handle'};
+
+    # drop table <tablename>
+    my $table = $self->get_metadata_table_name();
+    if($self->table_exists($table)) {
+	$dbh->do("drop table $table");# || warn("@@@ Couldn't delete $table");
+    }
+    $table = $self->get_fulltext_table_name();
+    if($self->table_exists($table)) {
+	$dbh->do("drop table $table");# || warn("@@@ Couldn't delete $table");
+    }
+}
+
+# Don't call this: it will delete the meta and full text tables for ALL collections in $db_name (localsite by default)!
+# This method is just here for debugging (for testing creating a database when there is none)
+#
 # "IF EXISTS is used to prevent an error from occurring if the database does not exist. ... DROP DATABASE returns the number of tables that were removed. The DROP DATABASE statement removes from the given database directory those files and directories that MySQL itself may create during normal operation.Jun 20, 2012"
 # MySQL 8.0 Reference Manual :: 13.1.22 DROP DATABASE Syntax
 # https://dev.mysql.com/doc/en/drop-database.html
-sub delete_collection_tables {
-    my $self= shift (@_);
-    my $dbh = $self->{'db_handle'};
-    
-    # drop table <tablename>
-    my $table = $self->get_metadata_table_name();
-    if($self->table_exists($table)) {
-	$dbh->do("drop table $table") || warn("@@@ Couldn't delete $table");
-    }
-    $table = $self->get_fulltext_table_name();
-    if($self->table_exists($table)) {
-	$dbh->do("drop table $table") || warn("@@@ Couldn't delete $table");
-    }
-}
-
-# Don't call this: it will delete the meta and full text tables for ALL collections in $db_name (localsite by default)!
-# This method is just here for debugging (for testing creating a database when there is none)
 sub _delete_database {
     my $self= shift (@_);
@@ -436,10 +448,10 @@
 sub insert_row_into_metadata_table {
     my $self = shift (@_);
-    my ($doc_oid, $section_name, $meta_name, $escaped_meta_value, $debug_only, $debug_out) = @_;
+    my ($doc_oid, $section_name, $meta_name, $escaped_meta_value, $debug_only) = @_;
     
     my $dbh = $self->{'db_handle'};
     
     my $tablename = $self->get_metadata_table_name();
-    my $sth = $dbh->prepare_cached(qq{INSERT INTO $tablename (did, sid, metaname, metavalue) VALUES (?, ?, ?, ?)}) || warn("Could not prepare insert statement for metadata table\n");
+    my $sth = $dbh->prepare_cached(qq{INSERT INTO $tablename (did, sid, metaname, metavalue) VALUES (?, ?, ?, ?)});# || warn("Could not prepare insert statement for metadata table\n");
 
     # Now we're ready to execute the command, unless we're only debugging
@@ -447,5 +459,5 @@
     if($debug_only) {
 	# just print the statement we were going to execute
-	print $debug_out $sth->{'Statement'} . "($doc_oid, $section_name, $meta_name, $escaped_meta_value)\n";
+	print STDERR $sth->{'Statement'} . "($doc_oid, $section_name, $meta_name, $escaped_meta_value)\n";
     }
     else {
@@ -463,17 +475,17 @@
     my $self = shift (@_);
     #my ($did, $sid, $fulltext) = @_;
-    my ($doc_oid, $section_name, $section_textref, $debug_only, $debug_out) = @_;
+    my ($doc_oid, $section_name, $section_textref, $debug_only) = @_;
     
     my $dbh = $self->{'db_handle'};
     
     my $tablename = $self->get_fulltext_table_name();
-    my $sth = $dbh->prepare_cached(qq{INSERT INTO $tablename (did, sid, fulltxt) VALUES (?, ?, ?)}) || warn("Could not prepare insert statement for fulltxt table\n");
+    my $sth = $dbh->prepare_cached(qq{INSERT INTO $tablename (did, sid, fulltxt) VALUES (?, ?, ?)});# || warn("Could not prepare insert statement for fulltxt table\n");
     
     # Now we're ready to execute the command, unless we're only debugging
 
-    # just print the statement we were going to execute, minus the fulltxt value
+    # don't display the fulltxt value as it could be too long
     my $txt_repr = $$section_textref ? "<TXT>" : "NULL";    
-    if($debug_only) {
-	print $debug_out $sth->{'Statement'} . "($doc_oid, $section_name, $txt_repr)\n";
+    if($debug_only) { # only print statement, don't execute it
+	print STDERR $sth->{'Statement'} . "($doc_oid, $section_name, $txt_repr)\n";
     }
     else {	
@@ -481,5 +493,5 @@
 	
 	$sth->execute($doc_oid, $section_name, $$section_textref)
-	    || warn ("Unable to write fulltxt row to db for row:\n\tOID $doc_oid, section $section_name");
+	    || warn ("Unable to write fulltxt row to db for row:\n\tOID $doc_oid, section $section_name"); # Execution failure will print out info anyway: since db connection sets PrintError
     }
 }
Index: /main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm
===================================================================
--- /main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32579)
+++ /main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32580)
@@ -427,5 +427,5 @@
     my $gs_sql = new gssql({
 	'collection_name' => $ENV{'GSDLCOLLECTION'},
-	'verbosity' => $self->{'verbosity'}
+	'verbosity' => $self->{'verbosity'} || 0
 			   });
 
Index: /main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm
===================================================================
--- /main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm	(revision 32579)
+++ /main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm	(revision 32580)
@@ -42,4 +42,6 @@
 # TODO Q: what about verbosity for debugging, instead of current situation of printing out upon debug set at the expense of writing to db
 # TODO Q: introduced site_name param to plugins and plugouts. Did I do it right? And should they have hiddengli = "yes"
+# Did I do the pass by ref in docprint's escape and unescape textref functions correctly, and how they're called here?
+#   Any more optimisation I can do around this?
 
 # this plugout does not output the metadata and/or fulltxt xml to a file,
@@ -49,13 +51,13 @@
 }
 
-# NOTTODO: die() statements need to be replaced with premature_termination
+# + NOTTODO: die() statements need to be replaced with premature_termination
 # which should ensure the GreenstoneXMLPlugin (group)'s stuff is closed and cleaned up SOMEHOW
 # It's fine: the die() stmts all take place before setting up the super class' begin
 
-# TODO Q: about build_mode: how to detect removeold. Now handled by 
+# + TODO Q: about build_mode: how to detect removeold. Now handled by 
 #   GreenstoneSQLPlugout::remove_all(), which is inherited from a base plugin.
-# TODO: deal with -removeold and everything? Or type out instructions for user
-
-# TODO Q: what is "group" in GreenstoneXMLPlugout?
+# + TODO: deal with -removeold and everything? Or type out instructions for user
+
+# + TODO Q: what is "group" in GreenstoneXMLPlugout?
 
 my $process_mode_list =
@@ -127,5 +129,5 @@
 
     my $self= shift (@_);
-
+    
     # The saveas.options
     #print STDERR "@@@@ PLUGOUT db_pwd: " . $self->{'db_client_pwd'} . "\n";
@@ -143,5 +145,5 @@
     my $db_params = {
 	'collection_name' => $ENV{'GSDLCOLLECTION'},
-	'verbosity' => 1
+	'verbosity' => $self->{'verbosity'} || 0
     };
 
@@ -226,5 +228,5 @@
 }
  
-# TODO: check arc-inf.db for whether each entry is to be deleted/indexed/reindexed/been indexed?
+# + X TODO: check arc-inf.db for whether each entry is to be deleted/indexed/reindexed/been indexed?
 # That's only for indexing, not for this step which only generates the content in archives dir
 sub saveas {
@@ -315,5 +317,5 @@
 	    # from db (unlike for reading back in from doc.xml)
 	    my $escaped_meta_value = &docprint::escape_text($data->[1]);
-
+	    
 	    # Write out the current section's meta to collection db's METADATA table	    
 	    
@@ -322,8 +324,7 @@
 	    # filling in the values
 	    # OR if debugging, then it will print the SQL insert statement but not execute it
-
-	    $gs_sql->insert_row_into_metadata_table($doc_oid, $section_name, $meta_name, $escaped_meta_value, $self->{'debug'}, $debug_out);	    
-
-	    }
+	    
+	    $gs_sql->insert_row_into_metadata_table($doc_oid, $section_name, $meta_name, $escaped_meta_value, $self->{'debug'});
+	}
     }
     
@@ -337,5 +338,5 @@
 	# The following will do the SQL insertion
 	# or if debug, the following will print the SQL insert stmt without executing it
-	$gs_sql->insert_row_into_fulltxt_table($doc_oid, $section_name, $section_textref, $self->{'debug'}, $debug_out);
+	$gs_sql->insert_row_into_fulltxt_table($doc_oid, $section_name, $section_textref, $self->{'debug'});
     
     }
