Repository: run-llama/llama_index
Pull request: feat: PgVectorStore support advanced metadata filtering
# Description

Apply `MetadataFilters.condition` to PGVectorStore query.

Fixes #9376 

## Type of Change

- [x] Bug fix (non-breaking change which fixes an issue)

# How Has This Been Tested?

Tested on Python 3.11 on Windows with:
* no metadata filters applied
* metadata filters with a single filter and either an "and" or "or" condition
* multiple metadata filters with either an "and" or "or" condition.

- [x] I stared at the code and made sure it makes sense

Due to limitations of my environment (work computer), I cannot create/run automated tests, or run `make`.

# Suggested Checklist:

- [x] I have performed a self-review of my own code

Problem statement:
[Bug]: PGVectorStore metadata filters behaviour
### Bug Description

Applying the following `MetadataFilters` object via `PGVectorStore.as_retriever` does not behave as expected:

```python
vector_store = PGVectorStore.from_params(
    database=DB_NAME,
    host=url.host,
    password=url.password,
    port=url.port,
    user=url.username,
    table_name=DB_TABLE,
    embed_dim=1536,  # openai embedding dimension
    perform_setup=True,
    hybrid_search=True,
    text_search_config="english",
)

storage_context = StorageContext.from_defaults(
    vector_store=vector_store,
)

vector_index = VectorStoreIndex.from_vector_store(
    vector_store=vector_store, storage_context=storage_context
)

# I have several files loaded into the VectorStore already.
# I want to retrieve chunks from only two of them.
filenames = ["interpretation-act.pdf", "llama2.pdf"]

filters = MetadataFilters(
    filters=[
        MetadataFilter(key="filename", value=filename) for filename in filenames
    ],
    condition="or",  # This is being ignored
)

retriever = vector_index.as_retriever(
    vector_store_query_mode="hybrid",
    similarity_top_k=top_k,
    filters=filters,
)
```

The `PGVectorStore._apply_filters_and_limit` function creates the following SQL, which includes `AND` operators, not `OR` as requested.

```sql
SELECT public.data_gradio_test.id, public.data_gradio_test.text, public.data_gradio_test.metadata_, public.data_gradio_test.node_id, public.data_gradio_test.embedding, public.data_gradio_test.text_search_tsv, public.data_gradio_test.embedding <=> :embedding_1 AS anon_1 
FROM public.data_gradio_test 
WHERE metadata_->>'filename' = :value_filename AND metadata_->>'filename' = :value_filename ORDER BY public.data_gradio_test.embedding <=> :embedding_2
 LIMIT :param_1

SELECT public.data_gradio_test.id, public.data_gradio_test.text, public.data_gradio_test.metadata_, public.data_gradio_test.node_id, public.data_gradio_test.embedding, public.data_gradio_test.text_search_tsv, ts_rank(public.data_gradio_test.text_search_tsv, plainto_tsquery(:param_1, :plainto_tsquery_1)) AS rank 
FROM public.data_gradio_test 
WHERE (public.data_gradio_test.text_search_tsv @@ plainto_tsquery(:param_1, :plainto_tsquery_1)) AND metadata_->>'filename' = :value_filename AND metadata_->>'filename' = :value_filename ORDER BY rank desc
 LIMIT :param_2
```

This is due to the code in that function ignoring the `MetadataFilters.condition` property entirely:
```python
def _apply_filters_and_limit(
    self,
    stmt: Select,
    limit: int,
    metadata_filters: Optional[MetadataFilters] = None,
) -> Any:
    import sqlalchemy

    if metadata_filters:
        for filter_ in metadata_filters.legacy_filters():
            bind_parameter = f"value_{filter_.key}"
            stmt = stmt.where(  # type: ignore
                sqlalchemy.text(f"metadata_->>'{filter_.key}' = :{bind_parameter}")
            )
            stmt = stmt.params(  # type: ignore
                **{bind_parameter: str(filter_.value)}
            )
    return stmt.limit(limit)  # type: ignore
```

This was quite easy to fix so I will open a Pull Request momentarily.

### Version

0.9.13

### Steps to Reproduce

