This is the purpose of the PR :

In the class PgVectorStore line 239 uses embeddingClient to embed documents.

On the other hand, we have a setEmbedding method in the Document class that lets you add the document embedding. To do this, the developer should use embeddingClient to retrieve the embedding. (use case A)

So, if we use setEmbedding to embed the document, line 239 of class PgVectorStore will make one more request to embed the document.

To avoid this situation, we check that the document hasn't already been embedded before making an embedding request.

embedding_clent

@tzolov you can see the topic, it may be on other vector store implementations, we can find double requests in some cases

Comment From: tzolov

Hi @ricken07 , this is bigger topic as it affects most if not all Vector Store implementations. Also it requires evaluation of the Document logic ensuring that the Embedding index is nullified if the content is changed. I've been postponing fixing this for very long time. Thanks for remanding me about it. I will do my research and will share here. I might as well submit a broader PR

Comment From: tzolov

@ricken07 please have a look at this: https://github.com/spring-projects/spring-ai/pull/413 It should generalise the solution for all vector stores including PG Vector. I wonder to what are the use-cases where multiple addition of the same Document would occur?

Comment From: ricken07

Perhaps when we chunking the document, once with a token size x and a second time with another token size.

Sample : src/main/java/fr/sciam/sprinai/techreview/BasicRAGApplication.java

Comment From: markpollack

When I first created the project, I was looking into langchain and llamaindex to understand those code bases. Llamaindex has the embedding as part of the BaseNode class, which in turn has subclasses TextNode and ImageNode. I adopted the llamaindex structure without really much thought as it was all new to me. Now in retrospect, I don't see the need for our Document class to have the embeddings. I think maybe it was related to using an in-memory embedding client implementation, but I am not sure. I did a search now and didn't find the use case in LlamaIndex where using the embeddings as part of the BaseNode/TextNode class is useful, but I haven't looked deep into all the many layers of code that make use of these indexes in Llama Index.

So long story short, maybe we can just get rid of having Embedding in the Document class. I'll do some more research on this and find out more.