Skip to content

Fix: #2577 - IllegalArgumentException when using findAndRerank without specifying $vector/$vectorize - #2584

Open
sl-at-ibm wants to merge 2 commits into
mainfrom
SL-issue2577-B-improveErrorMessage
Open

sl-at-ibm wants to merge 2 commits into
mainfrom
SL-issue2577-B-improveErrorMessage

Conversation

@sl-at-ibm

Copy link
Copy Markdown
Contributor

What this PR does:

Replaces a naked IllegalArgumentException / Unexpected server error occurring when a findAndRerank command lacks a prescription on how to run the vector leg, explicitly or implicitly (i.e. no $vector/$vectorize in the sort clause and no shorthand $hybrid: <string> form for sort).

Which issue(s) this PR fixes:

Fixes #2577 .

Notes:

This PR replaces #2579 , which is going to be closed after discussion (that one approached this problem by enabling a no-vector/ize findAndRerank, which was later clarified to not be in scope).

Note also that this PR does not simply replace the generic naked error with throwing the user-friendly new exception added to errors.yaml. Rather, it moves such validation up in the chain, so that the missing vector prescription is detected and thrown first, before "secondary" problems (such as, missing rerankOn) are detected. This, I believe, improves the experience of a developer approaching this command.

As a last point, I have added unit tests around the various findAndRerank call pattern that are supposed to trigger this error, as well as a few cases where the command should succeed. Given the intricacies of this API command, and the fact that clarifications were needed a few times around its exact behaviour, I deemed it reasonable to try and have the test be more prescriptive about the expected constraints of this command.

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • [n/a] Documentation added/updated
  • [n/a] CLA Signed: DataStax CLA

@github-actions

Copy link
Copy Markdown
Contributor

Unit Test Coverage Report

Overall Project 54.48% 🍏
Files changed 82.61% 🍏

File Coverage
RequestException.java 100% 🍏
FindAndRerankOperationBuilder.java 92.61% -0.54% 🍏

@github-actions

Copy link
Copy Markdown
Contributor

📉 Integration Test Coverage Delta vs Main Branch (dse69-it)

Metric Value
Main Branch 71.98%
This PR 71.98%
Delta 🔴 -0.01%
⚠️ Coverage decreased

@github-actions

Copy link
Copy Markdown
Contributor

Integration Test Coverage Report (dse69-it)

Overall Project 71.98% -0.01% 🍏
Files changed 65.22% 🍏

File Coverage
RequestException.java 100% 🍏
FindAndRerankOperationBuilder.java 84.54% -1.08%

INVALID_CREATE_COLLECTION_FIELD,
INVALID_RERANK_OVERRIDE,
MISSING_RERANK_QUERY_TEXT,
MISSING_VECTOR_OR_VECTORIZE_IN_HYBRID_SORT,

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.

this is an error in the sort clause, we should put the error in that class. Will do a longer review later today

This branch has not been deployed

No deployments
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.

IllegalArgumentException / UNEXPECTED_SERVER_ERROR in FindAndRerank when omitting $vectorize in $hybrid

2 participants