Skip to content

Commit 3761bc8

Browse files
committed
fix(upsert_records): drop redundant 'id' key when '_id' present
When a record contains both '_id' and 'id', the SDK previously raised a ValidationError. Per claim unified-bp-0007, '_id' must take precedence and the bare 'id' key must be silently dropped. Apply the same fix to both sync (Index) and async (AsyncIndex) codepaths. Fixes CI-0056; honors unified-bp-0007.
1 parent ca1616d commit 3761bc8

5 files changed

Lines changed: 40 additions & 20 deletions

File tree

‎pinecone/async_client/async_index.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ async def upsert_records(
202202
if "_id" not in r and "id" in r:
203203
r["_id"] = r.pop("id")
204204
elif "_id" in r and "id" in r:
205-
raise ValidationError(f"Record at index {i} cannot have both '_id' and 'id' fields")
205+
del r["id"] # _id wins; drop the redundant 'id' key
206206
resolved_id = r.get("_id")
207207
if not isinstance(resolved_id, str):
208208
got = type(resolved_id).__name__

‎pinecone/index/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ def upsert_records(
531531
if "_id" not in r and "id" in r:
532532
r["_id"] = r.pop("id")
533533
elif "_id" in r and "id" in r:
534-
raise ValidationError(f"Record at index {i} cannot have both '_id' and 'id' fields")
534+
del r["id"] # _id wins; drop the redundant 'id' key
535535
resolved_id = r.get("_id")
536536
if not isinstance(resolved_id, str):
537537
got = type(resolved_id).__name__

‎tests/integration/test_data_plane.py‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2017,11 +2017,19 @@ def test_upsert_records_id_must_be_string() -> None:
20172017
idx.upsert_records(namespace="ns", records=[{"_id": 123, "text": "hello"}])
20182018

20192019

2020-
def test_upsert_records_both_id_fields_rejected() -> None:
2021-
"""upsert_records raises ValidationError when record has both '_id' and 'id' fields."""
2020+
@respx.mock
2021+
def test_upsert_records_both_id_fields_strips_id() -> None:
2022+
"""When both '_id' and 'id' are present, 'id' is dropped and '_id' is used."""
2023+
import json as _json
2024+
2025+
upsert_url = "https://my-index.svc.pinecone.io/records/namespaces/ns/upsert"
2026+
route = respx.post(upsert_url).mock(return_value=httpx.Response(201))
20222027
idx = Index(host="my-index.svc.pinecone.io", api_key="test-key")
2023-
with pytest.raises(ValidationError, match="cannot have both '_id' and 'id'"):
2024-
idx.upsert_records(namespace="ns", records=[{"_id": "a", "id": "b", "text": "hello"}])
2028+
idx.upsert_records(namespace="ns", records=[{"_id": "wins", "id": "loses", "text": "hello"}])
2029+
body = route.calls.last.request.content.decode("utf-8")
2030+
parsed = _json.loads(body.strip())
2031+
assert parsed["_id"] == "wins"
2032+
assert "id" not in parsed
20252033

20262034

20272035
# ---------------------------------------------------------------------------

‎tests/unit/test_async_upsert_records.py‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,20 @@ async def test_async_upsert_records_keyword_only(self) -> None:
116116
with pytest.raises(TypeError):
117117
await idx.upsert_records([{"_id": "r1"}], "ns") # type: ignore[misc]
118118

119+
@respx.mock
119120
@pytest.mark.anyio
120-
async def test_async_upsert_records_both_id_fields_rejected(self) -> None:
121-
"""Records with both '_id' and 'id' fields raise ValidationError."""
121+
async def test_async_upsert_records_both_id_fields_strips_id(self) -> None:
122+
"""When both '_id' and 'id' are present, 'id' is dropped and '_id' wins."""
123+
route = respx.post(UPSERT_URL).mock(return_value=httpx.Response(201, content=b""))
122124
idx = _make_async_index()
123-
with pytest.raises(ValidationError, match="cannot have both '_id' and 'id'"):
124-
await idx.upsert_records(
125-
namespace="test-ns",
126-
records=[{"_id": "a", "id": "b", "text": "hello"}],
127-
)
125+
await idx.upsert_records(
126+
namespace="test-ns",
127+
records=[{"_id": "wins", "id": "loses", "text": "hello"}],
128+
)
129+
body = route.calls.last.request.content.decode("utf-8")
130+
parsed = json.loads(body.strip())
131+
assert parsed["_id"] == "wins"
132+
assert "id" not in parsed
128133

129134
@pytest.mark.anyio
130135
async def test_async_upsert_records_id_must_be_string(self) -> None:

‎tests/unit/test_upsert_records.py‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,21 @@ def test_upsert_records_mixed_id_formats(self) -> None:
139139
assert parsed_1["_id"] == "r2"
140140
assert "id" not in parsed_1
141141

142-
def test_upsert_records_both_id_fields_rejected(self) -> None:
143-
"""Records with both '_id' and 'id' fields raise ValidationError."""
142+
@respx.mock
143+
def test_upsert_records_both_id_fields_strips_id(self) -> None:
144+
"""When both '_id' and 'id' are present, 'id' is dropped and '_id' wins."""
145+
route = respx.post(UPSERT_URL).mock(
146+
return_value=httpx.Response(201),
147+
)
144148
idx = _make_index()
145-
with pytest.raises(ValidationError, match="cannot have both '_id' and 'id'"):
146-
idx.upsert_records(
147-
namespace="test-ns",
148-
records=[{"_id": "a", "id": "b", "text": "hello"}],
149-
)
149+
idx.upsert_records(
150+
namespace="test-ns",
151+
records=[{"_id": "wins", "id": "loses", "text": "hello"}],
152+
)
153+
body = route.calls.last.request.content.decode("utf-8")
154+
parsed = json.loads(body.strip())
155+
assert parsed["_id"] == "wins"
156+
assert "id" not in parsed
150157

151158
def test_upsert_records_id_must_be_string(self) -> None:
152159
"""Records where '_id' is not a string raise ValidationError."""

0 commit comments

Comments
 (0)