Backport #87426 to 25.7: Ignore only not found errors for s3_plain_rewritable (and some other S3 code)

robot-clickhouse · robot-clickhouse · commit 59049a0a03bb · 2025-09-23T07:12:37.000Z
diff --git a/src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp b/src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp
@@ -347,8 +347,7 @@ void S3ObjectStorage::removeObjectsIfExist(const StoredObjects & objects)
 std::optional<ObjectMetadata> S3ObjectStorage::tryGetObjectMetadata(const std::string & path) const
 {
     auto settings_ptr = s3_settings.get();
-    auto object_info = S3::getObjectInfo(
-        *client.get(), uri.bucket, path, {}, /* with_metadata= */ true, /* throw_on_error= */ false);
+    auto object_info = S3::getObjectInfoIfExists(*client.get(), uri.bucket, path, {}, /* with_metadata= */ true);
 
     if (object_info.size == 0 && object_info.last_modification_time == 0 && object_info.metadata.empty())
         return {};
diff --git a/src/IO/S3/getObjectInfo.cpp b/src/IO/S3/getObjectInfo.cpp
@@ -72,42 +72,56 @@ bool isNotFoundError(Aws::S3::S3Errors error)
         || error == Aws::S3::S3Errors::NO_SUCH_BUCKET;
 }
 
+ObjectInfo getObjectInfoIfExists(
+    const S3::Client & client,
+    const String & bucket,
+    const String & key,
+    const String & version_id,
+    bool with_metadata)
+{
+    Expect404ResponseScope scope; // 404 is not an error
+
+    auto [object_info, error] = tryGetObjectInfo(client, bucket, key, version_id, with_metadata);
+    if (object_info)
+        return *object_info;
+
+    if (isNotFoundError(error.GetErrorType()))
+        return {};
+
+    throw S3Exception(
+        error.GetErrorType(),
+        "Failed to get object info: {}. HTTP response code: {}",
+        error.GetMessage(),
+        static_cast<size_t>(error.GetResponseCode()));
+}
+
 ObjectInfo getObjectInfo(
     const S3::Client & client,
     const String & bucket,
     const String & key,
     const String & version_id,
-    bool with_metadata,
-    bool throw_on_error)
+    bool with_metadata)
 {
-    std::optional<Expect404ResponseScope> scope; // 404 is not an error
-    if (!throw_on_error)
-        scope.emplace();
+    Expect404ResponseScope scope; // 404 is not an error
 
     auto [object_info, error] = tryGetObjectInfo(client, bucket, key, version_id, with_metadata);
     if (object_info)
-    {
         return *object_info;
-    }
-    if (throw_on_error)
-    {
-        throw S3Exception(
-            error.GetErrorType(),
-            "Failed to get object info: {}. HTTP response code: {}",
-            error.GetMessage(),
-            static_cast<size_t>(error.GetResponseCode()));
-    }
-    return {};
+
+    throw S3Exception(
+        error.GetErrorType(),
+        "Failed to get object info: {}. HTTP response code: {}",
+        error.GetMessage(),
+        static_cast<size_t>(error.GetResponseCode()));
 }
 
 size_t getObjectSize(
     const S3::Client & client,
     const String & bucket,
     const String & key,
-    const String & version_id,
-    bool throw_on_error)
+    const String & version_id)
 {
-    return getObjectInfo(client, bucket, key, version_id, {}, throw_on_error).size;
+    return getObjectInfo(client, bucket, key, version_id, /*with_metadata=*/ false).size;
 }
 
 bool objectExists(
diff --git a/src/IO/S3/getObjectInfo.h b/src/IO/S3/getObjectInfo.h
@@ -20,20 +20,26 @@ struct ObjectInfo
     std::map<String, String> metadata = {}; /// Set only if getObjectInfo() is called with `with_metadata = true`.
 };
 
+/// Ignore if object does not exist
+ObjectInfo getObjectInfoIfExists(
+    const S3::Client & client,
+    const String & bucket,
+    const String & key,
+    const String & version_id = {},
+    bool with_metadata = false);
+
 ObjectInfo getObjectInfo(
     const S3::Client & client,
     const String & bucket,
     const String & key,
     const String & version_id = {},
-    bool with_metadata = false,
-    bool throw_on_error = true);
+    bool with_metadata = false);
 
 size_t getObjectSize(
     const S3::Client & client,
     const String & bucket,
     const String & key,
-    const String & version_id = {},
-    bool throw_on_error = true);
+    const String & version_id = {});
 
 bool objectExists(
     const S3::Client & client,

Original file line number	Diff line number	Diff line change
`@@ -347,8 +347,7 @@ void S3ObjectStorage::removeObjectsIfExist(const StoredObjects & objects)`
`347`	`347`	`std::optional<ObjectMetadata> S3ObjectStorage::tryGetObjectMetadata(const std::string & path) const`
`348`	`348`	`{`
`349`	`349`	`auto settings_ptr = s3_settings.get();`
`350`		`- auto object_info = S3::getObjectInfo(`
`351`		`- client.get(), uri.bucket, path, {}, / with_metadata= / true, / throw_on_error= */ false);`
	`350`	`+ auto object_info = S3::getObjectInfoIfExists(client.get(), uri.bucket, path, {}, / with_metadata= */ true);`
`352`	`351`
`353`	`352`	`if (object_info.size == 0 && object_info.last_modification_time == 0 && object_info.metadata.empty())`
`354`	`353`	`return {};`