-
Notifications
You must be signed in to change notification settings - Fork 39
Document new error codes for vector search filtering. #434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document new error codes for vector search filtering. #434
Conversation
renetapopova
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @Lojjs. I pushed some editorial updates, but I also left some suggestions that I am not sure are correct. Please take a look.
dogofbrian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Just some typos and a couple of suggestions.
bdde4e8 to
7522df0
Compare
| = 22ND3 | ||
|
|
||
| == Status description | ||
| error: data exception - wrong property for vector search filtering. The property `{ <<propKey>> }` is not an additional property of the vector index `{ <<idx>> }`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may not be clear what an "additional" property is, depending on how we called this in the original docs (I have not seen that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked @Hunterness about this in another comment thread. She is writing those docs today so we will update this if she ends up calling them something else, additional property is what the concept is called in her CIP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just added a PR for my bits, currently using 'additional property' but we'll see what reviewers say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question for both of you @Hunterness and @arnefischereit: I noticed we ended up using "additional property for filtering" consistently in the docs. Do you think adding the "for filtering" to the error text as well would be an improvement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I've called them both (usually dropping the "for filtering" when I want a short hand/already said that further up the page)
error: data exception - wrong property for vector search filtering. The property
{ <<propKey>> }is not an additional property for filtering of the vector index{ <<idx>> }.
that sounds a bit repetitive and hard to read though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I think I do agree with @Hunterness that this sound rather redundant, I found a lot of the error messages structured {subCondition}. {message} to be repetitive.
In this case, I think that maybe that should be the case and we should make it even more so:
The property ... is not an additional property on ... for vector search filtering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arnefischereit @Hunterness Coming back to this PR now when the general Cypher SEARCH docs are finished.
In the last suggestion, where you thinking it should say
error: data exception - wrong property for vector search filtering. The property is not an additional property on for vector search filtering.
I think that is sounds a bit weird with a property on an index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is this what Arne is after? (not having the on and filling in the ... with the original bits of the message)
error: data exception - wrong property for vector search filtering. The property
{ <<propKey>> }is not an additional property of the vector index{ <<idx>> }for vector search filtering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that sound better, I will go with that unless Arne protest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the above but changed "vector search filtering" to "vector search with filters" which was what we ended up deciding on with David Pond.
d2142d2 to
a1be14e
Compare
0a3695f to
56ac423
Compare
c2e90d5 to
4920db4
Compare
renetapopova
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments from me.
4920db4 to
62c3eca
Compare
|
@Lojjs, is this ready for merging? Shall I take a final look? |
renetapopova
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of questions.
Co-authored-by: Arne Fischereit <79841228+arnefischereit@users.noreply.github.com>
Co-authored-by: Reneta Popova <reneta.popova@neo4j.com>
Co-authored-by: Reneta Popova <reneta.popova@neo4j.com>
efe4e6c to
92e26d3
Compare
|
Thanks for the documentation updates. The preview documentation has now been torn down - reopening this PR will republish it. |
No description provided.