GH-50348: [C++] Fix duplicate keys issue in KeyValueMetadata

[OmBiradar]

Rationale for this change

KeyValueMetadata uses 2 vectors internally to manage the keys and the values. This causes it to accept duplicate keys. When new key-value pairs are appended such that the key already exists in the metadata, the metadata just accepts it as a duplicate, but while finding the key, it returns the first inserted value, not the last updated value. This causes the updated key added using the Append function to become "virtually non-existant". I use an hashmap - unordered_map to store pairs of (key, index) where index is the index of the key in the vector keys_. This enables significant performance gains also in some methods of KeyValueMetadata.

What changes are included in this PR?

• I added an unordered_map along with the existing vectors to keep track of the key value pairs in the metadata.
• Logic to avoid duplicate keys is added.
• If a user adds an already existing key, then the existing key's value gets update rather than making a duplicate entry.
• Reduces the time complexity of FindKey from O(n) to O(1).
• Equals works properly with the new logic

Are these changes tested?

I have added tests in key_value_metadata_test.cc testing the duplicate keys scenario.
I tested them locally, all tests pass.

Are there any user-facing changes?

No, I tried to keep the API as it is.

• GitHub Issue: [C++] KeyValueMetadata accepts duplicate keys #50348

[github-actions]

⚠️ GitHub issue #50348 has been automatically assigned in GitHub to PR creator.

[Copilot]

Pull request overview

This PR aims to make arrow::KeyValueMetadata enforce unique keys (updating values on re-insert) and speed up key lookups by adding an internal unordered_map index, addressing duplicate-key bugs described in GH-50348.

Changes:

• Add an internal unordered_map index (map_) to provide O(1) key lookup and enable in-place value updates for duplicate inserts.
• Update Append, FindKey, and deletion logic to use/maintain the index.
• Add a unit test covering duplicate-key append semantics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
cpp/src/arrow/util/key_value_metadata.h	Adds map_ member to index key → vector position.
cpp/src/arrow/util/key_value_metadata.cc	Implements map-backed append/find and attempts to keep the index in sync across mutations.
cpp/src/arrow/util/key_value_metadata_test.cc	Adds test coverage for duplicate-key behavior (and suggests extending coverage for bulk deletions + vector-construction).

Comments suppressed due to low confidence (1)

cpp/src/arrow/util/key_value_metadata.cc:142

• DeleteMany mutates keys_/values_ but doesn’t rebuild map_. After this PR, lookups (FindKey, Contains, Get, Set, Append) depend on map_, so DeleteMany can return stale indices or “find” deleted keys unless map_ is updated.

  }
  return Status::OK();
}

Status KeyValueMetadata::DeleteMany(std::vector<int64_t> indices) {

[OmBiradar]

I am still refactoring and adding a comprehensive test suite

[OmBiradar]

I see tests failing due to pyarrow having duplicate keys support.

        # test duplicate key support
        md = pa.KeyValueMetadata([
            ('a', 'alpha'),
            ('b', 'beta'),
            ('a', 'Alpha'),
            ('a', 'ALPHA'),
        ])
    
        expected = [
            (b'a', b'alpha'),
            (b'b', b'beta'),
            (b'a', b'Alpha'),
            (b'a', b'ALPHA')
        ]
>       assert len(md) == 4
E       assert 2 == 4
E        +  where 2 = len(\n-- metadata --\na: ALPHA\nb: beta)

Before opening the issue #50348, I had put up the idea to eliminate the duplicate keys support in KeyValueMetadata in arrow community meeting with @ianmcook, as it has problems with value retrieval and locating keys.

I received positive feedback over his, and so I decided to approach this.

@pitrou it would be really nice to have your feedback on this matter. My main question is should duplicates be allowed in KeyValueMetadata?