Merge pull request #1746

cats: fixes BigSqlQuery header fetching

CHANGELOG.md

@@ -71,6 +71,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - stored: fix authentication race condition / deadlock [PR #1732]
 - Fix warning about missing delcandidates table in director [PR #1721]
 - stored: fix not counting files correctly in mac jobs when autoxflate is enabled [PR #1745]
+- cats: fixes BigSqlQuery header fetching [PR #1746]
 
 [PR #1538]: https://github.com/bareos/bareos/pull/1538
 [PR #1581]: https://github.com/bareos/bareos/pull/1581
@@ -129,4 +130,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [PR #1728]: https://github.com/bareos/bareos/pull/1728
 [PR #1732]: https://github.com/bareos/bareos/pull/1732
 [PR #1745]: https://github.com/bareos/bareos/pull/1745
+[PR #1746]: https://github.com/bareos/bareos/pull/1746
 [unreleased]: https://github.com/bareos/bareos/tree/master

core/src/cats/postgresql.cc

@@ -564,6 +564,7 @@ bool BareosDbPostgresql::SqlQueryWithoutHandler(const char* query, int)
   num_rows_ = -1;
   row_number_ = -1;
   field_number_ = -1;
+  fields_fetched_ = false;
 
   if (result_) {
     PQclear(result_); /* hmm, someone forgot to free?? */
@@ -656,6 +657,7 @@ void BareosDbPostgresql::SqlFreeResult(void)
     free(fields_);
     fields_ = NULL;
   }
+  fields_fetched_ = false;
   num_rows_ = num_fields_ = 0;
 }
 
@@ -796,48 +798,58 @@ uint64_t BareosDbPostgresql::SqlInsertAutokeyRecord(const char* query,
   return id;
 }
 
-SQL_FIELD* BareosDbPostgresql::SqlFetchField(void)
+void BareosDbPostgresql::SqlUpdateField(int i)
 {
-  int i, j;
-  int max_length;
-  int this_length;
+  Dmsg1(500, "filling field %d\n", i);
+  fields_[i].name = PQfname(result_, i);
+  fields_[i].type = PQftype(result_, i);
+  fields_[i].flags = 0;
+
+  // For a given column, find the max length.
+  int max_length = 0;
+  int this_length = 0;
+  for (int j = 0; j < num_rows_; j++) {
+    if (PQgetisnull(result_, j, i)) {
+      this_length = 4; /* "NULL" */
+    } else {
+      this_length = cstrlen(PQgetvalue(result_, j, i));
+    }
+
+    if (max_length < this_length) { max_length = this_length; }
+  }
+  fields_[i].max_length = max_length;
 
+  Dmsg4(500,
+        "SqlUpdateField finds field '%s' has length='%d' type='%d' and "
+        "IsNull=%d\n",
+        fields_[i].name, fields_[i].max_length, fields_[i].type,
+        fields_[i].flags);
+}
+
+SQL_FIELD* BareosDbPostgresql::SqlFetchField(void)
+{
   Dmsg0(500, "SqlFetchField starts\n");
 
+  if (field_number_ >= num_fields_) {
+    Dmsg1(100, "requesting field number %d, but only %d fields given\n",
+          field_number_, num_fields_);
+    return nullptr;
+  }
+
   if (!fields_ || fields_size_ < num_fields_) {
+    fields_fetched_ = false;
     if (fields_) {
       free(fields_);
       fields_ = NULL;
     }
     Dmsg1(500, "allocating space for %d fields\n", num_fields_);
     fields_ = (SQL_FIELD*)malloc(sizeof(SQL_FIELD) * num_fields_);
     fields_size_ = num_fields_;
+  }
 
-    for (i = 0; i < num_fields_; i++) {
-      Dmsg1(500, "filling field %d\n", i);
-      fields_[i].name = PQfname(result_, i);
-      fields_[i].type = PQftype(result_, i);
-      fields_[i].flags = 0;
-
-      // For a given column, find the max length.
-      max_length = 0;
-      for (j = 0; j < num_rows_; j++) {
-        if (PQgetisnull(result_, j, i)) {
-          this_length = 4; /* "NULL" */
-        } else {
-          this_length = cstrlen(PQgetvalue(result_, j, i));
-        }
-
-        if (max_length < this_length) { max_length = this_length; }
-      }
-      fields_[i].max_length = max_length;
-
-      Dmsg4(500,
-            "SqlFetchField finds field '%s' has length='%d' type='%d' and "
-            "IsNull=%d\n",
-            fields_[i].name, fields_[i].max_length, fields_[i].type,
-            fields_[i].flags);
-    }
+  if (!fields_fetched_) {
+    for (int i = 0; i < num_fields_; i++) { SqlUpdateField(i); }
+    fields_fetched_ = true;
   }
 
   // Increment field number for the next time around

core/src/cats/postgresql.h

@@ -92,6 +92,7 @@ class BareosDbPostgresql : public BareosDb {
   int SqlAffectedRows(void) override;
   uint64_t SqlInsertAutokeyRecord(const char* query,
                                   const char* table_name) override;
+  void SqlUpdateField(int column);
   SQL_FIELD* SqlFetchField(void) override;
   bool SqlFieldIsNotNull(int field_type) override;
   bool SqlFieldIsNumeric(int field_type) override;
@@ -106,14 +107,16 @@ class BareosDbPostgresql : public BareosDb {
 
   bool CheckDatabaseEncoding(JobControlRecord* jcr);
 
-  int status_ = 0;              /**< Status */
-  int num_fields_ = 0;          /**< Number of fields returned by last query */
-  int rows_size_ = 0;           /**< Size of malloced rows */
-  int fields_size_ = 0;         /**< Size of malloced fields */
-  int row_number_ = 0;          /**< Row number from xx_data_seek */
-  int field_number_ = 0;        /**< Field number from SqlFieldSeek */
-  SQL_ROW rows_ = nullptr;      /**< Defined rows */
-  SQL_FIELD* fields_ = nullptr; /**< Defined fields */
+  int status_ = 0; /**< Status */
+  bool fields_fetched_
+      = false;         /**< Marker, if field descriptions are already fetched */
+  int num_fields_ = 0; /**< Number of fields returned by last query */
+  int rows_size_ = 0;  /**< Size of malloced rows */
+  int fields_size_ = 0;             /**< Size of malloced fields */
+  int row_number_ = 0;              /**< Row number from xx_data_seek */
+  int field_number_ = 0;            /**< Field number from SqlFieldSeek */
+  SQL_ROW rows_ = nullptr;          /**< Defined rows */
+  SQL_FIELD* fields_ = nullptr;     /**< Defined fields */
   bool allow_transactions_ = false; /**< Transactions allowed ? */
   bool transaction_ = false;        /**< Transaction started ? */
 

systemtests/tests/python-bareos/etc/bareos/bareos-dir.d/fileset/NumberedFiles.conf.in

@@ -0,0 +1,11 @@
+FileSet {
+  Name = "NumberedFiles"
+  Description = "fileset to backup some numbered filesjust to backup some files"
+  Include {
+    Options {
+      Signature = XXH128
+      xattrsupport = no
+    }
+    File = "@tmpdir@/backup-data-numbered-files/"
+  }
+}

systemtests/tests/python-bareos/test-setup

@@ -15,6 +15,11 @@ export TestName="$(get_test_name "$0")"
 
 # Fill ${BackupDirectory} with data.
 setup_data
+# Extra backup data with number of files.
+mkdir -p ${tmp}/backup-data-numbered-files
+for i in `seq 1000 2000`; do
+    echo $i > ${tmp}/backup-data-numbered-files/${i}.txt
+done
 
 # Create a list of paths to backup,
 # containing only simple file types.

systemtests/tests/python-bareos/test_list_command.py

@@ -1,7 +1,7 @@
 #
 #   BAREOS - Backup Archiving REcovery Open Sourced
 #
-#   Copyright (C) 2021-2023 Bareos GmbH & Co. KG
+#   Copyright (C) 2021-2024 Bareos GmbH & Co. KG
 #
 #   This program is Free Software; you can redistribute it and/or
 #   modify it under the terms of version three of the GNU Affero General Public
@@ -40,7 +40,7 @@
 import bareos_unittest
 
 
-class PythonBareosListCommandTest(bareos_unittest.Base):
+class PythonBareosListCommandTest(bareos_unittest.Json):
     def test_list_jobs(self):
         """
         verifying `list jobs` and `llist jobs ...` outputs correct data
@@ -55,7 +55,7 @@ def test_list_jobs(self):
             port=self.director_port,
             name=username,
             password=password,
-            **self.director_extra_options
+            **self.director_extra_options,
         )
 
         director.call("run job=backup-bareos-fd yes")
@@ -332,7 +332,7 @@ def test_list_media(self):
             port=self.director_port,
             name=username,
             password=password,
-            **self.director_extra_options
+            **self.director_extra_options,
         )
 
         director.call("run job=backup-bareos-fd yes")
@@ -418,7 +418,7 @@ def test_list_pool(self):
             port=self.director_port,
             name=username,
             password=password,
-            **self.director_extra_options
+            **self.director_extra_options,
         )
 
         director.call("run job=backup-bareos-fd yes")
@@ -509,3 +509,68 @@ def test_list_pool(self):
             result["pools"][0]["poolid"],
             "3",
         )
+
+    def test_list_files(self):
+        """
+        verifying `list files` outputs correct data
+        """
+        logger = logging.getLogger()
+
+        username = self.get_operator_username()
+        password = self.get_operator_password(username)
+
+        director_json = bareos.bsock.DirectorConsoleJson(
+            address=self.director_address,
+            port=self.director_port,
+            name=username,
+            password=password,
+            **self.director_extra_options,
+        )
+
+        jobid = self.run_job(
+            director_json,
+            jobname="backup-bareos-fd",
+            level="Full",
+            extra="fileset=NumberedFiles",
+            wait=True,
+        )
+        result = director_json.call(f"list files jobid={jobid}")
+        # Before fixing https://bugs.bareos.org/view.php?id=1007 the keys could be scrambled after 100 entries.
+        filenames = result["filenames"]
+        min_files = 100
+        self.assertGreater(
+            len(filenames),
+            min_files,
+            f"More than {min_files} files are required for this test, only {len(filenames)} given.",
+        )
+
+        bad_headers = [f for f in filenames if "filename" not in f]
+        self.assertEqual(
+            bad_headers,
+            [],
+            f"Some files were not listed with the correct 'filename' header.",
+        )
+
+        filenames_json = set(f["filename"] for f in filenames)
+
+        director = bareos.bsock.DirectorConsole(
+            address=self.director_address,
+            port=self.director_port,
+            name=username,
+            password=password,
+            **self.director_extra_options,
+        )
+        result = (
+            director.call(f"list files jobid={jobid}")
+            .decode("utf-8")
+            .replace("\n ", "\n")
+        )
+        # start at the first path,
+        # skipping lines like 'Automatically selected Catalog: MyCatalog\nUsing Catalog "MyCatalog"\n'
+        start = result.find("\n/") + 1
+        filenames_plain = set(result[start:].splitlines())
+        self.assertEqual(
+            filenames_json,
+            filenames_plain,
+            "JSON result is not identical to plain (api 0) result.",
+        )