lucasfang commented on code in PR #330:
URL: https://github.com/apache/paimon-cpp/pull/330#discussion_r4002884144
##########
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:
the identifier is only guarded by an assert before .value() is called. The
newly added public CommitContext constructor allows passing a non-null catalog
with a null identifier. In Debug builds, this terminates the process; in
Release builds, it throws std::bad_optional_access, bypassing the Result/Status
error model.
Suggestion: Either return Status::Invalid like FileStoreWrite::Create does,
or restrict the constructor so it can only be used by the builder.
--
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]
