Index: main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32576)
+++ main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32577)
@@ -43,17 +43,9 @@
 # back in from the sql db while the remainder is to be read back in from the docsql .xml files.
 
-# TODO: deal with incremental vs removeold. If docs removed from import folder, then import step
-# won't delete it from archives but buildcol step will. Need to implement this with this database plugin or wherever the actual flow is
-
-# TODO Q: is "reindex" = del from db + add to db?
-# - is this okay for reindexing, or will it need to modify existing values (update table)
-# - if it's okay, what does reindex need to accomplish (and how) if the OID changes because hash id produced is different?
-# - delete is accomplished in GS SQL Plugin, during buildcol.pl. When should reindexing take place?
-# during SQL plugout/import.pl or during plugin? If adding is done by GSSQLPlugout, does it need to
-# be reimplemented in GSSQLPlugin to support the adding portion of reindexing.
-
 # TODO: Add public instructions on using this plugin and its plugout: start with installing mysql binary, changing pwd, running the server (and the client against it for checking: basic cmds like create and drop). Then discuss db name, table names (per coll), db cols and col types, and how the plugout and plugin work.
 # Discuss the plugin/plugout parameters.
 
+# TODO, test on windows and mac.
+# Note: if parsing fails (e.g. using wrong plugout like GS XML plugout, which chokes on args intended for SQL plugout) then SQL plugin init would have already been called and done connection, but disconnect would not have been done because SQL plugin disconnect would not have been called upon parse failure.
 
 # DONE:
@@ -78,5 +70,37 @@
 # effect that if the db doesn't exist, gssql::use_db() fails, as it won't create db.
 #   This got fixed when GSSQLPlugin stopped connecting on init().
-
+#
+#
+#+ TODO: deal with incremental vs removeold. If docs removed from import folder, then import step
+# won't delete it from archives but buildcol step will. Need to implement this with this database plugin or wherever the actual flow is.
+#
+# + TODO Q: is "reindex" = del from db + add to db?
+# - is this okay for reindexing, or will it need to modify existing values (update table)
+# - if it's okay, what does reindex need to accomplish (and how) if the OID changes because hash id produced is different?
+# - delete is accomplished in GS SQL Plugin, during buildcol.pl. When should reindexing take place?
+# during SQL plugout/import.pl or during plugin? If adding is done by GSSQLPlugout, does it need to
+# be reimplemented in GSSQLPlugin to support the adding portion of reindexing.
+#
+# INCREMENTAL REBUILDING IMPLEMENTED CORRECTLY AND WORKS:
+# Overriding plugins' remove_all() method covered removeold.
+# Overriding plugins' remove_one() method is all I needed to do for reindex and deletion
+# (incremental and non-incremental) to work.
+# but doing all this needed an overhaul of gssql.pm and its use by the GS SQL plugin and plugout.
+# - needed to correct plugin.pm::remove_some() to process all files
+# - and needed to correct GreenstoneSQLPlugin::close_document() to setOID() after all
+# All incremental import and buildcol worked after that:
+# - deleting files and running incr-import and incr-buildcol (= "incr delete"),
+# - deleting files and running incr-import and buildcol (="non-incr delete")
+# - modifying meta and doing an incr rebuild
+# - modifying fulltext and doing an incr rebuild
+# - renaming a file forces a reindex: doc is removed from db and added back in, due to remove_one()
+# - tested CSV file: adding some records, changing some records
+#    + CSVPlugin test (collection csvsql)
+#    + MetadataCSVPlugin test (modified collection sqltest to have metadata.csv refer to the
+#      filenames of sqltest's documents)
+#    + shared image test (collection shareimg): if 2 html files reference the same image, the docs
+#      are indeed both reindexed if the image is modified (e.g. I replaced the image with another
+#      of the same name) which in the GS SQL plugin/plugout case is that the 2 docs are deleted
+#      and added in again.
 
 ########################################################################################
@@ -189,4 +213,6 @@
     my ($pluginfo, $base_dir, $processor, $maxdocs) = @_;
 
+    $self->SUPER::remove_all(@_);
+    
     print STDERR "   Building with removeold option set, so deleting current collection's tables if they exist\n" if($self->{'verbosity'});
     
@@ -227,7 +253,9 @@
            # SO DON'T RETURN IF CAN'T_PROCESS_THIS_FILE
     
+    
+    my $gs_sql = $self->{'gs_sql'} || return 0; # couldn't make the connection or no db etc
+
     print STDERR "*****************************\nAsked to remove_one oid\n***********************\n";
-    
-    my $gs_sql = $self->{'gs_sql'} || return 0; # couldn't make the connection or no db etc
+    print STDERR "Num oids: " . scalar (@$oids) . "\n";
     
     my $proc_mode = $self->{'process_mode'};
@@ -368,12 +396,8 @@
 
 
-# TODO: only want to work with sql db if buildcol.pl. Unfortunately, also runs on import.pl.
-# During import, the GS SQL Plugin is called before the GS SQL Plugout with undesirable side
-# effect that if the db doesn't exist, gssql::use_db() fails, as it won't create db.
-
 # GS SQL Plugin::init() (and deinit()) is called by import.pl and also by buildcol.pl
 # This means it connects and deconnects during import.pl as well. This is okay
-# as removeold, which should drop the collection tables, happens during the import phase
-# and therefore also requires a db connection.
+# as removeold, which should drop the collection tables, happens during the import phase,
+# calling GreenstoneSQLPlugin::and therefore also requires a db connection.
 # TODO: Eventually can try moving get_gssql_instance into gssql.pm? That way both GS SQL Plugin
 # and Plugout would be using one connection during import.pl phase when both plugs exist.
