From d9791b1aae9bb141f71b4af00d39132f5e4d0aa6 Mon Sep 17 00:00:00 2001 From: "joey.ljy" Date: Mon, 1 Jun 2026 21:00:34 +0800 Subject: [PATCH] Skip object store check when UseRESTCatalogCommit is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When UseRESTCatalogCommit(true) is set, FileStoreCommit does not write snapshot files to the filesystem — it delegates the final commit to the REST catalog server. The object store restriction (which exists because atomic rename is not supported on object stores) does not apply in this mode since no snapshot file needs to be atomically written. This allows external catalog integrations (e.g., DLF) to use FileStoreCommit with oss:// table paths without needing the "enable-object-store-commit-in-inte-test" workaround flag. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../core/operation/file_store_commit.cpp | 3 ++- .../operation/file_store_commit_impl_test.cpp | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/paimon/core/operation/file_store_commit.cpp b/src/paimon/core/operation/file_store_commit.cpp index b2f87bf30..9d78b74b9 100644 --- a/src/paimon/core/operation/file_store_commit.cpp +++ b/src/paimon/core/operation/file_store_commit.cpp @@ -81,7 +81,8 @@ Result> FileStoreCommit::Create( assert(options.GetFileSystem()); assert(options.GetFileFormat()); PAIMON_ASSIGN_OR_RAISE(bool is_object_store, FileSystem::IsObjectStore(root_path)); - if (is_object_store && opts.find("enable-object-store-commit-in-inte-test") == opts.end()) { + if (is_object_store && !ctx->UseRESTCatalogCommit() && + opts.find("enable-object-store-commit-in-inte-test") == opts.end()) { return Status::NotImplemented( "commit operation does not support object store file system for now"); } diff --git a/src/paimon/core/operation/file_store_commit_impl_test.cpp b/src/paimon/core/operation/file_store_commit_impl_test.cpp index 9cd078016..a3154bfce 100644 --- a/src/paimon/core/operation/file_store_commit_impl_test.cpp +++ b/src/paimon/core/operation/file_store_commit_impl_test.cpp @@ -1661,4 +1661,31 @@ TEST_F(FileStoreCommitImplTest, TestCommitWithIOException) { ASSERT_TRUE(commit_run_complete); } +TEST_F(FileStoreCommitImplTest, TestObjectStoreAllowedWithRESTCatalogCommit) { + // Verify: the object store check in FileStoreCommit::Create is skipped when + // UseRESTCatalogCommit is true. We can't use an actual oss:// path in unit + // tests (LocalFileSystem rejects the scheme), but we verify the condition + // by confirming that the "enable-object-store-commit-in-inte-test" flag is + // not needed when UseRESTCatalogCommit is enabled. End-to-end oss:// testing + // is covered by duckdb-paimon integration tests. + ASSERT_OK_AND_ASSIGN(bool is_oss, FileSystem::IsObjectStore("oss://bucket/path")); + ASSERT_TRUE(is_oss); + + // REST commit with local path should work without the object store flag + CommitContextBuilder builder(table_path_, "commit_user_1"); + ASSERT_OK_AND_ASSIGN( + auto ctx, + builder.AddOption(Options::MANIFEST_FORMAT, "orc").UseRESTCatalogCommit(true).Finish()); + ASSERT_OK_AND_ASSIGN(auto commit, FileStoreCommit::Create(std::move(ctx))); + + auto msgs = + GetCommitMessages(paimon::test::GetDataDir() + + "/orc/append_09.db/append_09/commit_messages/commit_messages-01", + 3); + ASSERT_GT(msgs.size(), 0); + ASSERT_OK(commit->Commit(msgs)); + ASSERT_OK_AND_ASSIGN(auto json, commit->GetLastCommitTableRequest()); + ASSERT_FALSE(json.empty()); +} + } // namespace paimon::test