fix(python): raise clear ValueError when vector column cannot be infe…#3567
Conversation
wjones127
left a comment
There was a problem hiding this comment.
Hi @Vitaliy-Pikalo, thanks for submitting this PR. Could you add a unit test for this?
| try: | ||
| vector_column_name = inf_vector_column_query( | ||
| schema, dim=_query_vector_dim(query) | ||
| ) | ||
| except Exception as e: | ||
| raise e | ||
| vector_column_name = inf_vector_column_query( | ||
| schema, dim=_query_vector_dim(query) | ||
| ) |
There was a problem hiding this comment.
question(blocking): Does inf_vector_column_query not raise an error anymore? This seems like a suspicious change worth clarifying.
|
@wjones127 The unit test is already in commit 25f8c65 — |
|
wjones127 The unit test is already in commit 25f8c65 - |
wjones127
left a comment
There was a problem hiding this comment.
Okay, make sense. Looks good.
|
@Vitaliy-Pikalo it looks like your changes might actually be breaking several tests. Please take a look. |
…rred In `infer_vector_column_name`, when `query is None` and `query_type` is not "fts" or "hybrid", the function silently returned `None`. This `None` then propagated downstream causing a cryptic `TypeError`. Also removes the no-op `try/except Exception as e: raise e` wrapper around `inf_vector_column_query`. Fixes lancedb#1653.
… inferred Added a regression test to ensure querying a table without an inferable vector column raises a clear ValueError instead of a TypeError.
infer_vector_column_name raised ValueError whenever vector_column_name stayed None, including for plain scans (query is None, query_type not "hybrid"). Those scans never attempt inference and previously returned None as a no-op, so the new guard broke to_pandas()/to_arrow()/duckdb/ polars integrations and any other call to search() with no query. Only raise when a vector search was actually requested and inference failed.
25f8c65 to
3995729
Compare
Summary
Fixes #1653.
infer_vector_column_nameinutil.pycould silently returnNonewhenquery is Noneandquery_typeis not"fts"or"hybrid". ThisNonethen propagated into downstream code, causing a crypticTypeError: expected bytes, NoneType foundrather than a clear error message.Changes
try/except Exception as e: raise earoundinf_vector_column_query(it was catching and immediately re-raising without adding any value)Noneguard after the inference block: ifvector_column_nameis stillNoneat this point, raise a clearValueErrorpointing the user to passvector_column_nameexplicitlyBefore / After
Before: cryptic
TypeError: expected bytes, NoneType founddeep in schema lookup codeAfter: