Copilot commented on code in PR #286:
URL: https://github.com/apache/fluss-rust/pull/286#discussion_r2778922659
##########
bindings/cpp/src/admin.cpp:
##########
@@ -204,4 +204,124 @@ Result Admin::CreatePartition(const TablePath& table_path,
return utils::from_ffi_result(ffi_result);
}
+Result Admin::DropPartition(const TablePath& table_path,
+ const std::unordered_map<std::string,
std::string>& partition_spec,
+ bool ignore_if_not_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_path = utils::to_ffi_table_path(table_path);
+
+ rust::Vec<ffi::FfiPartitionKeyValue> rust_spec;
+ for (const auto& [key, value] : partition_spec) {
+ ffi::FfiPartitionKeyValue kv;
+ kv.key = rust::String(key);
+ kv.value = rust::String(value);
+ rust_spec.push_back(std::move(kv));
+ }
+
+ auto ffi_result =
+ admin_->drop_partition(ffi_path, std::move(rust_spec),
ignore_if_not_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::CreateDatabase(const std::string& database_name,
+ const DatabaseDescriptor& descriptor,
+ bool ignore_if_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_desc = utils::to_ffi_database_descriptor(descriptor);
+ auto ffi_result =
+ admin_->create_database(rust::String(database_name), ffi_desc,
ignore_if_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::DropDatabase(const std::string& database_name, bool
ignore_if_not_exists,
+ bool cascade) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result =
+ admin_->drop_database(rust::String(database_name),
ignore_if_not_exists, cascade);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::ListDatabases(std::vector<std::string>& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->list_databases();
+ auto result = utils::from_ffi_result(ffi_result.result);
+ if (result.Ok()) {
+ out.clear();
+ out.reserve(ffi_result.database_names.size());
+ for (const auto& name : ffi_result.database_names) {
+ out.push_back(std::string(name));
+ }
+ }
+ return result;
+}
+
+Result Admin::DatabaseExists(const std::string& database_name, bool& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->database_exists(rust::String(database_name));
+ auto result = utils::from_ffi_result(ffi_result.result);
+ if (result.Ok()) {
+ out = ffi_result.value;
+ }
+ return result;
+}
+
+Result Admin::GetDatabaseInfo(const std::string& database_name, DatabaseInfo&
out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->get_database_info(rust::String(database_name));
+ auto result = utils::from_ffi_result(ffi_result.result);
Review Comment:
This call passes a temporary `rust::String` where the Rust FFI expects
`&str`, causing an unnecessary allocation/copy. Prefer passing `database_name`
directly (or as `rust::Str`) to match the FFI signature and avoid extra work.
##########
bindings/cpp/src/admin.cpp:
##########
@@ -204,4 +204,124 @@ Result Admin::CreatePartition(const TablePath& table_path,
return utils::from_ffi_result(ffi_result);
}
+Result Admin::DropPartition(const TablePath& table_path,
+ const std::unordered_map<std::string,
std::string>& partition_spec,
+ bool ignore_if_not_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_path = utils::to_ffi_table_path(table_path);
+
+ rust::Vec<ffi::FfiPartitionKeyValue> rust_spec;
+ for (const auto& [key, value] : partition_spec) {
+ ffi::FfiPartitionKeyValue kv;
+ kv.key = rust::String(key);
+ kv.value = rust::String(value);
+ rust_spec.push_back(std::move(kv));
+ }
+
+ auto ffi_result =
+ admin_->drop_partition(ffi_path, std::move(rust_spec),
ignore_if_not_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::CreateDatabase(const std::string& database_name,
+ const DatabaseDescriptor& descriptor,
+ bool ignore_if_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_desc = utils::to_ffi_database_descriptor(descriptor);
+ auto ffi_result =
+ admin_->create_database(rust::String(database_name), ffi_desc,
ignore_if_exists);
+ return utils::from_ffi_result(ffi_result);
Review Comment:
The Rust FFI for create_database takes `&str`, so passing a temporary
`rust::String` here causes an unnecessary allocation/copy (and may not match
the generated CXX signature on some setups). Prefer passing `database_name`
directly (or `rust::Str`/`std::string_view`) to match how other `&str` FFI
calls (e.g., `new_connection`) are invoked.
##########
bindings/cpp/src/admin.cpp:
##########
@@ -204,4 +204,124 @@ Result Admin::CreatePartition(const TablePath& table_path,
return utils::from_ffi_result(ffi_result);
}
+Result Admin::DropPartition(const TablePath& table_path,
+ const std::unordered_map<std::string,
std::string>& partition_spec,
+ bool ignore_if_not_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_path = utils::to_ffi_table_path(table_path);
+
+ rust::Vec<ffi::FfiPartitionKeyValue> rust_spec;
+ for (const auto& [key, value] : partition_spec) {
+ ffi::FfiPartitionKeyValue kv;
+ kv.key = rust::String(key);
+ kv.value = rust::String(value);
+ rust_spec.push_back(std::move(kv));
+ }
+
+ auto ffi_result =
+ admin_->drop_partition(ffi_path, std::move(rust_spec),
ignore_if_not_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::CreateDatabase(const std::string& database_name,
+ const DatabaseDescriptor& descriptor,
+ bool ignore_if_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_desc = utils::to_ffi_database_descriptor(descriptor);
+ auto ffi_result =
+ admin_->create_database(rust::String(database_name), ffi_desc,
ignore_if_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::DropDatabase(const std::string& database_name, bool
ignore_if_not_exists,
+ bool cascade) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result =
+ admin_->drop_database(rust::String(database_name),
ignore_if_not_exists, cascade);
+ return utils::from_ffi_result(ffi_result);
Review Comment:
Same as above: this Rust FFI method takes `&str`, so constructing a
`rust::String` just to pass it across the boundary adds avoidable
allocation/copy. Pass the `std::string` directly (or use `rust::Str`) to avoid
the extra work and align with existing patterns in the bindings.
##########
bindings/cpp/src/admin.cpp:
##########
@@ -204,4 +204,124 @@ Result Admin::CreatePartition(const TablePath& table_path,
return utils::from_ffi_result(ffi_result);
}
+Result Admin::DropPartition(const TablePath& table_path,
+ const std::unordered_map<std::string,
std::string>& partition_spec,
+ bool ignore_if_not_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_path = utils::to_ffi_table_path(table_path);
+
+ rust::Vec<ffi::FfiPartitionKeyValue> rust_spec;
+ for (const auto& [key, value] : partition_spec) {
+ ffi::FfiPartitionKeyValue kv;
+ kv.key = rust::String(key);
+ kv.value = rust::String(value);
+ rust_spec.push_back(std::move(kv));
+ }
+
+ auto ffi_result =
+ admin_->drop_partition(ffi_path, std::move(rust_spec),
ignore_if_not_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::CreateDatabase(const std::string& database_name,
+ const DatabaseDescriptor& descriptor,
+ bool ignore_if_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_desc = utils::to_ffi_database_descriptor(descriptor);
+ auto ffi_result =
+ admin_->create_database(rust::String(database_name), ffi_desc,
ignore_if_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::DropDatabase(const std::string& database_name, bool
ignore_if_not_exists,
+ bool cascade) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result =
+ admin_->drop_database(rust::String(database_name),
ignore_if_not_exists, cascade);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::ListDatabases(std::vector<std::string>& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->list_databases();
+ auto result = utils::from_ffi_result(ffi_result.result);
+ if (result.Ok()) {
+ out.clear();
+ out.reserve(ffi_result.database_names.size());
+ for (const auto& name : ffi_result.database_names) {
+ out.push_back(std::string(name));
+ }
+ }
+ return result;
+}
+
+Result Admin::DatabaseExists(const std::string& database_name, bool& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->database_exists(rust::String(database_name));
+ auto result = utils::from_ffi_result(ffi_result.result);
Review Comment:
These FFI methods use `&str` parameters on the Rust side. Creating temporary
`rust::String` values here adds avoidable allocation/copy; pass the
`std::string` directly (or use `rust::Str`) like other `&str` calls in the
codebase.
##########
bindings/cpp/src/admin.cpp:
##########
@@ -204,4 +204,124 @@ Result Admin::CreatePartition(const TablePath& table_path,
return utils::from_ffi_result(ffi_result);
}
+Result Admin::DropPartition(const TablePath& table_path,
+ const std::unordered_map<std::string,
std::string>& partition_spec,
+ bool ignore_if_not_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_path = utils::to_ffi_table_path(table_path);
+
+ rust::Vec<ffi::FfiPartitionKeyValue> rust_spec;
+ for (const auto& [key, value] : partition_spec) {
+ ffi::FfiPartitionKeyValue kv;
+ kv.key = rust::String(key);
+ kv.value = rust::String(value);
+ rust_spec.push_back(std::move(kv));
+ }
+
+ auto ffi_result =
+ admin_->drop_partition(ffi_path, std::move(rust_spec),
ignore_if_not_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::CreateDatabase(const std::string& database_name,
+ const DatabaseDescriptor& descriptor,
+ bool ignore_if_exists) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_desc = utils::to_ffi_database_descriptor(descriptor);
+ auto ffi_result =
+ admin_->create_database(rust::String(database_name), ffi_desc,
ignore_if_exists);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::DropDatabase(const std::string& database_name, bool
ignore_if_not_exists,
+ bool cascade) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result =
+ admin_->drop_database(rust::String(database_name),
ignore_if_not_exists, cascade);
+ return utils::from_ffi_result(ffi_result);
+}
+
+Result Admin::ListDatabases(std::vector<std::string>& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->list_databases();
+ auto result = utils::from_ffi_result(ffi_result.result);
+ if (result.Ok()) {
+ out.clear();
+ out.reserve(ffi_result.database_names.size());
+ for (const auto& name : ffi_result.database_names) {
+ out.push_back(std::string(name));
+ }
+ }
+ return result;
+}
+
+Result Admin::DatabaseExists(const std::string& database_name, bool& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->database_exists(rust::String(database_name));
+ auto result = utils::from_ffi_result(ffi_result.result);
+ if (result.Ok()) {
+ out = ffi_result.value;
+ }
+ return result;
+}
+
+Result Admin::GetDatabaseInfo(const std::string& database_name, DatabaseInfo&
out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->get_database_info(rust::String(database_name));
+ auto result = utils::from_ffi_result(ffi_result.result);
+ if (result.Ok()) {
+ out = utils::from_ffi_database_info(ffi_result.database_info);
+ }
+ return result;
+}
+
+Result Admin::ListTables(const std::string& database_name,
std::vector<std::string>& out) {
+ if (!Available()) {
+ return utils::make_error(1, "Admin not available");
+ }
+
+ auto ffi_result = admin_->list_tables(rust::String(database_name));
+ auto result = utils::from_ffi_result(ffi_result.result);
Review Comment:
This Rust FFI method takes `&str`; constructing a `rust::String` here adds
avoidable allocation/copy. Pass the `std::string` directly (or use `rust::Str`)
instead.
--
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]
