Skip to content

opensearch-java: fix the search_after values sent by HybridSpout - #2237

Merged
jnioche merged 2 commits into
mainfrom
hybridspout-search-after
Oct 13, 2026
Merged

jnioche merged 2 commits into
mainfrom
hybridspout-search-after

Conversation

@dpol1

@dpol1 dpol1 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

I hit this while comparing the two OpenSearch modules for #2212. With the opensearch.status.bucket.sort.field of the sample configuration (nextFetchDate, url), every query HybridSpout sends to reload a queue fails with Exception with OpenSearch query. The root cause in the OpenSearch response is:

failed to parse date field [org.opensearch.client.opensearch._types.FieldValue@c82c3cf8] with format [date_optional_time]

The spout then emits only the URLs returned by the aggregations. HybridSpout keeps the sort values as FieldValue objects and was sending their toString() in search_after. 3.6.0 and 3.7.0 are affected. The legacy opensearch module passes the sort values as they are and is not affected.

The new HybridSpoutTest puts three URLs in one queue and reads one URL per query, so the second and third come from reloads. It expects all three; on main it gets only the first.

HybridSpout passed the toString() of the FieldValue sort values to
search_after, so OpenSearch rejected every queue reload when sorting
on a date field.

Signed-off-by: Davide Polato <dpol1@apache.org>
@dpol1
dpol1 requested review from jnioche and rzo1 October 12, 2026 20:20
@dpol1 dpol1 self-assigned this Oct 12, 2026
@dpol1 dpol1 added this to the 4.0.0 milestone Oct 12, 2026

@jnioche jnioche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the fix itself looks very straightforward
I am not sure this warrants a test though. We want to strike the right balance between testing for what really matters and have too many tests - which makes building slower etc...
Made a similar call in #2235

@dpol1

dpol1 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Fair enough, removed it. On build time, AbstractOpenSearchTest starts a new container for every test method, sharing one would save more.

@dpol1
dpol1 requested a review from jnioche October 13, 2026 13:43
@jnioche
jnioche merged commit 4fdcc40 into main Oct 13, 2026
1 check passed
@jnioche
jnioche deleted the hybridspout-search-after branch October 13, 2026 13:44
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.

2 participants