Skip to content

Fix null issue when parsing search request attributes#2312

Merged
abhinavdangeti merged 11 commits intomasterfrom
nullIssue
Apr 15, 2026
Merged

Fix null issue when parsing search request attributes#2312
abhinavdangeti merged 11 commits intomasterfrom
nullIssue

Conversation

@CascadingRadium
Copy link
Copy Markdown
Member

@CascadingRadium CascadingRadium commented Apr 14, 2026

  • We allow parsing a few attributes of the search request as json.RawMessage like
    • KNN Params
    • KNN Filter
    • PresearchData
  • It is possible for the JSON body to contain the keyword null for these attributes, especially after a marshal-unmarshal cycle.
  • This null value is passed on downstream, causing every check for len(attribute) > 0 to succeed and take the path for which the attribute is valid even if the attribute is actually nil.
  • Fix this issue by adding a custom type that parses null specially and sets the attribute to nil when required.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 14, 2026

Coverage Status

coverage: 51.666% (-0.02%) from 51.682% — nullIssue into master

@Likith101 Likith101 requested a review from Copilot April 14, 2026 07:01
@CascadingRadium CascadingRadium changed the title Fix null issue when parsing search request objects Fix null issue when parsing search request attributes Apr 14, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses an edge case in SearchRequest JSON parsing where attributes stored as json.RawMessage can retain the literal bytes "null" after decoding, causing downstream len(...) > 0 checks to behave as if the attribute is present. It introduces a custom raw-message wrapper to treat JSON null as an unset/nil value during unmarshalling.

Changes:

  • Introduce OptionalRawMessage to decode JSON null (and empty) into nil, preventing "null" payloads from being treated as present.
  • Update SearchRequest.UnmarshalJSON (vectors + non-vectors) to use OptionalRawMessage for params and pre_search_data, and similarly for KNN params/filter.
  • Add a vectors-mode regression test covering knn[].params: null and knn[].filter: null, including a marshal/unmarshal round-trip.

Reviewed changes

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

File Description
search.go Adds OptionalRawMessage wrapper type for json.RawMessage-like decoding with special null handling.
search_knn.go Uses OptionalRawMessage in vectors-mode SearchRequest parsing for KNN/raw attributes; also adjusts JSON tags.
search_no_knn.go Uses OptionalRawMessage in non-vectors SearchRequest parsing for raw attributes; also adjusts JSON tags.
search_knn_test.go Adds vectors-mode test coverage for KNN null params/filter (and round-trip behavior).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread search_knn.go
Comment thread search_no_knn.go
Comment thread search_knn_test.go
Comment thread search.go Outdated
CascadingRadium and others added 3 commits April 14, 2026 12:49
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@CascadingRadium CascadingRadium self-assigned this Apr 14, 2026
@CascadingRadium CascadingRadium moved this from Todo to In Progress in GPU-Accelerated Vector Search Apr 14, 2026
@abhinavdangeti abhinavdangeti added this to the v2.6.0 milestone Apr 14, 2026
@abhinavdangeti abhinavdangeti merged commit 4051279 into master Apr 15, 2026
10 of 11 checks passed
@abhinavdangeti abhinavdangeti deleted the nullIssue branch April 15, 2026 18:50
@github-project-automation github-project-automation Bot moved this from In Progress to Done in GPU-Accelerated Vector Search Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

4 participants