Skip to content

Commit 1f2bf61

Browse files
Sandeep GottimukkalaSandeep Gottimukkala
Sandeep Gottimukkala
authored and
Sandeep Gottimukkala
committed
Precommit fixes
1 parent 0368952 commit 1f2bf61

File tree

8 files changed

+62
-50
lines changed

8 files changed

+62
-50
lines changed

‎src/iceberg/catalog/rest/json_serde.cc‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -560,16 +560,18 @@ nlohmann::json ToJson(const FetchScanTasksRequest& request) {
560560

561561
Status BaseScanTaskResponseFromJson(
562562
const nlohmann::json& json, BaseScanTaskResponse* response,
563-
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>& partition_specs_by_id,
563+
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>&
564+
partition_specs_by_id,
564565
const Schema& schema) {
565566
// 1. plan_tasks
566-
ICEBERG_ASSIGN_OR_RAISE(response->plan_tasks,
567-
GetJsonValueOrDefault<std::vector<std::string>>(json, kPlanTasks));
567+
ICEBERG_ASSIGN_OR_RAISE(
568+
response->plan_tasks,
569+
GetJsonValueOrDefault<std::vector<std::string>>(json, kPlanTasks));
568570

569571
// 2. delete_files
570-
ICEBERG_ASSIGN_OR_RAISE(auto delete_files_json,
571-
GetJsonValueOrDefault<nlohmann::json>(json, kDeleteFiles,
572-
nlohmann::json::array()));
572+
ICEBERG_ASSIGN_OR_RAISE(
573+
auto delete_files_json,
574+
GetJsonValueOrDefault<nlohmann::json>(json, kDeleteFiles, nlohmann::json::array()));
573575
for (const auto& entry_json : delete_files_json) {
574576
ICEBERG_ASSIGN_OR_RAISE(auto delete_file,
575577
DataFileFromJson(entry_json, partition_specs_by_id, schema));
@@ -589,7 +591,8 @@ Status BaseScanTaskResponseFromJson(
589591

590592
Result<PlanTableScanResponse> PlanTableScanResponseFromJson(
591593
const nlohmann::json& json,
592-
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>& partition_specs_by_id,
594+
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>&
595+
partition_specs_by_id,
593596
const Schema& schema) {
594597
PlanTableScanResponse response;
595598
ICEBERG_ASSIGN_OR_RAISE(response.plan_status,
@@ -604,11 +607,11 @@ Result<PlanTableScanResponse> PlanTableScanResponseFromJson(
604607

605608
Result<FetchPlanningResultResponse> FetchPlanningResultResponseFromJson(
606609
const nlohmann::json& json,
607-
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>& partition_specs_by_id,
610+
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>&
611+
partition_specs_by_id,
608612
const Schema& schema) {
609613
FetchPlanningResultResponse response;
610-
ICEBERG_ASSIGN_OR_RAISE(auto status_str,
611-
GetJsonValue<std::string>(json, kPlanStatus));
614+
ICEBERG_ASSIGN_OR_RAISE(auto status_str, GetJsonValue<std::string>(json, kPlanStatus));
612615
response.plan_status = PlanStatus(PlanStatus::FromString(status_str));
613616
ICEBERG_RETURN_UNEXPECTED(
614617
BaseScanTaskResponseFromJson(json, &response, partition_specs_by_id, schema));
@@ -618,7 +621,8 @@ Result<FetchPlanningResultResponse> FetchPlanningResultResponseFromJson(
618621

619622
Result<FetchScanTasksResponse> FetchScanTasksResponseFromJson(
620623
const nlohmann::json& json,
621-
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>& partition_specs_by_id,
624+
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>&
625+
partition_specs_by_id,
622626
const Schema& schema) {
623627
FetchScanTasksResponse response;
624628
ICEBERG_RETURN_UNEXPECTED(

‎src/iceberg/catalog/rest/resource_paths.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,17 @@ class ICEBERG_REST_EXPORT ResourcePaths {
8181
/// \brief Get the /v1/{prefix}/transactions/commit endpoint path.
8282
Result<std::string> CommitTransaction() const;
8383

84-
/// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint path.
84+
/// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan endpoint
85+
/// path.
8586
Result<std::string> ScanPlan(const TableIdentifier& ident) const;
8687

8788
/// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/plan/{plan_id}
8889
/// endpoint path.
89-
Result<std::string> ScanPlan(const TableIdentifier& ident, const std::string& plan_id) const;
90+
Result<std::string> ScanPlan(const TableIdentifier& ident,
91+
const std::string& plan_id) const;
9092

91-
/// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint path.
93+
/// \brief Get the /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint
94+
/// path.
9295
Result<std::string> ScanTask(const TableIdentifier& ident) const;
9396

9497
private:

‎src/iceberg/catalog/rest/rest_catalog.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -527,8 +527,8 @@ Result<PlanTableScanResponse> RestCatalog::PlanTableScan(
527527
ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(request_json));
528528
ICEBERG_ASSIGN_OR_RAISE(
529529
const auto response,
530-
client_->Post(path, json_request, /*headers=*/{},
531-
*ScanPlanErrorHandler::Instance(), *catalog_session_));
530+
client_->Post(path, json_request, /*headers=*/{}, *ScanPlanErrorHandler::Instance(),
531+
*catalog_session_));
532532

533533
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
534534
return PlanTableScanResponseFromJson(json, specs_ref.get(), *schema_ptr);
@@ -543,8 +543,8 @@ Result<FetchPlanningResultResponse> RestCatalog::FetchPlanningResult(
543543

544544
ICEBERG_ASSIGN_OR_RAISE(
545545
const auto response,
546-
client_->Get(path, /*params=*/{}, /*headers=*/{},
547-
*ScanPlanErrorHandler::Instance(), *catalog_session_));
546+
client_->Get(path, /*params=*/{}, /*headers=*/{}, *ScanPlanErrorHandler::Instance(),
547+
*catalog_session_));
548548
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
549549
return FetchPlanningResultResponseFromJson(json, specs_ref.get(), *schema_ptr);
550550
}
@@ -560,8 +560,8 @@ Status RestCatalog::CancelPlanning(const Table& table, const std::string& plan_i
560560
return {};
561561
}
562562

563-
Result<FetchScanTasksResponse> RestCatalog::FetchScanTasks(
564-
const Table& table, const std::string& plan_task) {
563+
Result<FetchScanTasksResponse> RestCatalog::FetchScanTasks(const Table& table,
564+
const std::string& plan_task) {
565565
ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::FetchScanTasks());
566566
ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->ScanTask(table.name()));
567567
ICEBERG_ASSIGN_OR_RAISE(auto schema_ptr, table.schema());
@@ -572,8 +572,8 @@ Result<FetchScanTasksResponse> RestCatalog::FetchScanTasks(
572572
ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request)));
573573
ICEBERG_ASSIGN_OR_RAISE(
574574
const auto response,
575-
client_->Post(path, json_request, /*headers=*/{},
576-
*ScanPlanErrorHandler::Instance(), *catalog_session_));
575+
client_->Post(path, json_request, /*headers=*/{}, *ScanPlanErrorHandler::Instance(),
576+
*catalog_session_));
577577

578578
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body()));
579579
return FetchScanTasksResponseFromJson(json, specs_ref.get(), *schema_ptr);

‎src/iceberg/catalog/rest/types.cc‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ bool BaseScanTaskResponse::operator==(const BaseScanTaskResponse& other) const {
139139
if (a.delete_files().size() != b.delete_files().size()) return false;
140140
for (size_t j = 0; j < a.delete_files().size(); ++j) {
141141
if (!a.delete_files()[j] != !b.delete_files()[j]) return false;
142-
if (a.delete_files()[j] && *a.delete_files()[j] != *b.delete_files()[j]) return false;
142+
if (a.delete_files()[j] && *a.delete_files()[j] != *b.delete_files()[j])
143+
return false;
143144
}
144145
if (a.residual_filter() != b.residual_filter()) return false;
145146
}
@@ -151,7 +152,8 @@ bool PlanTableScanResponse::operator==(const PlanTableScanResponse& other) const
151152
plan_id == other.plan_id;
152153
}
153154

154-
bool FetchPlanningResultResponse::operator==(const FetchPlanningResultResponse& other) const {
155+
bool FetchPlanningResultResponse::operator==(
156+
const FetchPlanningResultResponse& other) const {
155157
return BaseScanTaskResponse::operator==(other) &&
156158
plan_status.ToString() == other.plan_status.ToString();
157159
}
@@ -185,7 +187,8 @@ Status PlanTableScanRequest::Validate() const {
185187
if (snapshot_id.has_value()) {
186188
if (start_snapshot_id.has_value() || end_snapshot_id.has_value()) {
187189
return ValidationFailed(
188-
"Invalid scan: cannot provide both snapshotId and startSnapshotId/endSnapshotId");
190+
"Invalid scan: cannot provide both snapshotId and "
191+
"startSnapshotId/endSnapshotId");
189192
}
190193
}
191194
if (start_snapshot_id.has_value() || end_snapshot_id.has_value()) {
@@ -218,11 +221,13 @@ Status PlanTableScanResponse::Validate() const {
218221
}
219222
if (!plan_id.empty() && plan_status != "submitted" && plan_status != "completed") {
220223
return ValidationFailed(
221-
"Invalid response: plan id can only be defined when status is 'submitted' or 'completed'");
224+
"Invalid response: plan id can only be defined when status is 'submitted' or "
225+
"'completed'");
222226
}
223227
if (file_scan_tasks.empty() && !delete_files.empty()) {
224228
return ValidationFailed(
225-
"Invalid response: deleteFiles should only be returned with fileScanTasks that reference them");
229+
"Invalid response: deleteFiles should only be returned with fileScanTasks that "
230+
"reference them");
226231
}
227232
return {};
228233
}
@@ -238,7 +243,8 @@ Status FetchPlanningResultResponse::Validate() const {
238243
}
239244
if (file_scan_tasks.empty() && !delete_files.empty()) {
240245
return ValidationFailed(
241-
"Invalid response: deleteFiles should only be returned with fileScanTasks that reference them");
246+
"Invalid response: deleteFiles should only be returned with fileScanTasks that "
247+
"reference them");
242248
}
243249
return {};
244250
}
@@ -253,7 +259,8 @@ Status FetchScanTasksRequest::Validate() const {
253259
Status FetchScanTasksResponse::Validate() const {
254260
if (file_scan_tasks.empty() && !delete_files.empty()) {
255261
return ValidationFailed(
256-
"Invalid response: deleteFiles should only be returned with fileScanTasks that reference them");
262+
"Invalid response: deleteFiles should only be returned with fileScanTasks that "
263+
"reference them");
257264
}
258265
if (plan_tasks.empty() && file_scan_tasks.empty()) {
259266
return ValidationFailed(

‎src/iceberg/catalog/rest/types.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,12 +320,12 @@ struct ICEBERG_REST_EXPORT PlanTableScanRequest {
320320
struct ICEBERG_REST_EXPORT BaseScanTaskResponse {
321321
std::vector<std::string> plan_tasks;
322322
std::vector<FileScanTask> file_scan_tasks;
323-
std::vector<DataFile> delete_files;
323+
std::vector<DataFile> delete_files;
324324
// std::unordered_map<std::string, PartitionSpec> specsById;
325325

326326
Status Validate() const { return {}; };
327327

328-
bool operator==(const BaseScanTaskResponse&) const;
328+
bool operator==(const BaseScanTaskResponse&) const;
329329
};
330330

331331
/// \brief Response from initiating a scan planning operation, including plan status and

‎src/iceberg/json_serde.cc‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ constexpr std::string_view kReferencedDataFile = "referenced-data-file";
253253
constexpr std::string_view kContentOffset = "content-offset";
254254
constexpr std::string_view kContentSizeInBytes = "content-size-in-bytes";
255255

256-
257256
} // namespace
258257

259258
nlohmann::json ToJson(const SortField& sort_field) {
@@ -1803,8 +1802,7 @@ Result<DataFile> DataFileFromJson(
18031802
}
18041803
for (size_t pos = 0; pos < fields.size(); ++pos) {
18051804
ICEBERG_ASSIGN_OR_RAISE(
1806-
auto literal,
1807-
LiteralFromJson(partition_vals[pos], fields[pos].type().get()));
1805+
auto literal, LiteralFromJson(partition_vals[pos], fields[pos].type().get()));
18081806
literals.push_back(std::move(literal));
18091807
}
18101808
df.partition = PartitionValues(std::move(literals));
@@ -1844,10 +1842,11 @@ Result<DataFile> DataFileFromJson(
18441842
ICEBERG_ASSIGN_OR_RAISE(auto map_json, GetJsonValue<nlohmann::json>(json, key));
18451843
ICEBERG_ASSIGN_OR_RAISE(auto keys,
18461844
GetJsonValue<std::vector<int32_t>>(map_json, "keys"));
1847-
ICEBERG_ASSIGN_OR_RAISE(auto values,
1848-
GetJsonValue<std::vector<std::vector<uint8_t>>>(map_json, "values"));
1845+
ICEBERG_ASSIGN_OR_RAISE(
1846+
auto values, GetJsonValue<std::vector<std::vector<uint8_t>>>(map_json, "values"));
18491847
if (keys.size() != values.size()) {
1850-
return JsonParseError("'{}' binary map keys and values have different lengths", key);
1848+
return JsonParseError("'{}' binary map keys and values have different lengths",
1849+
key);
18511850
}
18521851
for (size_t i = 0; i < keys.size(); ++i) {
18531852
target[keys[i]] = values[i];
@@ -1871,8 +1870,7 @@ Result<DataFile> DataFileFromJson(
18711870
GetJsonValue<std::vector<int32_t>>(json, kEqualityIds));
18721871
}
18731872
if (json.contains(kSortOrderId) && !json.at(kSortOrderId).is_null()) {
1874-
ICEBERG_ASSIGN_OR_RAISE(df.sort_order_id,
1875-
GetJsonValue<int32_t>(json, kSortOrderId));
1873+
ICEBERG_ASSIGN_OR_RAISE(df.sort_order_id, GetJsonValue<int32_t>(json, kSortOrderId));
18761874
}
18771875
if (json.contains(kFirstRowId) && !json.at(kFirstRowId).is_null()) {
18781876
ICEBERG_ASSIGN_OR_RAISE(df.first_row_id, GetJsonValue<int64_t>(json, kFirstRowId));
@@ -1916,9 +1914,8 @@ Result<std::vector<FileScanTask>> FileScanTasksFromJson(
19161914
std::vector<std::shared_ptr<DataFile>> task_delete_files;
19171915
if (task_json.contains(kDeleteFileReferences) &&
19181916
!task_json.at(kDeleteFileReferences).is_null()) {
1919-
ICEBERG_ASSIGN_OR_RAISE(
1920-
auto refs,
1921-
GetJsonValue<std::vector<int32_t>>(task_json, kDeleteFileReferences));
1917+
ICEBERG_ASSIGN_OR_RAISE(auto refs, GetJsonValue<std::vector<int32_t>>(
1918+
task_json, kDeleteFileReferences));
19221919
for (int32_t ref : refs) {
19231920
if (ref < 0 || static_cast<size_t>(ref) >= delete_files.size()) {
19241921
return JsonParseError(
@@ -1930,8 +1927,7 @@ Result<std::vector<FileScanTask>> FileScanTasksFromJson(
19301927
}
19311928

19321929
std::shared_ptr<Expression> residual_filter;
1933-
if (task_json.contains(kResidualFilter) &&
1934-
!task_json.at(kResidualFilter).is_null()) {
1930+
if (task_json.contains(kResidualFilter) && !task_json.at(kResidualFilter).is_null()) {
19351931
ICEBERG_ASSIGN_OR_RAISE(auto filter_json,
19361932
GetJsonValue<nlohmann::json>(task_json, kResidualFilter));
19371933
ICEBERG_ASSIGN_OR_RAISE(residual_filter, ExpressionFromJson(filter_json));

‎src/iceberg/test/rest_catalog_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@
4040
#include "iceberg/catalog/rest/json_serde_internal.h"
4141
#include "iceberg/catalog/rest/rest_catalog.h"
4242
#include "iceberg/partition_spec.h"
43-
#include "iceberg/table_scan.h"
4443
#include "iceberg/result.h"
4544
#include "iceberg/schema.h"
4645
#include "iceberg/sort_order.h"
4746
#include "iceberg/table.h"
4847
#include "iceberg/table_identifier.h"
4948
#include "iceberg/table_requirement.h"
49+
#include "iceberg/table_scan.h"
5050
#include "iceberg/table_update.h"
5151
#include "iceberg/test/matchers.h"
5252
#include "iceberg/test/std_io.h"

‎src/iceberg/test/rest_json_serde_test.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
#include "iceberg/catalog/rest/json_serde_internal.h"
2727
#include "iceberg/catalog/rest/types.h"
2828
#include "iceberg/partition_spec.h"
29-
#include "iceberg/schema.h"
3029
#include "iceberg/result.h"
30+
#include "iceberg/schema.h"
3131
#include "iceberg/sort_order.h"
3232
#include "iceberg/table_identifier.h"
3333
#include "iceberg/table_metadata.h"
@@ -1381,16 +1381,18 @@ INSTANTIATE_TEST_SUITE_P(
13811381
return info.param.test_name;
13821382
});
13831383

1384-
// Helper: empty schema and specs for scan response tests that don't need partition parsing.
1384+
// Helper: empty schema and specs for scan response tests that don't need partition
1385+
// parsing.
13851386
static Schema EmptySchema() { return Schema({}, 0); }
1386-
static std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>> EmptySpecs() { return {}; }
1387+
static std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>> EmptySpecs() {
1388+
return {};
1389+
}
13871390

13881391
// --- PlanTableScanResponse ---
13891392

13901393
TEST(PlanTableScanResponseFromJsonTest, SubmittedStatusMissingOptionalFields) {
13911394
// "submitted" response: only status and plan-id, no tasks
1392-
auto json = nlohmann::json::parse(
1393-
R"({"status":"submitted","plan-id":"abc-123"})");
1395+
auto json = nlohmann::json::parse(R"({"status":"submitted","plan-id":"abc-123"})");
13941396
auto result = PlanTableScanResponseFromJson(json, EmptySpecs(), EmptySchema());
13951397
ASSERT_THAT(result, IsOk());
13961398
EXPECT_EQ(result->plan_status, "submitted");

0 commit comments

Comments
 (0)