MB-59863: Vector validation, compliant to [email protected] #15
Conversation
Pull Request Test Coverage Report for Build 7040379617
💛 - Coveralls |
abhinavdangeti
force-pushed
the
vectors
branch
2 times, most recently
from
ecc2f49
to
2724d4d
Compare
abhinavdangeti
changed the title
abhinavdangeti
changed the title
abhinavdangeti
requested review from
Thejas-bhat,
metonymic-smokey,
CascadingRadium,
Likith101 and
moshaad7
abhinavdangeti
changed the title
| @@ -29,6 +29,7 @@ type Document struct { | |||
| fieldNames []string | |||
| fieldTokenFreqs []index.TokenFrequencies | |||
| fieldLens []int | |||
| vectorDims []int // applicable to vector fields only | |||
There was a problem hiding this comment.
i didn't understand why the datatype for the vectorDims is an []int instead of int. is it for like per field?
There was a problem hiding this comment.
Because there can be multiple vector fields over which multi knn can be run :)
There was a problem hiding this comment.
yeah and with respect to non-vector fields it will be zero right?
There was a problem hiding this comment.
correct, we don't allow vector fields with dims = 0 anyway - so should be good there.
| v.done = true | ||
| return nil, nil | ||
| } | ||
| return v.Next(preAlloced) |
There was a problem hiding this comment.
this is more of a doubt/minor detail. is it a good pattern to invoke a Next() call inside an Advance() function? since they are two different things in iterators design/principle - although i realize there aren't any iterators in this reader. it seems a little odd to me that we are invoking Next and Advance concepts which are related to iterators on a non-iterative object - more specifically invoking Next() inside of Advance().
There was a problem hiding this comment.
This is a single document index - so there're no other moves to make here :)
See TermFieldReader in term.go, you'll notice the same maneuver.
| } | ||
|
|
||
| func (v *VectorFieldReader) Size() int { | ||
| return 0 |
There was a problem hiding this comment.
i'm not sure if i follow this is bit (so could you please clarify this) - is it supposed to be no-op? like shouldn't it be like a static size of the struct VectorFieldReader?
There was a problem hiding this comment.
During validation we don't really care about this, just a no-op. See TermFieldReader also.
abhinavdangeti
changed the title
Also, move up bleve@unstable Requires: blevesearch/sear#15 Change-Id: Ie1ba3573f4ed754ea7a91f51ba93bc959d466ce1 Reviewed-on: https://review.couchbase.org/c/n1fty/+/201928 Tested-by: Abhi Dangeti <[email protected]> Reviewed-by: Abhi Dangeti <[email protected]>
Given that sear can only ever hold a single document, the VectorReader here will only
make sure that the field is available in the document and that the dimensions set for it
matches that of the query vector.