Pass a `MetadataFilters` object with an "or" condition to `PGVectorStore.as_retriever(filters=...)`. See example code above.

### Relevant Logs/Tracbacks

_No response_

Hints from issue discussion:
_🤖_
<!-- Answer -->
That's great to hear that you've identified the issue and are planning to open a Pull Request to address it. Your contribution will be valuable in improving the functionality of LlamaIndex. Thank you for taking the initiative to help enhance the framework! If you have any questions or need assistance with the PR process, feel free to ask.


---

This response is meant to be useful and save you time. It is **not meant to be a precise solution**, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@juleskuehn pgvector store does not support the newer metadata filter abstractions yet, hence, `condition=` is not working yet

Feel free to make a PR to add support though. There is also the fact that it does not use the `operator` in the metadata filter yet either

Commit message: fix: PGVectorStore metadata filters conditions

Patch to review:
diff --git a/llama_index/vector_stores/postgres.py b/llama_index/vector_stores/postgres.py
index bedb5da24e..91e9bc2a43 100644
--- a/llama_index/vector_stores/postgres.py
+++ b/llama_index/vector_stores/postgres.py
@@ -331,15 +331,25 @@ def _apply_filters_and_limit(
     ) -> Any:
         import sqlalchemy
 
+        sqlalchemy_conditions = {
+            "or": sqlalchemy.sql.or_,
+            "and": sqlalchemy.sql.and_,
+        }
+
         if metadata_filters:
-            for filter_ in metadata_filters.legacy_filters():
-                bind_parameter = f"value_{filter_.key}"
-                stmt = stmt.where(  # type: ignore
-                    sqlalchemy.text(f"metadata_->>'{filter_.key}' = :{bind_parameter}")
+            if metadata_filters.condition not in sqlalchemy_conditions:
+                raise ValueError(
+                    f"Invalid condition: {metadata_filters.condition}. "
+                    f"Must be one of {list(sqlalchemy_conditions.keys())}"
                 )
-                stmt = stmt.params(  # type: ignore
-                    **{bind_parameter: str(filter_.value)}
+            stmt = stmt.where(
+                sqlalchemy_conditions[metadata_filters.condition](
+                    *(sqlalchemy.text(
+                        f"metadata_->>'{filter_.key}' = '{filter_.value}'"
+                    )
+                    for filter_ in metadata_filters.legacy_filters())
                 )
+            )
         return stmt.limit(limit)  # type: ignore
 
     def _build_query(


Code review comments:
diff_hunk: @@ -331,15 +331,25 @@ def _apply_filters_and_limit(
     ) -> Any:
         import sqlalchemy
 
+        sqlalchemy_conditions = {
+            "or": sqlalchemy.sql.or_,
+            "and": sqlalchemy.sql.and_,
+        }
+
         if metadata_filters:
-            for filter_ in metadata_filters.legacy_filters():
-                bind_parameter = f"value_{filter_.key}"
-                stmt = stmt.where(  # type: ignore
-                    sqlalchemy.text(f"metadata_->>'{filter_.key}' = :{bind_parameter}")
+            if metadata_filters.condition not in sqlalchemy_conditions:
+                raise ValueError(
+                    f"Invalid condition: {metadata_filters.condition}. "
+                    f"Must be one of {list(sqlalchemy_conditions.keys())}"
                 )
-                stmt = stmt.params(  # type: ignore
-                    **{bind_parameter: str(filter_.value)}
+            stmt = stmt.where(
+                sqlalchemy_conditions[metadata_filters.condition](
+                    *(sqlalchemy.text(
+                        f"metadata_->>'{filter_.key}' = '{filter_.value}'"
+                    )
+                    for filter_ in metadata_filters.legacy_filters())
path: llama_index/vector_stores/postgres.py
text: @user1:
@author this is great.
btw `legacy_filters` is going to be deprecated. Can you change it to use `MetadataFilter`. And `MetadataFilter` has `operator` support as well.
Related code : https://github.com/run-llama/llama_index/blob/main/llama_index/vector_stores/types.py#L82-L94

@author:
Good idea. I'll give it a try.