Skip to content

Reject empty source-ids in PartitionField / SortField#3411

Merged
Fokko merged 1 commit into
apache:mainfrom
kevinjqliu:kevinjqliu/walrus-partition-and-sort
May 25, 2026
Merged

Reject empty source-ids in PartitionField / SortField#3411
Fokko merged 1 commit into
apache:mainfrom
kevinjqliu:kevinjqliu/walrus-partition-and-sort

Conversation

@kevinjqliu
Copy link
Copy Markdown
Contributor

@kevinjqliu kevinjqliu commented May 24, 2026

Inspired by the walrus issue in #3353

Problem

PartitionField and SortField accept the v3 source-ids list (added in #1554) and map it to the legacy singular source-id. Both validators try to reject an empty list:

if "source-id" not in data and (source_ids := data["source-ids"]):
    if isinstance(source_ids, list):
        if len(source_ids) == 0:
            raise ValueError("Empty source-ids is not allowed")
        ...
        data["source-id"] = source_ids[0]

The walrus uses truthiness, and [] is falsy — so the len(source_ids) == 0 branch is unreachable. Passing {"source-ids": []} silently skips the mapping, and Pydantic then reports a generic "field required" error instead of the intended message. A missing source-ids key also raises KeyError instead of being handled cleanly.

Fix

Replace the walrus with an explicit key check in both validators:

if "source-id" not in data and "source-ids" in data:
    source_ids = data["source-ids"]
    ...

This makes the empty-list validation reachable and avoids the KeyError.

Tests

Added regression tests that deserialize {"source-ids": []} and assert ValueError("Empty source-ids is not allowed") is raised, for both PartitionField and SortField.

@kevinjqliu kevinjqliu requested a review from Fokko May 24, 2026 21:57
Copy link
Copy Markdown
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

@Fokko Fokko merged commit b7ca7be into apache:main May 25, 2026
16 checks passed
@Fokko
Copy link
Copy Markdown
Contributor

Fokko commented May 25, 2026

Thanks @kevinjqliu for fixing this, and thanks @ndrluis for the prompt review 🚀

@kevinjqliu kevinjqliu deleted the kevinjqliu/walrus-partition-and-sort branch May 25, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants