Merge pull request #51345 from cbodley/wip-59639

rgw/dbstore: allow NULL RealmIDs in sqlite schema Reviewed-by: Soumya Koduri <skoduri@redhat.com> Reviewed-by: Daniel Gryniewicz <dang@redhat.com>
ceph · May 10, 2023 · 6c07ed5 · 6c07ed5
2 parents 9ac1007 + 23968f9
commit 6c07ed5
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 12 deletions.
diff --git a/qa/suites/rgw/dbstore/overrides.yaml b/qa/suites/rgw/dbstore/overrides.yaml
@@ -6,6 +6,7 @@ overrides:
         setgroup: ceph
         debug rgw: 20
         rgw crypt require ssl: false
+        rgw config store: dbstore
         rgw backend store: dbstore
   rgw:
     frontend: beast
diff --git a/src/rgw/driver/dbstore/config/sqlite.cc b/src/rgw/driver/dbstore/config/sqlite.cc
@@ -58,6 +58,17 @@ static constexpr const char* P4 = ":4";
 static constexpr const char* P5 = ":5";
 static constexpr const char* P6 = ":6";
 
+// bind as text unless value is empty
+void bind_text_or_null(const DoutPrefixProvider* dpp,
+                       const sqlite::stmt_binding& stmt,
+                       const char* name, std::string_view value)
+{
+  if (value.empty()) {
+    sqlite::bind_null(dpp, stmt, name);
+  } else {
+    sqlite::bind_text(dpp, stmt, name, value);
+  }
+}
 
 void read_text_rows(const DoutPrefixProvider* dpp,
                     const sqlite::stmt_execution& stmt,
@@ -934,7 +945,7 @@ class SQLiteZoneGroupWriter : public sal::ZoneGroupWriter {
       }
       auto binding = sqlite::stmt_binding{stmt.get()};
       sqlite::bind_text(dpp, binding, P1, info.id);
-      sqlite::bind_text(dpp, binding, P2, info.realm_id);
+      bind_text_or_null(dpp, binding, P2, info.realm_id);
       sqlite::bind_text(dpp, binding, P3, data);
       sqlite::bind_int(dpp, binding, P4, ver);
       sqlite::bind_text(dpp, binding, P5, tag);
@@ -1073,7 +1084,7 @@ int SQLiteConfigStore::write_default_zonegroup_id(const DoutPrefixProvider* dpp,
       }
     }
     auto binding = sqlite::stmt_binding{stmt->get()};
-    sqlite::bind_text(dpp, binding, P1, realm_id);
+    bind_text_or_null(dpp, binding, P1, realm_id);
     sqlite::bind_text(dpp, binding, P2, zonegroup_id);
 
     auto reset = sqlite::stmt_execution{stmt->get()};
@@ -1103,7 +1114,7 @@ int SQLiteConfigStore::read_default_zonegroup_id(const DoutPrefixProvider* dpp,
       stmt = sqlite::prepare_statement(dpp, conn->db.get(), sql);
     }
     auto binding = sqlite::stmt_binding{stmt.get()};
-    sqlite::bind_text(dpp, binding, P1, realm_id);
+    bind_text_or_null(dpp, binding, P1, realm_id);
 
     auto reset = sqlite::stmt_execution{stmt.get()};
     sqlite::eval1(dpp, reset);
@@ -1135,7 +1146,7 @@ int SQLiteConfigStore::delete_default_zonegroup_id(const DoutPrefixProvider* dpp
       stmt = sqlite::prepare_statement(dpp, conn->db.get(), sql);
     }
     auto binding = sqlite::stmt_binding{stmt.get()};
-    sqlite::bind_text(dpp, binding, P1, realm_id);
+    bind_text_or_null(dpp, binding, P1, realm_id);
 
     auto reset = sqlite::stmt_execution{stmt.get()};
     sqlite::eval0(dpp, reset);
@@ -1198,7 +1209,7 @@ int SQLiteConfigStore::create_zonegroup(const DoutPrefixProvider* dpp,
     auto binding = sqlite::stmt_binding{stmt->get()};
     sqlite::bind_text(dpp, binding, P1, info.id);
     sqlite::bind_text(dpp, binding, P2, info.name);
-    sqlite::bind_text(dpp, binding, P3, info.realm_id);
+    bind_text_or_null(dpp, binding, P3, info.realm_id);
     sqlite::bind_text(dpp, binding, P4, data);
     sqlite::bind_int(dpp, binding, P5, ver);
     sqlite::bind_text(dpp, binding, P6, tag);
@@ -1439,7 +1450,7 @@ class SQLiteZoneWriter : public sal::ZoneWriter {
       }
       auto binding = sqlite::stmt_binding{stmt.get()};
       sqlite::bind_text(dpp, binding, P1, info.id);
-      sqlite::bind_text(dpp, binding, P2, info.realm_id);
+      bind_text_or_null(dpp, binding, P2, info.realm_id);
       sqlite::bind_text(dpp, binding, P3, data);
       sqlite::bind_int(dpp, binding, P4, ver);
       sqlite::bind_text(dpp, binding, P5, tag);
@@ -1581,7 +1592,7 @@ int SQLiteConfigStore::write_default_zone_id(const DoutPrefixProvider* dpp,
       }
     }
     auto binding = sqlite::stmt_binding{stmt->get()};
