Conversation
Signed-off-by: cmuhao <sduxuhao@gmail.com>
Signed-off-by: cmuhao <sduxuhao@gmail.com>
| "pgvector_enabled" in config.online_config | ||
| and config.online_config["pgvector_enabled"] | ||
| "pgvector_enabled" in config.online_store | ||
| and config.online_store.pgvector_enabled |
There was a problem hiding this comment.
Shouldn't this be some sort of a feature view-level config? This global config makes it impossible to use postgres both for traditional feature lookup and vector search at the same time, isn't that right?
There was a problem hiding this comment.
That's good point. Before the pr we were unable to do that because vector is mixed with feature value. Now it's doable as vector is a new feature now. Let me see how that works
There was a problem hiding this comment.
@tokoko on a second thought, I would rather not mess up the feature view at the moment. Until we figure out a better design.
Also it still won't impact the original read write flow with or without the pgvrctor_enabled.
And Postgres user has to install the extension before it's able to use the pgvector, so it's better to have this config at high level as well.
franciscojavierarceo
left a comment
There was a problem hiding this comment.
This global config makes it impossible to use postgres both for traditional feature lookup and vector search at the same time, isn't that right?
Agreed with @tokoko here. I'll approve but we should plan to tackle this soon because context aware RAG would be very useful for a case in which you want to inject structured data into the context along with the documents.
Hmmm, the get_online_feature will still work :) |
|
@HaoXuAI I also think this is not urgent, but we should definitely try to end up with a good design of how these two worlds will interact in the future, for example as @franciscojavierarceo pointed out, can there be a use case when a single feature service would generate the results containing results from both feature lookup and vector search? In this specific case I was referring to the fact that if |
With this pr it should work, the get_list_val_str will return null for not acceptable value type. And the vector_value field is null able. And materialization should still work in that way. |
What this PR does / why we need it:
Make the postgres online store compatible with old write and read, when the pgvector is enabled. This is done by adding a new column as
vector_value, to store the vector value if the pgvector is enabled.Which issue(s) this PR fixes:
Fixes