From c28e084282f48ea127f0c0d5c3f19168c08c9204 Mon Sep 17 00:00:00 2001
From: Dimitris Staratzis <33267511+DimitrisStaratzis@users.noreply.github.com>
Date: Thu, 16 May 2024 08:59:02 +0300
Subject: [PATCH] throw error on dimension drop in schema evolution. (#4958)

Adding an error message when trying to drop a non existent attribute or
a dimension with the Schema Evolution API. + Test

[sc-30774]

---
TYPE: BUG
DESC: Throw error on dimension drop in schema evolution.
---
 test/src/unit-cppapi-schema-evolution.cc      | 44 +++++++++++++++++++
 .../sm/array_schema/array_schema_evolution.cc |  7 ++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/test/src/unit-cppapi-schema-evolution.cc b/test/src/unit-cppapi-schema-evolution.cc
index a6e2d237018..15bf64b0775 100644
--- a/test/src/unit-cppapi-schema-evolution.cc
+++ b/test/src/unit-cppapi-schema-evolution.cc
@@ -107,6 +107,50 @@ TEST_CASE(
   }
 }
 
+TEST_CASE(
+    "C++ API: SchemaEvolution, check error when dropping dimension",
+    "[cppapi][schema][evolution][drop]") {
+  using namespace tiledb;
+  Context ctx;
+  VFS vfs(ctx);
+
+  std::string array_uri = "test_schema_evolution_array";
+
+  Domain domain(ctx);
+  auto id1 = Dimension::create<int>(ctx, "d1", {{-100, 100}}, 10);
+  auto id2 = Dimension::create<int>(ctx, "d2", {{0, 100}}, 5);
+  domain.add_dimension(id1).add_dimension(id2);
+
+  auto a1 = Attribute::create<int>(ctx, "a1");
+  auto a2 = Attribute::create<int>(ctx, "a2");
+
+  ArraySchema schema(ctx, TILEDB_DENSE);
+  schema.set_domain(domain);
+  schema.add_attribute(a1);
+  schema.add_attribute(a2);
+  schema.set_cell_order(TILEDB_ROW_MAJOR);
+  schema.set_tile_order(TILEDB_COL_MAJOR);
+
+  if (vfs.is_dir(array_uri)) {
+    vfs.remove_dir(array_uri);
+  }
+
+  Array::create(array_uri, schema);
+
+  auto evolution = ArraySchemaEvolution(ctx);
+
+  // try to drop dimension d1
+  evolution.drop_attribute("d1");
+
+  // check that an exception is thrown
+  CHECK_THROWS(evolution.array_evolve(array_uri));
+
+  // Clean up
+  if (vfs.is_dir(array_uri)) {
+    vfs.remove_dir(array_uri);
+  }
+}
+
 TEST_CASE(
     "C++ API: SchemaEvolution, add attributes and read",
     "[cppapi][schema][evolution][add]") {
diff --git a/tiledb/sm/array_schema/array_schema_evolution.cc b/tiledb/sm/array_schema/array_schema_evolution.cc
index 117dee9607b..560db92f149 100644
--- a/tiledb/sm/array_schema/array_schema_evolution.cc
+++ b/tiledb/sm/array_schema/array_schema_evolution.cc
@@ -152,9 +152,12 @@ shared_ptr<ArraySchema> ArraySchemaEvolution::evolve_schema(
   for (auto& attr_name : attributes_to_drop_) {
     bool has_attr = false;
     throw_if_not_ok(schema->has_attribute(attr_name, &has_attr));
-    if (has_attr) {
-      throw_if_not_ok(schema->drop_attribute(attr_name));
+    if (!has_attr) {
+      throw ArraySchemaEvolutionException(
+          "Cannot drop attribute; Input attribute name refers to a dimension "
+          "or does not exist");
     }
+    throw_if_not_ok(schema->drop_attribute(attr_name));
   }
 
   // Drop enumerations