BUG#35895247 "One-file-per-thread" inadequate for single files with escaped wildcards

When importing a single file which contained only escaped wildcards
characters, util.importTable() was loading the whole file using a single
thread. With this fix, that file is loaded in chunks by multiple
threads.

Change-Id: I75f4eba97f1aa54ac7274e6d20d8f4165ea31595
(cherry picked from commit 191093263d5ab1582d4c8ce83a1ab5dfd9e75d93)

Author: Paweł Andruszkiewicz

modules/util/import_table/import_table_options.cc

@@ -40,6 +40,7 @@
 #include "mysqlshdk/libs/storage/ifile.h"
 #include "mysqlshdk/libs/utils/strformat.h"
 #include "mysqlshdk/libs/utils/utils_file.h"
+#include "mysqlshdk/libs/utils/utils_general.h"
 #include "mysqlshdk/libs/utils/utils_path.h"
 
 namespace {
@@ -212,9 +213,24 @@ void Import_table_option_pack::set_decode_columns(
   }
 }
 
-bool Import_table_option_pack::check_if_multifile() const {
+bool Import_table_option_pack::check_if_multifile() {
   if (m_filelist_from_user.size() == 1) {
-    return has_wildcard(m_filelist_from_user[0]);
+    if (storage_config() ||
+        shcore::OperatingSystem::WINDOWS != shcore::get_os_type()) {
+      // local non-Windows and remote filesystems allow for * and ? characters
+      // in file names
+      // BUG#35895247: if all wildcard characters are escaped, load the file in
+      // chunks
+      if (auto path = shcore::unescape_glob(m_filelist_from_user[0]);
+          path.has_value()) {
+        m_filelist_from_user[0] = std::move(*path);
+        return false;
+      } else {
+        return true;
+      }
+    } else {
+      return has_wildcard(m_filelist_from_user[0]);
+    }
   }
 
   return true;

modules/util/import_table/import_table_options.h

@@ -146,7 +146,7 @@ class Import_table_option_pack {
   void set_bytes_per_chunk(const std::string &value);
   void set_max_rate(const std::string &value);
   void on_unpacked_options();
-  bool check_if_multifile() const;
+  bool check_if_multifile();
 
  protected:
   std::vector<std::string> m_filelist_from_user;

mysqlshdk/libs/utils/utils_general.cc

@@ -1118,6 +1118,10 @@ std::string to_string(OperatingSystem os_type) {
 
 namespace {
 
+constexpr char k_glob_escape = '\\';
+constexpr char k_glob_match_single = '?';
+constexpr char k_glob_match_many = '*';
+
 /**
  * https://research.swtch.com/glob
  */
@@ -1134,23 +1138,25 @@ bool _match_glob(const std::string_view pat, const std::string_view str) {
       char c = pat[px];
 
       switch (c) {
-        case '?':
+        case k_glob_match_single:
           if (sx < send) {
             ++px;
             ++sx;
             continue;
           }
           break;
 
-        case '*':
+        case k_glob_match_many:
           ppx = px;
           psx = sx + 1;
           ++px;
           continue;
 
-        case '\\':
-          // if '\' is followed by * or ?, it's an escape sequence, skip '\'
-          if ((px + 1) < pend && (pat[px + 1] == '*' || pat[px + 1] == '?')) {
+        case k_glob_escape:
+          // if '\' is followed by \, * or ?, it's an escape sequence, skip '\'
+          if ((px + 1) < pend && (pat[px + 1] == k_glob_escape ||
+                                  pat[px + 1] == k_glob_match_many ||
+                                  pat[px + 1] == k_glob_match_single)) {
             ++px;
             c = pat[px];
           }
@@ -1168,6 +1174,11 @@ bool _match_glob(const std::string_view pat, const std::string_view str) {
     }
 
     if (0 < psx && psx <= send) {
+      // original code has here:
+      //  px = ppx;
+      //  sx = psx;
+      // which jumps back to the '*' case, we skip than one iteration by setting
+      // the correct values right away
       px = ppx + 1;
       sx = psx;
       ++psx;
@@ -1200,6 +1211,42 @@ bool match_glob(const std::string_view pattern, const std::string_view s,
   return _match_glob(pattern, s);
 }
 
+std::optional<std::string> unescape_glob(const std::string_view pattern) {
+  const auto length = pattern.length();
+  std::string result;
+  result.reserve(length);
+
+  for (auto i = decltype(length){0}; i < length; ++i) {
+    auto c = pattern[i];
+
+    switch (c) {
+      case k_glob_match_single:
+      case k_glob_match_many:
+        // unescaped wildcard
+        return std::nullopt;
+
+      case k_glob_escape:
+        if (i + 1 < length) {
+          if (const auto cc = pattern[i + 1]; k_glob_escape == cc ||
+                                              k_glob_match_single == cc ||
+                                              k_glob_match_many == cc) {
+            // escaped wildcard, skip escape character and copy wildcard
+            // verbatim
+            ++i;
+            c = cc;
+          }
+        }
+        [[fallthrough]];
+
+      default:
+        result.push_back(c);
+        break;
+    }
+  }
+
+  return result;
+}
+
 const char *get_long_version() {
   return "Ver " MYSH_FULL_VERSION " for " SYSTEM_TYPE " on " MACHINE_TYPE
          " - for MySQL " LIBMYSQL_VERSION " (" MYSQL_COMPILATION_COMMENT ")";

mysqlshdk/libs/utils/utils_general.h

@@ -28,6 +28,7 @@
 #include <chrono>
 #include <functional>
 #include <list>
+#include <optional>
 #include <set>
 #include <sstream>
 #include <string>
@@ -196,6 +197,18 @@ bool SHCORE_PUBLIC match_glob(const std::string_view pattern,
                               const std::string_view s,
                               bool case_sensitive = false);
 
+/**
+ * Replaces all escaped wildcard characters with their unescaped counterparts.
+ * If pattern already contains an unescaped wildcard characters, does not return
+ * a value.
+ *
+ * @param pattern A pattern to process.
+ *
+ * @returns Processed pattern or no value.
+ */
+std::optional<std::string> SHCORE_PUBLIC
+unescape_glob(const std::string_view pattern);
+
 std::string SHCORE_PUBLIC to_camel_case(std::string_view name);
 std::string SHCORE_PUBLIC from_camel_case(std::string_view name);
 std::string SHCORE_PUBLIC from_camel_case_to_dashes(std::string_view name);

unittest/mysqlshdk/libs/utils/utils_general_t.cc

@@ -512,6 +512,7 @@ TEST(utils_general, match_glob) {
   EXPECT_TRUE(match_glob("\\", "\\"));
   EXPECT_FALSE(match_glob("\\", "x"));
   EXPECT_TRUE(match_glob("\\opti*", "\\option"));
+  EXPECT_TRUE(match_glob("\\\\opti*", "\\option"));
   EXPECT_FALSE(match_glob("\\opti\\*", "\\option"));
   EXPECT_TRUE(match_glob("*\\*", "abcdefgh*"));
 
@@ -555,6 +556,39 @@ TEST(utils_general, match_glob) {
   EXPECT_TRUE(match_glob("*x", "xxx"));
 }
 
+TEST(utils_general, unescape_glob) {
+  EXPECT_EQ("", unescape_glob(""));
+
+  EXPECT_EQ(std::nullopt, unescape_glob("*"));
+  EXPECT_EQ(std::nullopt, unescape_glob("?"));
+  EXPECT_EQ("\\", unescape_glob("\\"));
+  EXPECT_EQ("a", unescape_glob("a"));
+
+  EXPECT_EQ(std::nullopt, unescape_glob("\\??"));
+  EXPECT_EQ(std::nullopt, unescape_glob("\\?*"));
+  EXPECT_EQ("?\\", unescape_glob("\\?\\"));
+  EXPECT_EQ("?\\", unescape_glob("\\?\\\\"));
+  EXPECT_EQ("?a", unescape_glob("\\?a"));
+  EXPECT_EQ("??", unescape_glob("\\?\\?"));
+  EXPECT_EQ("?*", unescape_glob("\\?\\*"));
+
+  EXPECT_EQ(std::nullopt, unescape_glob("\\*?"));
+  EXPECT_EQ(std::nullopt, unescape_glob("\\**"));
+  EXPECT_EQ("*\\", unescape_glob("\\*\\"));
+  EXPECT_EQ("*\\", unescape_glob("\\*\\\\"));
+  EXPECT_EQ("*a", unescape_glob("\\*a"));
+  EXPECT_EQ("*?", unescape_glob("\\*\\?"));
+  EXPECT_EQ("**", unescape_glob("\\*\\*"));
+
+  EXPECT_EQ("\\option", unescape_glob("\\option"));
+  EXPECT_EQ(std::nullopt, unescape_glob("\\optio*"));
+  EXPECT_EQ(std::nullopt, unescape_glob("\\optio?"));
+  EXPECT_EQ("\\optio?", unescape_glob("\\optio\\?"));
+  EXPECT_EQ("\\optio*", unescape_glob("\\optio\\*"));
+  EXPECT_EQ("\\optio\\", unescape_glob("\\optio\\"));
+  EXPECT_EQ("\\optio\\n", unescape_glob("\\optio\\n"));
+}
+
 TEST(utils_general, get_long_version) {
   if (*MYSH_BUILD_ID) {
     std::cout << "Verifying values for the build server" << std::endl;

unittest/scripts/auto/py_oci/scripts/util_import_table_oci_norecord.py

@@ -152,6 +152,13 @@
 
 testutil.clear_traps("os_bucket")
 
+#@<> BUG#35895247 - importing a file with escaped wildcard characters should load it in chunks {__os_type != "windows"}
+testutil.anycopy(os.path.join(__import_data_path, SOURCE_FILE), {'osBucketName': OS_BUCKET_NAME, 'osNamespace': OS_NAMESPACE, 'ociConfigFile': OCI_CONFIG_FILE, 'name': "will *this work?"})
+session.run_sql(f"TRUNCATE TABLE {quote_identifier(TARGET_SCHEMA, 'cities')}")
+
+EXPECT_NO_THROWS(lambda: util.import_table("will \\*this work\\?", { "schema": TARGET_SCHEMA, "table": "cities", "showProgress": False, 'osBucketName': OS_BUCKET_NAME, 'osNamespace': OS_NAMESPACE, 'ociConfigFile': OCI_CONFIG_FILE }), "import should not fail")
+EXPECT_STDOUT_CONTAINS(f"Importing from file 'will *this work?' to table ")
+
 #@<> Cleanup
 delete_bucket(OS_BUCKET_NAME)
 

unittest/scripts/auto/py_shell/scripts/util_import_table_norecord.py

@@ -3,6 +3,7 @@
 #@<> This is unit test file for WL12193 Parallel data import
 import os
 import os.path
+import shutil
 
 target_port = __mysql_port
 target_xport = __port
@@ -395,6 +396,16 @@ def TEST_STRING_OPTION(option):
 #@<> s3EndpointOverride is using wrong scheme
 EXPECT_THROWS(lambda: util.import_table(world_x_cities_dump, { "s3BucketName": "bucket", "s3EndpointOverride": "FTp://endpoint", "schema": target_schema, "table": target_table }), "ValueError: Util.import_table: Argument #2: The value of the option 's3EndpointOverride' uses an invalid scheme 'FTp://', expected: http:// or https://.")
 
+#@<> BUG#35895247 - importing a file with escaped wildcard characters should load it in chunks {__os_type != "windows"}
+full_path = os.path.join(__tmp_dir, "will *this work?")
+shutil.copyfile(world_x_cities_dump, full_path)
+truncate_table()
+
+EXPECT_NO_THROWS(lambda: util.import_table(os.path.join(__tmp_dir, "will \\*this work\\?"), { "schema": target_schema, "table": target_table, "showProgress": False }), "import should not fail")
+EXPECT_STDOUT_CONTAINS(f"Importing from file '{filename_for_output(full_path)}' to table ")
+
+os.remove(full_path)
+
 #@<> Teardown
 session.run_sql("DROP SCHEMA IF EXISTS " + target_schema)
 session.close()