Skip to content
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

Removed limit param from vector_match() #2080

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

snej
Copy link
Collaborator

@snej snej commented Jun 24, 2024

vector_match() no longer takes an optional 3rd parameter.
The limit should now be passed as the overall LIMIT of the SELECT.

Also switched to using MATCH instead of LIKE in the generated SQL, because I found MATCH is slightly more efficient.

Also, with the latest vectorsearch extension, there were a couple of test failures caused by unexpected "Untrained index..." warnings. Fixed them.

With the latest vectorsearch extension, there were a couple of test
failures caused by unexpected "Untrained index..." warnings.
@snej snej force-pushed the fix/vector_match_no_limit-CBL-5902 branch from cf41b43 to 64de52d Compare June 24, 2024 19:43
@snej snej requested a review from pasin June 24, 2024 21:20
Copy link
Collaborator

@pasin pasin left a comment

Choose a reason for hiding this comment

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

"N1QL Vector Search" test needs update.

@pasin
Copy link
Collaborator

pasin commented Jun 24, 2024

Error from PR validation for "N1QL Vector Search" test failure:

[2024-06-24T20:23:12.728Z] -----------------------------------------------------------------------------------------------------------------------
[2024-06-24T20:23:12.728Z] N1QL Vector Search
[2024-06-24T20:23:12.728Z] -----------------------------------------------------------------------------------------------------------------------
[2024-06-24T20:23:12.728Z] /var/lib/jenkins/mobile/workspace/line_couchbase-lite-core_PR-2080/couchbase-lite-core/LiteCore/tests/N1QLParserTest.cc:573
[2024-06-24T20:23:12.728Z] .......................................................................................................................
[2024-06-24T20:23:12.728Z] 
[2024-06-24T20:23:12.728Z] /var/lib/jenkins/mobile/workspace/line_couchbase-lite-core_PR-2080/couchbase-lite-core/LiteCore/tests/N1QLParserTest.cc:580: FAILED:
[2024-06-24T20:23:12.728Z]   CHECK( translate("SELECT META().id, VECTOR_DISTANCE(vecIndex) AS distance " "WHERE VECTOR_MATCH(vecIndex, $target, 5) ORDER BY distance") == "{'ORDER_BY':[['.distance']],'WHAT':[['_.',['meta()'],'.id']," "['AS',['VECTOR_DISTANCE()','vecIndex'],'distance']]," "'WHERE':['VECTOR_MATCH()','vecIndex',['$target'],5]}" )
[2024-06-24T20:23:12.728Z] due to unexpected exception with messages:
[2024-06-24T20:23:12.728Z]   N1QL: SELECT META().id, VECTOR_DISTANCE(vecIndex) AS distance WHERE VECTOR_MATCH(vecIndex, $target, 5) ORDER BY
[2024-06-24T20:23:12.728Z]   distance
[2024-06-24T20:23:12.728Z]   {'ORDER_BY':[['.distance']],'WHAT':[['_.',['meta()'],'.id'],['AS',['VECTOR_DISTANCE()','vecIndex'],'distance']],
[2024-06-24T20:23:12.728Z]   'WHERE':['VECTOR_MATCH()','vecIndex',['$target'],5]}
[2024-06-24T20:23:12.728Z]   Too many arguments for function 'VECTOR_MATCH'

vector_match() no longer takes an optional 3rd parameter.
The limit should now be passed as the overall LIMIT of the SELECT.

Also switched to using MATCH instead of LIKE in the generated SQL,
because I found MATCH is slightly more efficient.
@snej snej force-pushed the fix/vector_match_no_limit-CBL-5902 branch from 64de52d to ab37982 Compare June 24, 2024 21:47
@cbl-bot
Copy link

cbl-bot commented Jun 24, 2024

Code Coverage Results:

Type Percentage
branches 68.36
functions 79.29
instantiations 35.32
lines 78.91
regions 75.53

@jianminzhao jianminzhao merged commit dcaf693 into master Jun 25, 2024
9 checks passed
@jianminzhao jianminzhao deleted the fix/vector_match_no_limit-CBL-5902 branch June 25, 2024 02:13
callumbirks pushed a commit that referenced this pull request Jun 26, 2024
* Fixed vector-search test failures due to Untrained-index warning

With the latest vectorsearch extension, there were a couple of test
failures caused by unexpected "Untrained index..." warnings.

* Removed limit param from vector_match()

vector_match() no longer takes an optional 3rd parameter.
The limit should now be passed as the overall LIMIT of the SELECT.

Also switched to using MATCH instead of LIKE in the generated SQL,
because I found MATCH is slightly more efficient.

(cherry picked from commit dcaf693)
jianminzhao added a commit that referenced this pull request Jul 11, 2024
CBL-5813: Flaky test, "Multiple Collections Incremental Revisions" (#2088)
dcaf693 Removed limit param from vector_match() (#2080)
def4ffb Correct set vecOpt.lazy to lazyEmbedding.
7d953d8 Pick up VectorOptions.lazy from c4Options.lazy
312e654 Clear SQL statement bindings after use (#2072)
03ca4bd Use same vector-index options struct as vectorsearch repo (#2058)
280d305 Added explanatory note to c4index_beginUpdate doc-comment [CBL-5842] (#2070)
CBL-5594: Increase Rev Tree Depth Limit to 100 (#2066)
CBL-5785: Deadlock when setParentObjectRef is called (#2063)
CBL-5639: Crash in setting Housekeeper::_doExpiration() (#2059)
CBL-5685: Null dereference crash in gotHTTPResponse (#2060)
CBL-5633: The mailbox's thread-pool may hang. (#2055)
CBL-5798: Passive replicator incorrectly errors on collection ID in the BLIP message (#2051)
bdcac52 Allow inspecting contents of vector index with c4db_getIndexRows() (#2050)
2c1f5f4 LazyIndex: reject wrong-dimensional vectors [CBL-5814] (#2053)
CBL-5680 + CBL-5681: ReplacementRev enhancement (#2021)
CBL-5688: Update replication protocol doc per ReplacementRev changes. (#2039)
jianminzhao added a commit that referenced this pull request Aug 26, 2024
CBL-5813: Flaky test, "Multiple Collections Incremental Revisions" (#2088)
dcaf693 Removed limit param from vector_match() (#2080)
def4ffb Correct set vecOpt.lazy to lazyEmbedding.
7d953d8 Pick up VectorOptions.lazy from c4Options.lazy
312e654 Clear SQL statement bindings after use (#2072)
03ca4bd Use same vector-index options struct as vectorsearch repo (#2058)
280d305 Added explanatory note to c4index_beginUpdate doc-comment [CBL-5842] (#2070)
CBL-5594: Increase Rev Tree Depth Limit to 100 (#2066)
CBL-5785: Deadlock when setParentObjectRef is called (#2063)
CBL-5639: Crash in setting Housekeeper::_doExpiration() (#2059)
CBL-5685: Null dereference crash in gotHTTPResponse (#2060)
CBL-5633: The mailbox's thread-pool may hang. (#2055)
CBL-5798: Passive replicator incorrectly errors on collection ID in the BLIP message (#2051)
bdcac52 Allow inspecting contents of vector index with c4db_getIndexRows() (#2050)
2c1f5f4 LazyIndex: reject wrong-dimensional vectors [CBL-5814] (#2053)
CBL-5680 + CBL-5681: ReplacementRev enhancement (#2021)
CBL-5688: Update replication protocol doc per ReplacementRev changes. (#2039)
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.

4 participants