Repository navigation
fix(lancedb): use paginated __contains__ for table existence check - #86
zzx04-cool wants to merge 1 commit into
Conversation
LanceDB's table_names() defaults to a 10-table limit, so existing tables beyond the first page were incorrectly treated as nonexistent, causing create_table() to fail with "Table already exists". Replace the membership check with `self.table_name not in self.db`, which uses LanceDB's Connection.__contains__ that iterates all pages. Adds regression tests covering tables beyond the first page, new tables, and tables within the first page. Closes aiming-lab#85
|
Gentle ping: this PR has been ready for review for 5 days. It's a one-line fix (plus regression tests) for the LanceDB |
|
Gentle follow-up on this PR. It's been open for a while and remains ready for review. Please let me know if you'd like any adjustments — happy to iterate. |
1 similar comment
|
Gentle follow-up on this PR. It's been open for a while and remains ready for review. Please let me know if you'd like any adjustments — happy to iterate. |
|
Following up once more: this PR has been open for three weeks and is still mergeable as-is — a one-line fix plus regression tests for #85. If there are no objections, could a maintainer review and merge it? If anything needs adjustment, please leave a note and I'll address it promptly. If the change isn't a fit for the project, a quick comment would also be appreciated so I can close it and keep the queue tidy. Thanks! |
|
Friendly ping — this has been open for about three weeks and is still mergeable. Is there anything you'd like changed before a review? The change replaces the unpaginated LanceDB Happy to drop the test or reshape the helper if you'd prefer the lookup inlined. |
Problem
When a LanceDB database contains more than 10 tables, reopening an existing memory table fails with
Table '<name>' already exists.LanceDBVectorStoreBackend._init_table()checks existence viaself.table_name not in self.db.table_names(). In recent LanceDB versions,table_names()is deprecated and defaults to returning only the first 10 tables. An existing table beyond that first page is incorrectly treated as nonexistent, so the backend callscreate_table(), which fails because the table actually already exists.Solution
Replace
self.db.table_names()withself.db(LanceDBConnection.__contains__), which iterates through all table-list pages instead of checking only the first 10.Changes
simplemem/core/database/vector_store_backend.py: one-line fix in_init_table()— useself.dbmembership test instead ofself.db.table_names().tests/test_lancedb_table_pagination.py: new regression test file with 3 cases:Testing
python -m pytest tests/test_lancedb_table_pagination.py -vlancedb.connectwith a_MockDBwhosetable_names()returns only the first 10 entries (reproducing the deprecated default limit) while__contains__sees all 15 tables. This directly exercises the bug path.tests/test_vector_store_backend.pyare unaffected: the change only replaces the membership-check expression, and the happy path (table within first page / new table) behaves identically.Notes