Fixed inability to restore a single database contained in a tablespac…

…e using --db-include.

Fixed by Cynthia Shang.

doc/xml/release.xml

@@ -12,6 +12,16 @@
     <release-list>
         <release date="XXXX-XX-XX" version="2.00dev" title="UNDER DEVELOPMENT">
             <release-core-list>
+                <release-bug-list>
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-contributor id="shang.cynthia"/>
+                        </release-item-contributor-list>
+
+                        <p>Fixed inability to restore a single database contained in a tablespace using --db-include.</p>
+                    </release-item>
+                </release-bug-list>
+
                 <release-feature-list>
                     <release-item>
                         <p>The <cmd>archive-push</cmd> command is now partially coded in C which allows the <postgres/> <file>archive_command</file> to run significantly faster when processing status messages from the asynchronous archive process.</p>

lib/pgBackRest/Manifest.pm

@@ -461,7 +461,8 @@ sub repoPathGet
 
     my $strRepoFile = $strTarget;
 
-    if ($self->isTargetTablespace($strTarget))
+    if ($self->isTargetTablespace($strTarget) &&
+        ($self->dbVersion() >= PG_VERSION_90))
     {
         $strRepoFile .= '/' . $self->tablespacePathGet();
     }

lib/pgBackRest/Restore.pm

