gsandeep1241 commented on code in PR #614:
URL: https://github.com/apache/iceberg-cpp/pull/614#discussion_r3199141626
##########
src/iceberg/catalog/rest/error_handlers.cc:
##########
@@ -183,4 +186,52 @@ Status ViewCommitErrorHandler::Accept(const ErrorResponse&
error) const {
return DefaultErrorHandler::Accept(error);
}
+const std::shared_ptr<ScanPlanErrorHandler>& ScanPlanErrorHandler::Instance() {
Review Comment:
Done!
##########
src/iceberg/catalog/rest/error_handlers.cc:
##########
@@ -183,4 +186,52 @@ Status ViewCommitErrorHandler::Accept(const ErrorResponse&
error) const {
return DefaultErrorHandler::Accept(error);
}
+const std::shared_ptr<ScanPlanErrorHandler>& ScanPlanErrorHandler::Instance() {
+ static const std::shared_ptr<ScanPlanErrorHandler> instance{new
ScanPlanErrorHandler()};
+ return instance;
+}
+
+Status ScanPlanErrorHandler::Accept(const ErrorResponse& error) const {
+ switch (error.code) {
+ case 404:
+ if (error.type == kNoSuchNamespaceException) {
+ return NoSuchNamespace(error.message);
+ }
+ if (error.type == kNoSuchTableException) {
+ return NoSuchTable(error.message);
+ }
+ if (error.type == kNoSuchPlanIdException) {
+ return NoSuchPlanId(error.message);
+ }
+ if (error.type == kNoSuchPlanTaskException) {
+ return NoSuchPlanTask(error.message);
+ }
+ return NotFound(error.message);
+ case 406:
+ return NotSupported(error.message);
Review Comment:
NoSuchPlanTask is definitely not required. After @zhjwpku 's comment, I
created a new PlanTaskHandler and handled it there. This is a leftover.
Removing it now.
As for the 406, I got it from here:
https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L705
This is not handled in the DefaultErrorHandler. Wdyt?
##########
src/iceberg/catalog.h:
##########
@@ -188,6 +190,44 @@ class ICEBERG_EXPORT Catalog {
/// \return a Table instance or ErrorKind::kAlreadyExists if the table
already exists
virtual Result<std::shared_ptr<Table>> RegisterTable(
const TableIdentifier& identifier, const std::string&
metadata_file_location) = 0;
+
+ /// \brief Initiate a scan planning operation for the given table.
+ ///
+ /// \param table The table to scan.
+ /// \param context The scan context containing snapshot, filter, and other
options.
+ /// \return A PlanTableScanResponse with the plan status and initial scan
tasks.
+ virtual Result<rest::PlanTableScanResponse> PlanTableScan(
Review Comment:
I debated this and I felt that a catalog handler should really honor a
Request-Response contract. (That said, I'm not doing the "Request" part as
such). The java implementation I felt handled core logic directly coupled with
the handlers.
But if we want to keep it similar, it makes sense to split into two PRs to
prevent this from getting complicated. I'll remove the stuff in catalog.h and
rest_catalog.h.
Agree?
##########
src/iceberg/catalog/rest/resource_paths.cc:
##########
@@ -102,4 +102,27 @@ Result<std::string> ResourcePaths::CommitTransaction()
const {
return std::format("{}/v1/{}transactions/commit", base_uri_, prefix_);
}
+Result<std::string> ResourcePaths::ScanPlan(const TableIdentifier& ident)
const {
+ ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
EncodeNamespace(ident.ns));
+ ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name,
EncodeString(ident.name));
+ return std::format("{}/v1/{}namespaces/{}/tables/{}/plan", base_uri_,
prefix_,
+ encoded_namespace, encoded_table_name);
+}
+
+Result<std::string> ResourcePaths::ScanPlan(const TableIdentifier& ident,
+ const std::string& plan_id) const {
+ ICEBERG_ASSIGN_OR_RAISE(std::string encoded_namespace,
EncodeNamespace(ident.ns));
+ ICEBERG_ASSIGN_OR_RAISE(std::string encoded_table_name,
EncodeString(ident.name));
+ ICEBERG_ASSIGN_OR_RAISE(std::string encoded_plan_id, EncodeString(plan_id));
+ return std::format("{}/v1/{}namespaces/{}/tables/{}/plan/{}", base_uri_,
prefix_,
+ encoded_namespace, encoded_table_name, encoded_plan_id);
+}
+
+Result<std::string> ResourcePaths::ScanTask(const TableIdentifier& ident)
const {
Review Comment:
Done!
##########
src/iceberg/catalog/rest/types.h:
##########
@@ -295,4 +298,73 @@ struct ICEBERG_REST_EXPORT OAuthTokenResponse {
bool operator==(const OAuthTokenResponse&) const = default;
};
+/// \brief Request to initiate a server-side scan planning operation.
+struct ICEBERG_REST_EXPORT PlanTableScanRequest {
+ std::optional<int64_t> snapshot_id;
+ std::vector<std::string> select;
+ std::shared_ptr<Expression> filter;
+ bool case_sensitive = true;
+ bool use_snapshot_schema = false;
+ std::optional<int64_t> start_snapshot_id;
+ std::optional<int64_t> end_snapshot_id;
+ std::vector<std::string> statsFields;
+ std::optional<int64_t> min_rows_required;
Review Comment:
Done
##########
src/iceberg/catalog/rest/types.h:
##########
@@ -295,4 +298,73 @@ struct ICEBERG_REST_EXPORT OAuthTokenResponse {
bool operator==(const OAuthTokenResponse&) const = default;
};
+/// \brief Request to initiate a server-side scan planning operation.
+struct ICEBERG_REST_EXPORT PlanTableScanRequest {
+ std::optional<int64_t> snapshot_id;
+ std::vector<std::string> select;
+ std::shared_ptr<Expression> filter;
+ bool case_sensitive = true;
+ bool use_snapshot_schema = false;
+ std::optional<int64_t> start_snapshot_id;
+ std::optional<int64_t> end_snapshot_id;
+ std::vector<std::string> statsFields;
Review Comment:
Done.
##########
src/iceberg/catalog/rest/resource_paths.h:
##########
@@ -81,6 +81,19 @@ class ICEBERG_REST_EXPORT ResourcePaths {
/// \brief Get the /v1/{prefix}/transactions/commit endpoint path.
Result<std::string> CommitTransaction() const;
+ /// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan
endpoint
+ /// path.
+ Result<std::string> ScanPlan(const TableIdentifier& ident) const;
Review Comment:
Makes sense, done.
##########
src/iceberg/catalog/rest/types.h:
##########
@@ -295,4 +298,73 @@ struct ICEBERG_REST_EXPORT OAuthTokenResponse {
bool operator==(const OAuthTokenResponse&) const = default;
};
+/// \brief Request to initiate a server-side scan planning operation.
+struct ICEBERG_REST_EXPORT PlanTableScanRequest {
+ std::optional<int64_t> snapshot_id;
+ std::vector<std::string> select;
+ std::shared_ptr<Expression> filter;
+ bool case_sensitive = true;
+ bool use_snapshot_schema = false;
+ std::optional<int64_t> start_snapshot_id;
+ std::optional<int64_t> end_snapshot_id;
+ std::vector<std::string> statsFields;
+ std::optional<int64_t> min_rows_required;
+
+ Status Validate() const;
+
+ bool operator==(const PlanTableScanRequest&) const;
+};
+
+/// \brief Base response containing scan tasks and delete files returned by
scan plan
+/// endpoints.
+struct ICEBERG_REST_EXPORT BaseScanTaskResponse {
+ std::vector<std::string> plan_tasks;
+ std::vector<FileScanTask> file_scan_tasks;
Review Comment:
I think you mean vector<shared_ptr>, right?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
