From 800f243db9871625f3211aa33e0883ccc7e0451d Mon Sep 17 00:00:00 2001 From: dkp116 Date: Thu, 9 Jul 2026 17:57:24 +0100 Subject: [PATCH 1/8] [C++][Docs] Add description to KeyValueMetadata::DeleteMany --- cpp/src/arrow/util/key_value_metadata.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cpp/src/arrow/util/key_value_metadata.h b/cpp/src/arrow/util/key_value_metadata.h index 57ade11e7586..b65bbb3dfd46 100644 --- a/cpp/src/arrow/util/key_value_metadata.h +++ b/cpp/src/arrow/util/key_value_metadata.h @@ -50,6 +50,12 @@ class ARROW_EXPORT KeyValueMetadata { // Note that deleting may invalidate known indices Status Delete(std::string_view key); Status Delete(int64_t index); + + /// \brief Delete metadata entries at specified index in keys and values array + /// \param indices Vector of distinct indices identifying the entries to + /// remove from the metadata. + /// \return Status indicating success or failure. + Status DeleteMany(std::vector indices); Status Set(std::string key, std::string value); From c306326b2097a72b7bfab7b76bbbbc5a0e4f50e0 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sun, 20 Sep 2026 16:17:45 +0100 Subject: [PATCH 2/8] GH-50351: [C++] Change validation of DeleteMany to return Status errors so errors would return on all build types --- cpp/src/arrow/util/key_value_metadata.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 67bf02b4e1ac..f8c0be9a3bb5 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -121,10 +121,17 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { ++shift; const auto start = indices[i] + 1; const auto stop = indices[i + 1]; - DCHECK_GE(start, 0); - DCHECK_LE(start, size); - DCHECK_GE(stop, 0); - DCHECK_LE(stop, size); + + if (ARROW_PREDICT_TRUE(start < 0 || start > size)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1, + " out of bounds for metadata of size ", size); + } + + if (ARROW_PREDICT_TRUE(stop < 0 || stop > size)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: Stop index ", stop, + " out of bounds for metadata of size ", size); + } + for (int64_t index = start; index < stop; ++index) { keys_[index - shift] = std::move(keys_[index]); values_[index - shift] = std::move(values_[index]); From 9c8615da62b3225c2efce6d007f701094fbb8af1 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sun, 20 Sep 2026 16:18:41 +0100 Subject: [PATCH 3/8] GH-50351: [C++] Add bug fix for duplicate values in keyvaluemetadata deletemany with unit test --- cpp/src/arrow/util/key_value_metadata.cc | 8 ++++ cpp/src/arrow/util/key_value_metadata_test.cc | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index f8c0be9a3bb5..95f0b806ff19 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -112,6 +112,9 @@ Status KeyValueMetadata::Delete(int64_t index) { } Status KeyValueMetadata::DeleteMany(std::vector indices) { + if (indices.size() == 1) { + return Delete(indices[0]); + } std::sort(indices.begin(), indices.end()); const int64_t size = static_cast(keys_.size()); indices.push_back(size); @@ -133,6 +136,11 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { } for (int64_t index = start; index < stop; ++index) { + if (ARROW_PREDICT_TRUE(index < shift)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", index, + " in indices to delete"); + } + keys_[index - shift] = std::move(keys_[index]); values_[index - shift] = std::move(values_[index]); } diff --git a/cpp/src/arrow/util/key_value_metadata_test.cc b/cpp/src/arrow/util/key_value_metadata_test.cc index 7c021429ac17..946b363277e3 100644 --- a/cpp/src/arrow/util/key_value_metadata_test.cc +++ b/cpp/src/arrow/util/key_value_metadata_test.cc @@ -226,6 +226,47 @@ TEST(KeyValueMetadataTest, Delete) { ASSERT_OK(metadata.DeleteMany({})); ASSERT_TRUE(metadata.Equals(KeyValueMetadata({"bb", "dd", "ee"}, {"2", "4", "5"}))); } + { + KeyValueMetadata metadata(keys, values); + + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: Start index -3 out of bounds for " + "metadata of size 7"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({-2, -3})); + } + + { + KeyValueMetadata metadata(keys, values); + + std::string expected_error_message = + "Index error: KeyValueMetadata::Delete: index -1 is out of bounds for metadata " + "of size 7"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({-1})); + } + + { + KeyValueMetadata metadata(keys, values); + + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: Stop index 8 out of bounds for " + "metadata of size 7"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({0, 8})); + } + { + KeyValueMetadata metadata(keys, values); + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: duplicate index 1 in indices to " + "delete"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({0, 0, 5, 2})); + } } } // namespace arrow From c1870cc6f8452d297d6b3082a2a0959af5bca879 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Mon, 21 Sep 2026 18:31:53 +0100 Subject: [PATCH 4/8] GH-50351: [C++] Use correct Arrow Predict for error handle --- cpp/src/arrow/util/key_value_metadata.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 95f0b806ff19..522b3e4d4f70 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -125,18 +125,18 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { const auto start = indices[i] + 1; const auto stop = indices[i + 1]; - if (ARROW_PREDICT_TRUE(start < 0 || start > size)) { + if (ARROW_PREDICT_FALSE(start < 0 || start > size)) { return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1, " out of bounds for metadata of size ", size); } - if (ARROW_PREDICT_TRUE(stop < 0 || stop > size)) { + if (ARROW_PREDICT_FALSE(stop < 0 || stop > size)) { return Status::IndexError("KeyValueMetadata::DeleteMany: Stop index ", stop, " out of bounds for metadata of size ", size); } for (int64_t index = start; index < stop; ++index) { - if (ARROW_PREDICT_TRUE(index < shift)) { + if (ARROW_PREDICT_FALSE(index < shift)) { return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", index, " in indices to delete"); } From 586e6f29ffcabea7a809f1040d1b7e437752db46 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Thu, 1 Oct 2026 18:01:57 +0100 Subject: [PATCH 5/8] GH-50351: [C++] Correct validation error when duplicate indicies in DeleteMany --- cpp/src/arrow/util/key_value_metadata.cc | 9 ++++----- cpp/src/arrow/util/key_value_metadata_test.cc | 11 ++++++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 522b3e4d4f70..c1270d9e9383 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -125,6 +125,10 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { const auto start = indices[i] + 1; const auto stop = indices[i + 1]; + if (ARROW_PREDICT_FALSE(indices[i] == stop)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", + indices[i], " in indices to delete"); + } if (ARROW_PREDICT_FALSE(start < 0 || start > size)) { return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1, " out of bounds for metadata of size ", size); @@ -136,11 +140,6 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { } for (int64_t index = start; index < stop; ++index) { - if (ARROW_PREDICT_FALSE(index < shift)) { - return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", index, - " in indices to delete"); - } - keys_[index - shift] = std::move(keys_[index]); values_[index - shift] = std::move(values_[index]); } diff --git a/cpp/src/arrow/util/key_value_metadata_test.cc b/cpp/src/arrow/util/key_value_metadata_test.cc index 946b363277e3..599200df2f4e 100644 --- a/cpp/src/arrow/util/key_value_metadata_test.cc +++ b/cpp/src/arrow/util/key_value_metadata_test.cc @@ -261,12 +261,21 @@ TEST(KeyValueMetadataTest, Delete) { { KeyValueMetadata metadata(keys, values); std::string expected_error_message = - "Index error: KeyValueMetadata::DeleteMany: duplicate index 1 in indices to " + "Index error: KeyValueMetadata::DeleteMany: duplicate index 0 in indices to " "delete"; ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.DeleteMany({0, 0, 5, 2})); } + { + KeyValueMetadata metadata(keys, values); + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: duplicate index 6 in indices to " + "delete"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({6, 6})); + } } } // namespace arrow From ef79cdea3a683bd42c13dad4eb035ae0b268502e Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sat, 3 Oct 2026 14:49:57 +0100 Subject: [PATCH 6/8] GH-50351: [C++] Correct order of validation for KeyValueMetaData::DeleteMany --- cpp/src/arrow/util/key_value_metadata.cc | 8 ++++---- cpp/src/arrow/util/key_value_metadata_test.cc | 9 +++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index c1270d9e9383..09f4546fa219 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -125,10 +125,6 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { const auto start = indices[i] + 1; const auto stop = indices[i + 1]; - if (ARROW_PREDICT_FALSE(indices[i] == stop)) { - return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", - indices[i], " in indices to delete"); - } if (ARROW_PREDICT_FALSE(start < 0 || start > size)) { return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1, " out of bounds for metadata of size ", size); @@ -139,6 +135,10 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { " out of bounds for metadata of size ", size); } + if (ARROW_PREDICT_FALSE(indices[i] == stop)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", + indices[i], " in indices to delete"); + } for (int64_t index = start; index < stop; ++index) { keys_[index - shift] = std::move(keys_[index]); values_[index - shift] = std::move(values_[index]); diff --git a/cpp/src/arrow/util/key_value_metadata_test.cc b/cpp/src/arrow/util/key_value_metadata_test.cc index 599200df2f4e..d8ca8cea462e 100644 --- a/cpp/src/arrow/util/key_value_metadata_test.cc +++ b/cpp/src/arrow/util/key_value_metadata_test.cc @@ -276,6 +276,15 @@ TEST(KeyValueMetadataTest, Delete) { ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.DeleteMany({6, 6})); } + { + KeyValueMetadata metadata(keys, values); + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: Stop index 8 out of bounds for " + "metadata of size 7"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({8, 8, 6})); + } } } // namespace arrow From f81acaf3bc05fc71bca7cc3a65d5cc04ea0e2160 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Fri, 16 Oct 2026 22:23:12 +0100 Subject: [PATCH 7/8] GH-50351: [C++] Add more test cases for DeleteMany caught edge case --- cpp/src/arrow/util/key_value_metadata.cc | 6 +++++ cpp/src/arrow/util/key_value_metadata_test.cc | 22 ++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 09f4546fa219..d17afd171236 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -121,6 +121,12 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { int64_t shift = 0; for (int64_t i = 0; i < static_cast(indices.size() - 1); ++i) { + if (ARROW_PREDICT_FALSE(indices[i] < 0 || i >= static_cast(keys_.size()))) { + return Status::IndexError("KeyValueMetadata::DeleteMany: index ", indices[i], + " is out of bounds for metadata " + "of size ", + keys_.size()); + } ++shift; const auto start = indices[i] + 1; const auto stop = indices[i + 1]; diff --git a/cpp/src/arrow/util/key_value_metadata_test.cc b/cpp/src/arrow/util/key_value_metadata_test.cc index d8ca8cea462e..53c5f763c5c0 100644 --- a/cpp/src/arrow/util/key_value_metadata_test.cc +++ b/cpp/src/arrow/util/key_value_metadata_test.cc @@ -230,7 +230,7 @@ TEST(KeyValueMetadataTest, Delete) { KeyValueMetadata metadata(keys, values); std::string expected_error_message = - "Index error: KeyValueMetadata::DeleteMany: Start index -3 out of bounds for " + "Index error: KeyValueMetadata::DeleteMany: index -3 is out of bounds for " "metadata of size 7"; ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, @@ -285,6 +285,26 @@ TEST(KeyValueMetadataTest, Delete) { ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.DeleteMany({8, 8, 6})); } + + { + KeyValueMetadata metadata(keys, values); + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: duplicate index 2 in indices to " + "delete"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({0, 2, 2})); + } + + { + KeyValueMetadata metadata(keys, values); + std::string expected_error_message = + "Index error: KeyValueMetadata::DeleteMany: index -1 is out of bounds for " + "metadata of size 7"; + + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, + metadata.DeleteMany({-1, 0})); + } } } // namespace arrow From 900cfd96387c26cdd1f0c488dfcb6854875ede32 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sat, 17 Oct 2026 14:32:35 +0100 Subject: [PATCH 8/8] GH-50351 : [C++] Refactor DeleteMany member function in KeyValueMetaData, for readability --- cpp/src/arrow/util/key_value_metadata.cc | 56 +++++++++++++++--------- cpp/src/arrow/util/key_value_metadata.h | 3 ++ 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index d17afd171236..4a40a3ce5680 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -111,6 +111,37 @@ Status KeyValueMetadata::Delete(int64_t index) { return Status::OK(); } +Status KeyValueMetadata::KeyValueMetaDataValidation(const std::vector& indices, + const int64_t& start, + const int64_t& stop, + const int64_t& current_index, + const int64_t& size) { + if (ARROW_PREDICT_FALSE(indices[current_index] < 0 || current_index >= size)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: index ", + indices[current_index], + " is out of bounds for metadata " + "of size ", + size); + } + + if (ARROW_PREDICT_FALSE(indices[current_index] == stop)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", + indices[current_index], " in indices to delete"); + } + + if (ARROW_PREDICT_FALSE(start < 0 || start > size)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1, + " out of bounds for metadata of size ", size); + } + + if (ARROW_PREDICT_FALSE(stop < 0 || stop > size)) { + return Status::IndexError("KeyValueMetadata::DeleteMany: Stop index ", stop, + " out of bounds for metadata of size ", size); + } + + return Status::OK(); +} + Status KeyValueMetadata::DeleteMany(std::vector indices) { if (indices.size() == 1) { return Delete(indices[0]); @@ -121,29 +152,12 @@ Status KeyValueMetadata::DeleteMany(std::vector indices) { int64_t shift = 0; for (int64_t i = 0; i < static_cast(indices.size() - 1); ++i) { - if (ARROW_PREDICT_FALSE(indices[i] < 0 || i >= static_cast(keys_.size()))) { - return Status::IndexError("KeyValueMetadata::DeleteMany: index ", indices[i], - " is out of bounds for metadata " - "of size ", - keys_.size()); - } - ++shift; const auto start = indices[i] + 1; const auto stop = indices[i + 1]; - - if (ARROW_PREDICT_FALSE(start < 0 || start > size)) { - return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1, - " out of bounds for metadata of size ", size); - } - - if (ARROW_PREDICT_FALSE(stop < 0 || stop > size)) { - return Status::IndexError("KeyValueMetadata::DeleteMany: Stop index ", stop, - " out of bounds for metadata of size ", size); - } - - if (ARROW_PREDICT_FALSE(indices[i] == stop)) { - return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ", - indices[i], " in indices to delete"); + ++shift; + Status validation_status = KeyValueMetaDataValidation(indices, start, stop, i, size); + if (ARROW_PREDICT_FALSE(!validation_status.ok())) { + return validation_status; } for (int64_t index = start; index < stop; ++index) { keys_[index - shift] = std::move(keys_[index]); diff --git a/cpp/src/arrow/util/key_value_metadata.h b/cpp/src/arrow/util/key_value_metadata.h index b65bbb3dfd46..fc87814edd3a 100644 --- a/cpp/src/arrow/util/key_value_metadata.h +++ b/cpp/src/arrow/util/key_value_metadata.h @@ -57,6 +57,9 @@ class ARROW_EXPORT KeyValueMetadata { /// \return Status indicating success or failure. Status DeleteMany(std::vector indices); + Status KeyValueMetaDataValidation(const std::vector& indices, + const int64_t& start, const int64_t& stop, + const int64_t& current_index, const int64_t& size); Status Set(std::string key, std::string value); void reserve(int64_t n);