Backport #87392 to 25.9: Fix EmbeddedRocksDB upgrade

robot-clickhouse · robot-clickhouse · commit 6c69d5e16970 · 2025-09-22T15:12:54.000Z
diff --git a/src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp b/src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp
@@ -213,17 +213,25 @@ StorageEmbeddedRocksDB::StorageEmbeddedRocksDB(const StorageID & table_id_,
     setSettings(std::move(settings_));
 
     if (rocksdb_dir.empty())
-        rocksdb_dir = fs::path{getContext()->getUserFilesPath()} / relative_data_path_;
-
-    bool is_local = context_->getApplicationType() == Context::ApplicationType::LOCAL;
-    fs::path user_files_path = is_local ? "" : fs::canonical(getContext()->getUserFilesPath());
-    if (fs::path(rocksdb_dir).is_relative())
-        rocksdb_dir = user_files_path / rocksdb_dir;
-    rocksdb_dir = fs::absolute(rocksdb_dir).lexically_normal();
-
-    if (!is_local && !pathStartsWith(fs::path(rocksdb_dir), user_files_path))
-        throw Exception(ErrorCodes::BAD_ARGUMENTS,
-                        "Path must be inside user-files path: {}", user_files_path.string());
+    {
+        /// We used to create databases under the database directory by default instead of user files. Check first if it exists there
+        auto old_path = context_->getPath() + relative_data_path_;
+        if (mode >= LoadingStrictnessLevel::ATTACH && fs::exists(old_path))
+            rocksdb_dir = old_path;
+        else
+            rocksdb_dir = fs::path{getContext()->getUserFilesPath()} / relative_data_path_;
+    }
+    else
+    {
+        bool is_local = context_->getApplicationType() == Context::ApplicationType::LOCAL;
+        fs::path user_files_path = is_local ? "" : fs::canonical(getContext()->getUserFilesPath());
+        if (fs::path(rocksdb_dir).is_relative())
+            rocksdb_dir = user_files_path / rocksdb_dir;
+        rocksdb_dir = fs::absolute(rocksdb_dir).lexically_normal();
+
+        if (!is_local && !pathStartsWith(fs::path(rocksdb_dir), user_files_path))
+            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path must be inside user-files path: {}", user_files_path.string());
+    }
 
     if (mode < LoadingStrictnessLevel::ATTACH)
     {
diff --git a/tests/integration/test_backward_compatibility/test_rocksdb_upgrade.py b/tests/integration/test_backward_compatibility/test_rocksdb_upgrade.py
@@ -0,0 +1,51 @@
+import pytest
+
+from helpers.cluster import CLICKHOUSE_CI_MIN_TESTED_VERSION, ClickHouseCluster
+
+cluster = ClickHouseCluster(__file__)
+node = cluster.add_instance(
+    "node",
+    with_zookeeper=False,
+    image="clickhouse/clickhouse-server",
+    tag=CLICKHOUSE_CI_MIN_TESTED_VERSION,
+    stay_alive=True,
+    with_installed_binary=True,
+)
+
+
+@pytest.fixture(scope="module")
+def start_cluster():
+    try:
+        cluster.start()
+        yield cluster
+
+    finally:
+        cluster.shutdown()
+
+
+@pytest.fixture(autouse=True)
+def cleanup():
+    yield
+    node.restart_with_original_version(clear_data_dir=True)
+
+
+def test_rocksdb_upgrade(start_cluster):
+    node.query(
+        """
+               CREATE TABLE vt
+               (
+                   `sha256` FixedString(64),
+                   `amount` Nullable(Int8)
+               )
+               ENGINE = EmbeddedRocksDB
+               PRIMARY KEY sha256
+               """
+    )
+
+    assert node.query("SELECT count() FROM vt") == "0\n"
+
+    node.restart_with_latest_version()
+
+    assert node.query("SELECT count() FROM vt") == "0\n"
+
+    node.query("""DROP TABLE vt""")
