Changeset 32591 for main/trunk/greenstone2/perllib/gssql.pm

Timestamp:

2018-11-09T19:01:04+13:00 (5 years ago)

Author:

ak19

Message:

1. gssql destructor DESTROY doesn't really do anything now, as DBI's destructor will ensure disconnection if not already disconnected. So we don't disconnect in gssql DESTROY, which we used to do but which would only happen on Cancel specifically as other gssql code would already disconnect on natural termination. 2. gssql no longer sets connection parameter fallbacks for those that have fallbacks, since the defaults for these parameters are set by the GS SQL Plugs' configuration options. 3. No need to (html) escape meta/full text when storing in DB or unescape when reading it out of DB, since DB is like a regular text file: can just put text in there. It's only html/xml files where text needs to be stored escaped. 4. Having committed doc.pm::add_utf8_textREF() method in previous commit, GreenstoneSQLPlugin can now pass the fulltxt by reference using this method when constructing the doc_obj from what's read in from the DB.

File:

• main/trunk/greenstone2/perllib/gssql.pm (modified) (8 diffs)

main/trunk/greenstone2/perllib/gssql.pm

@@ -52 +52 @@
 # + TODO: remove unnecessary warn() since PrintError is active
 
-# TODO: drop table if exists and create table if exists are available in MySQL. Use those cmds
+# + 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?
@@ -157 +157 @@
 #
 # 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.
+# "Disconnects the database from the database handle. disconnect is typically only used before exiting 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.
@@ -166 +166 @@
 #
 # 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 {
@@ -171 +172 @@
     
     if (${^GLOBAL_PHASE} eq 'DESTRUCT') {
-    
+
     if ($_dbh_instance) { # database handle still active. Use singleton handle!
-
-        # rollback code has moved to finish_signal_handler() where it belongs?
-        
-        # NOTE: if RaiseError is set on dbi connection, then on any error, perl process will die()
-        # which will end up calling this DESTROY. If it was a die() that called DESTROY
-        # then need to rollback the db here. However, if it was not a die() but natural termination
-        # of the perl process, destroy() will also get called. In that case we don't want to rollback
-        # but do a commit() to the DB instead.
-        # Perhaps detecting the difference may be accomplished by checking ref_count:
-        # - If ref_count not 0 it may require a rollback?
-        # - If ref_count 0 it may be a natural termination and require a commit? Except that ref_count
-        # is set back to 0 in finished(), which will do the commit when ref_count becomes 0. So shouldn't
-        # (have to) do that here.
+                          # dbh instance being active implies build was cancelled
+
+        # rollback code has moved to finish_signal_handler() where it belongs
+        # as rollback() should only happen on cancel/unnatural termination
+        # vs commit() happening in finished() before disconnect, which is natural termination.
+
         
         # We're now finally ready to disconnect, as is required for both natural and premature termination
-        print STDERR "XXXXXXXX Global Destruct: Disconnecting from database\n";
-        $_dbh_instance->disconnect or warn $_dbh_instance->errstr;
-        $_dbh_instance = undef;
-        $ref_count = 0;
+        # (Though natural termination would have disconnected already)
+        # We now leave DBI's own destructor to do the disconnection when perl calls its DESTROY()
+        # We'll just print a message to stop anyone from worrying whether cancelling build
+        # will ensure disconnection still happens. It happens, but silently.
+        print STDERR "   Global Destruct Phase: DBI's own destructor will disconnect database\n";
+        #$_dbh_instance->disconnect or warn $_dbh_instance->errstr;
+        #$_dbh_instance = undef;
+        #$ref_count = 0;
     }
     return;
@@ -246 +244 @@
     }
     if($params_map->{'autocommit'}) {
-        print STDERR "   SQL DB CANCEL SUPPORT OFF.\n";
+        print STDERR "   SQL DB CANCEL SUPPORT OFF.\n" if($params_map->{'verbosity'} > 2);
     } else {
         print STDERR "   SQL DB CANCEL SUPPORT ON.\n";
@@ -259 +257 @@
     my $db_enc = "utf8mb4" if $params_map->{'db_encoding'} eq "utf8";
 
-    # these are the params for connecting to MySQL
-    my $db_driver = $params_map->{'db_driver'} || "mysql";
-    my $db_user = $params_map->{'db_client_user'} || "root";
+    # Params for connecting to MySQL
+    # These params are ensured default/fallback values by the GS SQL Plugs
+    # so no need to set it here
+    my $db_driver = $params_map->{'db_driver'};
+    my $db_host = $params_map->{'db_host'};
+    my $db_user = $params_map->{'db_client_user'};
+
+    # params that can be undef are db_client_pwd and db_port
     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
@@ -355 +357 @@
     if($ref_count == 0) { # Only commit transaction when we're about to actually disconnect, not before
     
-    # TODO: If AutoCommit was off, meaning transactions were on/enabled,
+    # + 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
@@ -521 +523 @@
     }
 
-    # TODO Q: commit here, so that future select statements work?
+    # + TODO Q: commit here, so that future select statements work?
     # See https://metacpan.org/pod/release/TIMB/DBI-1.634_50/DBI.pm#Transactions
 }