Improve os read #1065
Improve os read #1065
Conversation
| @@ -47,6 +48,9 @@ def read_records(self, query_params: BaseDBReader.QueryParams) -> "OpenSearchRea | |||
| if "size" not in query_params.query and "size" not in query_params.kwargs: | |||
| query_params.kwargs["size"] = 200 | |||
| result = [] | |||
| # We only fetch the minimum required fields for full document retrieval/reconstruction | |||
| if query_params.reconstruct_document: | |||
| query_params.kwargs["_source_includes"] = "doc_id,parent_id,properties" | |||
There was a problem hiding this comment.
I've looked into only fetching "parent_id", but the problem with only getting child documents here is that we also need parent docs (which do not have "parent_id") and their properties as during explode() we do not transfer all properties from parent to child docs:
sycamore/lib/sycamore/sycamore/transforms/explode.py
Lines 49 to 51 in 8471d43
Otherwise, we would have to make another round of calls to OpenSearch just to fetch parent docs which may be less efficient and more time consuming than just fetching doc_id, parent_id and properties here.
There was a problem hiding this comment.
I don't understand why you need properties. At least in my testing; I was getting parent ids in the _id field.
There was a problem hiding this comment.
Do we not need to reconstruct all of the properties? If that is the case, I think we can just reconstruct using only the child elements.
There was a problem hiding this comment.
This is the test I ran to verify correctness:
No description provided.