Repository navigation
Conversation
| if (sortMatch.matched().size() != 1) { | ||
| throw new JsonMappingException( | ||
| jsonParser, | ||
| "sort clause must have exactly one of $hybrid, $vectorize, $vector, $lexical", | ||
| jsonParser.currentLocation()); | ||
| } |
There was a problem hiding this comment.
I think this is a user-visible error? If so, use specific API error
There was a problem hiding this comment.
Fixed in 787722f: now throws SORT_CLAUSE_INVALID.
There was a problem hiding this comment.
is this a user-visible error? What makes you believe this is the right exception? Why you decided to use JsonMappingException previously? Why did you decide to throw this error or do the validation inside the deserializer?
There was a problem hiding this comment.
I initially treated conflicting keys as a JSON shape error, but SORT_CLAUSE_INVALID gives users a more specific error. The check belongs in the deserializer because it needs one top-level key to select the sort mode. Do you agree with keeping that check in the deserializer?
| // top level has exactly one of $hybrid, $vectorize, $vector or $lexical | ||
| // { "sort" : { "$hybrid" : "i like cheese" } } | ||
| // { "sort" : { "$hybrid" : { "$vectorize" : "i like cheese", "$lexical" : "cows"} } } | ||
| // { "sort" : { "$hybrid" : { "$vector" : [1, 2, 3], "$lexical" : "cows"} } } |
There was a problem hiding this comment.
why delete this example in the comment?
There was a problem hiding this comment.
why delete this example in the comment? Does it become invalid?
There was a problem hiding this comment.
The example is still valid, and there was no behavior change that justified removing it. I have restored it.
| CommandFeature feature) | ||
| throws JsonMappingException { | ||
| return switch (node) { | ||
| case null -> null; |
There was a problem hiding this comment.
what does {"sort": {"$lexical": null}} mean at the top level? Inside $hybrid a null skips that leg, but here it is the only leg. Where is that rejected, and is there a test for it?
There was a problem hiding this comment.
Records LEXICAL with no value; planner rejects it (UNSUPPORTED_FIND_AND_RERANK_SORT, #2602). Test added: failSingleLegModeWithNoValue.
There was a problem hiding this comment.
could you explain this? I don't really understand this
There was a problem hiding this comment.
The parser records a lexical sort with no text, and the test only covers its rejection because single-leg execution is not implemented yet. Are you OK with adding the null and blank checks in #2602 alongside that execution?
This branch has not been deployed
2 participants
What this PR does:
Parse the single-leg findAndRerank sorts. The sort clause now accepts exactly one top-level key:
$hybrid,$vectorize,$vectoror$lexical. The$hybridforms parse as before.{"$vectorize": "text"}isLegMode.VECTORIZE{"$vector": [...]}and{"$vector": {"$binary": "..."}}areLegMode.VECTOR{"$lexical": "text"}isLegMode.LEXICALNull and blank values are recorded as no value, the same as inside
$hybrid. Two top-level keys fail when the sort is parsed. The record rejects a single-leg mode that carries a value for another mode.The value parsing for
$vectorize,$lexicaland$vectorwas moved into two private functions in the deserializer, so the$hybridobject and the top-level keys use the same code. No duplicated parsing.NOTE: this PR itself does not run single-leg sorts. The planner has no legs for these modes yet, so they still fail with
UNSUPPORTED_FIND_AND_RERANK_SORT. Building the legs is #2602, andrerankCandidateLimitand field sorts are not in this PR.NOTE: this PR is stacked on #2607, which is stacked on #2637. Merge those first.
Tests: 217 tests pass on Java 21, and
fmt:checkpasses.FindAndRerankSortClauseDeserializerTest: new cases for the three single-leg sorts, null and blank values,$binaryvectors, two top-level keys, and wrong value typesFindAndRerankPlannerTest: single-leg modes still fail withUNSUPPORTED_FIND_AND_RERANK_SORTFindAndRerankOperationBuilderBehaviorTest: the 75 request shapes for$hybridare unchangedWhich issue(s) this PR fixes:
Refs #2601
Refs #2602
NOTE: this PR does not close #2601 or #2602. It is the parsing slice only.
Checklist