@@ -546,11 +546,12 @@ sub clean
         # Check the directory for files
         else
         {
-            # If this is a tablespace search for the special directory that Postgres puts in every tablespace directory
+            # If this is a tablespace search for the special directory that Postgres puts in versions 9.0 and above
             if ($oManifest->isTargetTablespace($strTarget))
             {
                 # Construct the special tablespace path
-                ${$self->{oTargetPath}}{$strTarget} = "${$self->{oTargetPath}}{$strTarget}/" . $oManifest->tablespacePathGet();
+                ${$self->{oTargetPath}}{$strTarget} = "${$self->{oTargetPath}}{$strTarget}" .
+                    (($oManifest->dbVersion() >= PG_VERSION_90) ? "/" . $oManifest->tablespacePathGet() : "");
 
                 # If this path does not exist then skip the rest of the checking - the path will be created later
                 if (!$oStorageDb->pathExists(${$self->{oTargetPath}}{$strTarget}))
@@ -787,9 +788,11 @@ sub build
 
             if ($oManifest->isTargetTablespace($strTarget))
             {
+                # ??? On PG versions < 9.0 this is getting a path above what is really the tblspc if the database is in a tblspc
                 $strPath = dirname($strPath);
             }
 
+            # ??? This may be dead code as the clean function requires the top level target directories to exist
             if (!$oStorageDb->pathExists($strPath))
             {
                 $oStorageDb->pathCreate(
@@ -1128,7 +1131,20 @@ sub process
 
         foreach my $strFile ($oManifest->keys(MANIFEST_SECTION_TARGET_FILE))
         {
-            if ($strFile =~ ('^' . MANIFEST_TARGET_PGDATA . '\/base\/[0-9]+\/PG\_VERSION'))
+            my $strTblspcRegEx;
+            if ($oManifest->dbVersion() < PG_VERSION_90)
+            {
+                $strTblspcRegEx = '^' . MANIFEST_TARGET_PGTBLSPC . '\/[0-9]+\/[0-9]+\/PG\_VERSION';
+            }
+            else
+            {
+                $strTblspcRegEx = '^' . MANIFEST_TARGET_PGTBLSPC .
+                    '\/[0-9]+\/'.$oManifest->tablespacePathGet().'\/[0-9]+\/PG\_VERSION';
+            }
+
+            # Check for DBs not in a tablespace and those that were created in a tablespace
+            if ($strFile =~ ('^' . MANIFEST_TARGET_PGDATA . '\/base\/[0-9]+\/PG\_VERSION') ||
+                $strFile =~ ($strTblspcRegEx))
             {
                 my $lDbId = basename(dirname($strFile));
 
@@ -1190,8 +1206,15 @@ sub process
                 {
                     if ($oManifest->isTargetTablespace($strTarget))
                     {
-                        $strDbFilter .=
-                            '|(^' . $strTarget . '\/' . $oManifest->tablespacePathGet() . '\/' . $strDbKey . '\/)';
+                        if ($oManifest->dbVersion() < PG_VERSION_90)
+                        {
+                            $strDbFilter .= '|(^' . $strTarget . '\/' . $strDbKey . '\/)';
+                        }
+                        else
+                        {
+                            $strDbFilter .=
+                                '|(^' . $strTarget . '\/' . $oManifest->tablespacePathGet() . '\/' . $strDbKey . '\/)';
+                        }
                     }
                 }
             }

test/expect/real-all-001.log

@@ -163,7 +163,7 @@ restore, type 'default', expect exit 40 - path not empty (db-master host)
 ------------------------------------------------------------------------------------------------------------------------------------
 
 restore, type 'default' (db-master host)
-> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf  --db-include=test1 --buffer-size=16384  --link-all --stanza=db restore
+> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf  --db-include=test2 --db-include=test3 --buffer-size=16384  --link-all --stanza=db restore
 ------------------------------------------------------------------------------------------------------------------------------------
 
 + supplemental file: [TEST_PATH]/db-master/db/base/recovery.conf

test/lib/pgBackRestTest/Env/Host/HostDbTest.pm

@@ -257,7 +257,7 @@ sub sqlSelect
     my $hDb = $self->sqlConnect({strDb => $$hParam{strDb}});
 
     # Log and execute the statement
-    &log(DEBUG, "SQL: ${strSql}");
+    &log(DEBUG, (defined($$hParam{strDb}) ? "DB: $$hParam{strDb}, " : "") . "SQL: ${strSql}");
     my $hStatement = $hDb->prepare($strSql);
 
     $hStatement = $hDb->prepare($strSql);
@@ -277,7 +277,11 @@ sub sqlSelect
 ####################################################################################################################################
 sub sqlSelectOne
 {
-    return (shift->sqlSelect(shift))[0];
+    my $self = shift;
+    my $strSql = shift;
+    my $hParam = shift;
+
+    return ($self->sqlSelect($strSql, $hParam))[0];
 }
 
 ####################################################################################################################################
@@ -298,8 +302,8 @@ sub sqlSelectOneTest
 
     do
     {
-        $self->sqlConnect();
-        $strActualValue = $self->sqlSelectOne($strSql);
+        $self->sqlConnect($hParam);
+        $strActualValue = $self->sqlSelectOne($strSql, $hParam);
 
         if (defined($strActualValue) && $strActualValue eq $strExpectedValue)
         {

test/lib/pgBackRestTest/Module/Manifest/ManifestAllTest.pm

@@ -727,6 +727,12 @@ sub run
         $self->testResult(sub {$oManifest->repoPathGet($strTablespace, BOGUS)}, $strTablespace . "/PG_" . PG_VERSION_94 . "_" .
             $iDbCatalogVersion . "/" . BOGUS, 'repoPathGet() - tablespace valid with subpath');
 
+        # Set the DB version to < 9.0 - there is no special sudirectory in earlier PG versions
+        $oManifest->set(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_DB_VERSION, undef, PG_VERSION_84);
+        $self->testResult(sub {$oManifest->repoPathGet($strTablespace, BOGUS)}, $strTablespace . "/" . BOGUS,
+            'repoPathGet() - tablespace in 8.4 valid with subpath');
+        $oManifest->set(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_DB_VERSION, undef, PG_VERSION_94);
+
         # isTargetLink
         #---------------------------------------------------------------------------------------------------------------------------
         $self->testResult(sub {$oManifest->isTargetLink(MANIFEST_TARGET_PGDATA)}, false, "isTargetLink - false");

test/lib/pgBackRestTest/Module/Real/RealAllTest.pm

@@ -599,6 +599,22 @@ sub run
         $oHostDbMaster->sqlExecute("update test set message = '$strDefaultMessage'");
         $oHostDbMaster->sqlWalRotate();
 
+        # Create a database in the tablespace and a table to check
+        $oHostDbMaster->sqlExecute("create database test3 with tablespace ts1", {bAutoCommit => true});
+        $oHostDbMaster->sqlExecute(
+            'create table test3_exists (id int);' .
+            'insert into test3_exists values (1);',
+            {strDb => 'test3', bAutoCommit => true});
+
+        if ($bTestLocal)
+        {
+            # Create a table in test1 to check - test1 will not be restored
+            $oHostDbMaster->sqlExecute(
+                'create table test1_zeroed (id int);' .
+                'insert into test1_zeroed values (1);',
+                {strDb => 'test1', bAutoCommit => true});
+        }
+
         # Start a backup so the next backup has to restart it.  This test is not required for PostgreSQL >= 9.6 since backups
         # are run in non-exlusive mode.
         if ($bTestLocal && $oHostDbMaster->pgVersion() >= PG_VERSION_93 && $oHostDbMaster->pgVersion() < PG_VERSION_96)
@@ -676,6 +692,11 @@ sub run
 
         # Create a table and data in database test2
         #---------------------------------------------------------------------------------------------------------------------------
+
+        # Initialize variables for SHA1 and path of the pg_filenode.map for the database that will not be restored
+        my $strDb1TablePath;
+        my $strDb1TableSha1;
+
         if ($bTestLocal)
         {
             $oHostDbMaster->sqlExecute(
@@ -684,7 +705,14 @@ sub run
                 'create table test_ts1 (id int) tablespace ts1;' .
                 'insert into test_ts1 values (2);',
                 {strDb => 'test2', bAutoCommit => true});
+
             $oHostDbMaster->sqlWalRotate();
+
+            # Get the SHA1 and path of the table for the database that will not be restored
+            $strDb1TablePath =  $oHostDbMaster->dbBasePath(). "/base/" .
+                $oHostDbMaster->sqlSelectOne("select oid from pg_database where datname='test1'") . "/" .
+                $oHostDbMaster->sqlSelectOne("select relfilenode from pg_class where relname='test1_zeroed'", {strDb => 'test1'});
+            $strDb1TableSha1 = storageTest()->hashSize($strDb1TablePath);
         }
 
         # Restore (type = default)
@@ -717,11 +745,41 @@ sub run
         # Now the restore should work
         $oHostDbMaster->restore(
             undef, cfgDefOptionDefault(CFGCMD_RESTORE, CFGOPT_SET),
-            {strOptionalParam => ($bTestLocal ? ' --db-include=test1' : '') . ' --buffer-size=16384'});
+            {strOptionalParam => ($bTestLocal ? ' --db-include=test2 --db-include=test3' : '') . ' --buffer-size=16384'});
+
+        # Test that the first database has not been restored since --db-include did not include test1
+        if ($bTestLocal)
+        {
+            my ($strSHA1, $lSize) = storageTest()->hashSize($strDb1TablePath);
+
+            # Create a zeroed sparse file in the test directory that is the same size as the filenode.map
+            my $strTestTable = $self->testPath() . "/testtable";
+            my $oDestinationFileIo = storageTest()->openWrite($strTestTable);
+            $oDestinationFileIo->open();
+
+            # Truncate to the original size which will create a sparse file.
+            truncate($oDestinationFileIo->handle(), $lSize);
+            $oDestinationFileIo->close();
+
+            # Confirm the test filenode.map and the database test1 filenode.map are zeroed
+            my ($strSHA1Test, $lSizeTest) = storageTest()->hashSize($strTestTable);
+            $self->testResult(sub {($strSHA1Test eq $strSHA1) && ($lSizeTest == $lSize) && ($strSHA1 ne $strDb1TableSha1)},
+                true, 'database test1 not restored');
+        }
 
         $oHostDbMaster->clusterStart();
         $oHostDbMaster->sqlSelectOneTest('select message from test', $bTestLocal ? $strNameMessage : $strIncrMessage);
 
+        # Once the cluster is back online, make sure the database & table in the tablespace exists properly
+        if ($bTestLocal)
+        {
+            $oHostDbMaster->sqlSelectOneTest('select id from test_ts1', 2, {strDb => 'test2'});
+            $oHostDbMaster->sqlDisconnect({strDb => 'test2'});
+
+            $oHostDbMaster->sqlSelectOneTest('select id from test3_exists', 1, {strDb => 'test3'});
+            $oHostDbMaster->sqlDisconnect({strDb => 'test3'});
+        }
+
         # The tablespace path should exist and have files in it
         my $strTablespacePath = $oHostDbMaster->tablespacePath(1);
 
@@ -763,7 +821,7 @@ sub run
         $oHostDbMaster->sqlSelectOneTest("select count(*) from test_exists", 0);
         $oHostDbMaster->sqlExecute('drop table test_exists');
 
-        # Now it should be OK to drop database test2
+        # Now it should be OK to drop database test2 and test3
         if ($bTestLocal)
         {
             $oHostDbMaster->sqlExecute('drop database test2', {bAutoCommit => true});
@@ -781,6 +839,7 @@ sub run
         }
 
         # And drop the tablespace
+        $oHostDbMaster->sqlExecute('drop database test3', {bAutoCommit => true});
         $oHostDbMaster->sqlExecute("drop tablespace ts1", {bAutoCommit => true});
 
         # Restore (restore type = immediate, inclusive)