Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements full-text search functionality for Firestore pipelines, adding a new search stage along with SearchOptions and supporting expressions like between, geo_distance, and document_matches. The changes include updates to the base pipeline, expression definitions, and stage logic, supported by new system and unit tests. Review feedback recommends expanding type hints for the between method to include integers and simplifying the Search stage's protobuf serialization by removing a redundant null check.
packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_expressions.py
Show resolved
Hide resolved
| if self.options.query is not None: | ||
| options["query"] = self.options.query._to_pb() |
There was a problem hiding this comment.
The query parameter in SearchOptions.__init__ is not optional, and the initializer ensures self.options.query is always a BooleanExpression. Therefore, this if check for None is redundant and can be removed to simplify the code.
options["query"] = self.options.query._to_pb()
daniel-sanche
changed the title
| select (Optional[Sequence[Selectable | str]]): The fields to keep or add to each document, | ||
| specified as an array of `Selectable` or strings. | ||
| offset (Optional[int]): The number of documents to skip. | ||
| query_enhancement (Optional[str | QueryEnhancement]): Define the query expansion behavior used by full-text search expressions |
There was a problem hiding this comment.
Do we need to mention here that "preferred" is the default value if none provided?
|
I saw something called snippet in code samples https://docs.google.com/document/d/164LK_QblqnsM3kAXqWsNAouOinhmBUgu2mPbTDVlA30/edit?tab=t.0#heading=h.hso4h1r9trkp Will that be included in subsequent PR ? |
|
There's a spot in the doc that says "Scope estimates for Next", listing snippets as being out of scope for the initial release I also confirmed this through the Next26 chat |
2 participants
Implements Search functionality:
searchstagebetweenexpression (as alias forAnd(GreaterThanOrEqual(x), LessThanOrEqual(y))geo_distanceexpression (for use in search stages only)Score()expression (for use in search stages only)