[DRAFT] feat(firestore): pipeline search#16469
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.
|
|
||
| @expose_as_static | ||
| def between( | ||
| self, lower: Expression | float, upper: Expression | float |
There was a problem hiding this comment.
The type hint for lower and upper is Expression | float, which is too restrictive. The docstring example uses integers (Field.of("age").between(18, 65)), which would be flagged as a type error by some static analysis tools. To more accurately reflect the supported types and align with the example, consider widening the type hint to include int.
| self, lower: Expression | float, upper: Expression | float | |
| self, lower: Expression | int | float, upper: Expression | int | float |
| 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()1 participant
WIP