integrations: LanceDB update #13416
integrations: LanceDB update #13416
Conversation
dosubot
bot
added
the
size:L
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
@ravi03071991 review pls |
...ns/vector_stores/llama-index-vector-stores-lancedb/llama_index/vector_stores/lancedb/base.py
Outdated
Show resolved
Hide resolved
| ) | ||
|
|
||
| if table: | ||
| assert isinstance(table, lancedb.db.LanceTable) |
There was a problem hiding this comment.
please add a complimentary error msg here
| if query_type == "vector": | ||
| _query = query.query_embedding | ||
| else: | ||
| self._table.create_fts_index(self.text_key, replace=True) |
There was a problem hiding this comment.
This will create index every time query is called, let's keep track of index if that's created in an instance variable and not recreate index every time
There was a problem hiding this comment.
so this is also done because if the user adds more data, a full reindex is needed from our(lance) end, creating it every time is also not the best way but we would have to handle this in the future.
Resolving it as per your comment as of now thanks.
There was a problem hiding this comment.
hmmm..actually just set the flag to False every time table.add is called? just use fts_index_created or something
There was a problem hiding this comment.
yes I actually just did that haha, basically setting the instance variable back to default and on query time we are checking the variable status
| self._table.create_fts_index(self.text_key, replace=True) | ||
| if query_type == "hybrid": | ||
| _query = (query.query_embedding, query.query_str) | ||
| else: |
There was a problem hiding this comment.
elif query_type="fts" and else: raise ValueError("invalid query_type")
|
|
||
| def __init__( | ||
| self, | ||
| uri: Optional[str], | ||
| table_name: str = "vectors", | ||
| connection: Optional[Any] = None, |
There was a problem hiding this comment.
will changing the order of args affect backwards compat? If so, let's try to add the new args towards the end?
There was a problem hiding this comment.
Yes, this was supposed to be a minor breaking change to synch with other integrations, thanks ill change it back.
Feature update :
from_table- creates index from existing tableUpdated demo notebook ( tested locally ) and tested via make commands.