-    sqlite::bind_text(dpp, binding, P1, realm_id);
+    bind_text_or_null(dpp, binding, P1, realm_id);
     sqlite::bind_text(dpp, binding, P2, zone_id);
 
     auto reset = sqlite::stmt_execution{stmt->get()};
@@ -1611,7 +1622,7 @@ int SQLiteConfigStore::read_default_zone_id(const DoutPrefixProvider* dpp,
       stmt = sqlite::prepare_statement(dpp, conn->db.get(), sql);
     }
     auto binding = sqlite::stmt_binding{stmt.get()};
-    sqlite::bind_text(dpp, binding, P1, realm_id);
+    bind_text_or_null(dpp, binding, P1, realm_id);
 
     auto reset = sqlite::stmt_execution{stmt.get()};
     sqlite::eval1(dpp, reset);
@@ -1643,7 +1654,7 @@ int SQLiteConfigStore::delete_default_zone_id(const DoutPrefixProvider* dpp,
       stmt = sqlite::prepare_statement(dpp, conn->db.get(), sql);
     }
     auto binding = sqlite::stmt_binding{stmt.get()};
-    sqlite::bind_text(dpp, binding, P1, realm_id);
+    bind_text_or_null(dpp, binding, P1, realm_id);
 
     auto reset = sqlite::stmt_execution{stmt.get()};
     sqlite::eval0(dpp, reset);
@@ -1706,7 +1717,7 @@ int SQLiteConfigStore::create_zone(const DoutPrefixProvider* dpp,
     auto binding = sqlite::stmt_binding{stmt->get()};
     sqlite::bind_text(dpp, binding, P1, info.id);
     sqlite::bind_text(dpp, binding, P2, info.name);
-    sqlite::bind_text(dpp, binding, P3, info.realm_id);
+    bind_text_or_null(dpp, binding, P3, info.realm_id);
     sqlite::bind_text(dpp, binding, P4, data);
     sqlite::bind_int(dpp, binding, P5, ver);
     sqlite::bind_text(dpp, binding, P6, tag);

diff --git a/src/rgw/driver/dbstore/config/sqlite_schema.h b/src/rgw/driver/dbstore/config/sqlite_schema.h
@@ -53,15 +53,15 @@ CREATE TABLE IF NOT EXISTS PeriodConfigs (
 CREATE TABLE IF NOT EXISTS ZoneGroups (
   ID TEXT PRIMARY KEY NOT NULL,
   Name TEXT UNIQUE NOT NULL,
-  RealmID TEXT NOT NULL REFERENCES Realms (ID),
+  RealmID TEXT REFERENCES Realms (ID),
   Data TEXT NOT NULL,
   VersionNumber INTEGER,
   VersionTag TEXT
 );
 CREATE TABLE IF NOT EXISTS Zones (
   ID TEXT PRIMARY KEY NOT NULL,
   Name TEXT UNIQUE NOT NULL,
-  RealmID TEXT NOT NULL REFERENCES Realms (ID),
+  RealmID TEXT REFERENCES Realms (ID),
   Data TEXT NOT NULL,
   VersionNumber INTEGER,
   VersionTag TEXT

diff --git a/src/rgw/driver/dbstore/sqlite/statement.cc b/src/rgw/driver/dbstore/sqlite/statement.cc
@@ -58,6 +58,21 @@ static int bind_index(const DoutPrefixProvider* dpp,
   return index;
 }
 
+void bind_null(const DoutPrefixProvider* dpp, const stmt_binding& stmt,
+               const char* name)
+{
+  const int index = bind_index(dpp, stmt, name);
+
+  int result = ::sqlite3_bind_null(stmt.get(), index);
+  auto ec = std::error_code{result, sqlite::error_category()};
+  if (ec != sqlite::errc::ok) {
+    ldpp_dout(dpp, 1) << "binding failed on parameter name="
+        << name << dendl;
+    sqlite3* db = ::sqlite3_db_handle(stmt.get());
+    throw sqlite::error(db, ec);
+  }
+}
+
 void bind_text(const DoutPrefixProvider* dpp, const stmt_binding& stmt,
                const char* name, std::string_view value)
 {

diff --git a/src/rgw/driver/dbstore/sqlite/statement.h b/src/rgw/driver/dbstore/sqlite/statement.h
@@ -48,6 +48,10 @@ using stmt_execution = std::unique_ptr<sqlite3_stmt, stmt_execution_deleter>;
 stmt_ptr prepare_statement(const DoutPrefixProvider* dpp,
                            sqlite3* db, std::string_view sql);
 
+// bind a NULL input for the given parameter name
+void bind_null(const DoutPrefixProvider* dpp, const stmt_binding& stmt,
+               const char* name);
+
 // bind an input string for the given parameter name
 void bind_text(const DoutPrefixProvider* dpp, const stmt_binding& stmt,
                const char* name, std::string_view value);