From 11005c5311061ce2610f1612021b393d06ea945c Mon Sep 17 00:00:00 2001 From: dkp116 Date: Thu, 9 Jul 2026 17:57:24 +0100 Subject: [PATCH 1/6] [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 4c4613537bb072f164d7e50e35d747dfeb8620aa Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sun, 20 Sep 2026 16:17:45 +0100 Subject: [PATCH 2/6] 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 444fd891115ed2c4c4b13970d02000b250927994 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sun, 20 Sep 2026 16:18:41 +0100 Subject: [PATCH 3/6] 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 e8d6da0eaa5f20016f46b1313ecade6350d6805c Mon Sep 17 00:00:00 2001 From: dkp116 Date: Mon, 21 Sep 2026 18:31:53 +0100 Subject: [PATCH 4/6] 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 42e98913b3fa8c696ab303189df6546c776ad613 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Thu, 1 Oct 2026 18:01:57 +0100 Subject: [PATCH 5/6] 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 3a256e7bcb65bf3315a79573e63095506e9342a0 Mon Sep 17 00:00:00 2001 From: dkp116 Date: Sat, 3 Oct 2026 14:49:57 +0100 Subject: [PATCH 6/6] 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