Error out no-op set_config calls: FragmentInfo (#3885)

Setting a config on a `FragmentInfo` object is a no-op after `FragmentInfo::load()` has been called; throw accordingly. --- TYPE: IMPROVEMENT DESC: Error out no-op set_config calls, FragmentInfo
TileDB-Inc · Mar 2, 2023 · fcceef3 · fcceef3
1 parent 191589a
commit fcceef3
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 11 deletions.
diff --git a/test/src/unit-capi-fragment_info.cc b/test/src/unit-capi-fragment_info.cc
@@ -272,6 +272,10 @@ TEST_CASE(
     CHECK(rc == TILEDB_OK);
     rc = tiledb_fragment_info_load(ctx, fragment_info);
     CHECK(rc == TILEDB_OK);
+
+    // Try setting the config after load
+    rc = tiledb_fragment_info_set_config(ctx, fragment_info, cfg);
+    CHECK(rc == TILEDB_ERR);
   } else {
     // Load fragment info
     rc = tiledb_fragment_info_load(ctx, fragment_info);

diff --git a/tiledb/sm/c_api/tiledb.cc b/tiledb/sm/c_api/tiledb.cc
@@ -4842,7 +4842,7 @@ int32_t tiledb_fragment_info_set_config(
     return TILEDB_ERR;
   api::ensure_config_is_valid(config);
 
-  throw_if_not_ok(fragment_info->fragment_info_->set_config(config->config()));
+  fragment_info->fragment_info_->set_config(config->config());
 
   return TILEDB_OK;
 }

diff --git a/tiledb/sm/fragment/fragment_info.cc b/tiledb/sm/fragment/fragment_info.cc
@@ -5,7 +5,7 @@
  *
  * The MIT License
  *
- * @copyright Copyright (c) 2020-2022 TileDB, Inc.
+ * @copyright Copyright (c) 2020-2023 TileDB, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -92,9 +92,12 @@ FragmentInfo& FragmentInfo::operator=(FragmentInfo&& fragment_info) {
 /*                API                */
 /* ********************************* */
 
-Status FragmentInfo::set_config(const Config& config) {
-  config_ = config;
-  return Status::Ok();
+void FragmentInfo::set_config(const Config& config) {
+  if (loaded_) {
+    throw StatusException(
+        Status_FragmentInfoError("[set_config] Cannot set config after load"));
+  }
+  config_.inherit(config);
 }
 
 void FragmentInfo::expand_anterior_ndrange(

diff --git a/tiledb/sm/fragment/fragment_info.h b/tiledb/sm/fragment/fragment_info.h
@@ -5,7 +5,7 @@
  *
  * The MIT License
  *
- * @copyright Copyright (c) 2020-2022 TileDB, Inc.
+ * @copyright Copyright (c) 2020-2023 TileDB, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -80,10 +80,11 @@ class FragmentInfo {
   /* ********************************* */
 
   /**
-   * Sets a config to the fragment info. Useful for retrieving timestamps
-   * and encryption key.
+   * Sets a config to the fragment info. Useful for encryption information.
+   *
+   * @pre The FragmentInfo object must not yet be loaded.
    */
-  Status set_config(const Config& config);
+  void set_config(const Config& config);
 
   /** Expand the non empty domain before start with a new range */
   void expand_anterior_ndrange(const Domain& domain, const NDRange& range);

diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc
@@ -5,7 +5,7 @@
  *
  * The MIT License
  *
- * @copyright Copyright (c) 2022 TileDB, Inc.
+ * @copyright Copyright (c) 2023 TileDB, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -80,7 +80,7 @@ Status fragment_info_request_from_capnp(
     tdb_unique_ptr<Config> decoded_config = nullptr;
     RETURN_NOT_OK(config_from_capnp(
         fragment_info_req_reader.getConfig(), &decoded_config));
-    RETURN_NOT_OK(fragment_info->set_config(*decoded_config));
+    fragment_info->set_config(*decoded_config);
   }
 
   return Status::Ok();