SteNicholas commented on code in PR #330:
URL: https://github.com/apache/paimon-cpp/pull/330#discussion_r4003898371
##########
src/paimon/core/operation/file_store_commit.cpp:
##########
@@ -142,9 +185,38 @@ Result<std::unique_ptr<FileStoreCommit>>
FileStoreCommit::Create(
return NewFormatTableCommit(given_table, *ctx);
}
+ PAIMON_RETURN_NOT_OK(CheckVersionManagementImplemented(ctx->GetCatalog()));
+ // Use catalog credentials unless the caller supplied a file system.
+ std::shared_ptr<FileSystem> specific_fs = ctx->GetSpecificFileSystem();
+ if (specific_fs == nullptr && ctx->GetCatalog() != nullptr) {
+ specific_fs = ctx->GetCatalog()->GetFileSystem();
+ }
PAIMON_ASSIGN_OR_RAISE(CoreOptions tmp_options,
- CoreOptions::FromMap(ctx->GetOptions(),
ctx->GetSpecificFileSystem()));
+ CoreOptions::FromMap(ctx->GetOptions(),
specific_fs));
const std::string& root_path = ctx->GetRootPath();
+ // Catalog-managed schemas may be absent from the table directory.
+ std::optional<std::string> catalog_table_schema;
+ FileStoreCommitImpl::SchemaIdLoader schema_id_loader;
+ if (ctx->GetCatalog() != nullptr) {
+ assert(ctx->GetIdentifier());
Review Comment:
Fixed in 1c8bb4e1 with the first suggestion: the assert is replaced by an
explicit check that returns `Status::Invalid("a catalog commit requires a table
identifier")`, mirroring the handling in `FileStoreWrite::Create`, so a
`CommitContext` constructed directly with a catalog but no identifier now fails
through the `Result`/`Status` error model in both Debug and Release builds.
Also added a regression test
`FileStoreCommitTest.TestCreateWithCatalogRequiresIdentifier` covering this
path. The constructor is kept public for symmetry with `WriteContext`;
validating at the `Create()` boundary covers every path that can reach the
`.value()` calls.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
