Build query strings dynamically

Author: abrookins

redisvl/query/query.py

@@ -14,19 +14,19 @@ class BaseQuery(RedisQuery):
     Base query class used to subclass many query types.
 
     NOTE: In the base class, the `_query_string` field is set once on
-    initialization, and afterward, the redis-py codebase expects to be able to
-    read it. By contrast, our query subclasses allow users to call methods that
-    alter the query string at runtime. To avoid having to rebuild
-    `_query_string` every time one of these methods is called, we lazily build
-    the query string when a user calls `query()` or accesses the property
-    `_query_string`, when the underlying `__query_string` field is None. Any
-    method that alters the query string should set `__query_string` to None so
-    that the next time the query string is accessed, it is rebuilt.
+    initialization, and afterward, redis-py expects to be able to read it. By
+    contrast, our query subclasses allow users to call methods that alter the
+    query string at runtime. To avoid having to rebuild `_query_string` every
+    time one of these methods is called, we lazily build the query string when a
+    user calls `query()` or accesses the property `_query_string`, when the
+    underlying `_built_query_string` field is None. Any method that alters the query
+    string should set `_built_query_string` to None so that the next time the query
+    string is accessed, it is rebuilt.
     """
 
     _params: Dict[str, Any] = {}
     _filter_expression: Union[str, FilterExpression] = FilterExpression("*")
-    __query_string: Optional[str] = None
+    _built_query_string: Optional[str] = None
 
     def __init__(self, query_string: str = "*"):
         """
@@ -35,8 +35,15 @@ def __init__(self, query_string: str = "*"):
         Args:
             query_string (str, optional): The query string to use. Defaults to '*'.
         """
+        # The parent class expects a query string, so we pass it in, but we'll
+        # actually manage building it dynamically.
         super().__init__(query_string)
 
+        # This is a private field that we use to track whether the query string
+        # has been built, and we set it to None here to indicate that the field
+        # has not been built yet.
+        self._built_query_string = None
+
     def __str__(self) -> str:
         """Return the string representation of the query."""
         return " ".join([str(x) for x in self.get_args()])
@@ -68,7 +75,7 @@ def set_filter(
             )
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     @property
     def filter(self) -> Union[str, FilterExpression]:
@@ -88,14 +95,14 @@ def params(self) -> Dict[str, Any]:
     @property
     def _query_string(self) -> str:
         """Maintains compatibility with parent class while providing lazy loading."""
-        if self.__query_string is None:
-            self.__query_string = self._build_query_string()
-        return self.__query_string
+        if self._built_query_string is None:
+            self._built_query_string = self._build_query_string()
+        return self._built_query_string
 
     @_query_string.setter
     def _query_string(self, value: Optional[str]):
         """Setter for _query_string to maintain compatibility with parent class."""
-        self.__query_string = value
+        self._built_query_string = value
 
 
 class FilterQuery(BaseQuery):
@@ -134,7 +141,7 @@ def __init__(
 
         # Initialize the base query with the query string from the property
         super().__init__("*")
-        self.__query_string = None  # Ensure it's invalidated after initialization
+        self._built_query_string = None  # Ensure it's invalidated after initialization
 
         # Handle query settings
         if return_fields:
@@ -188,7 +195,7 @@ def __init__(
 
         # Initialize the base query with the query string from the property
         super().__init__("*")
-        self.__query_string = None
+        self._built_query_string = None
 
         # Query specific modifications
         self.no_content().paging(0, 0).dialect(dialect)
@@ -203,7 +210,8 @@ def _build_query_string(self) -> str:
 class BaseVectorQuery:
     DISTANCE_ID: str = "vector_distance"
     VECTOR_PARAM: str = "vector"
-    EF_RUNTIME_PARAM: str = "EF_RUNTIME"
+    EF_RUNTIME: str = "EF_RUNTIME"
+    EF_RUNTIME_PARAM: str = "EF"
 
     _normalize_vector_distance: bool = False
 
@@ -296,7 +304,7 @@ def __init__(
 
         # Initialize the base query
         super().__init__("*")
-        self.__query_string = None
+        self._built_query_string = None
 
         # Handle query modifiers
         if return_fields:
@@ -345,7 +353,7 @@ def _build_query_string(self) -> str:
 
         # Add EF_RUNTIME parameter if specified
         if self._ef_runtime:
-            knn_query += f" {self.EF_RUNTIME_PARAM} {self._ef_runtime}"
+            knn_query += f" {self.EF_RUNTIME} ${self.EF_RUNTIME_PARAM}"
 
         # Add distance field alias
         knn_query += f" AS {self.DISTANCE_ID}"
@@ -370,7 +378,7 @@ def set_hybrid_policy(self, hybrid_policy: str):
             )
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def set_batch_size(self, batch_size: int):
         """Set the batch size for the query.
@@ -389,7 +397,7 @@ def set_batch_size(self, batch_size: int):
         self._batch_size = batch_size
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def set_ef_runtime(self, ef_runtime: int):
         """Set the EF_RUNTIME parameter for the query.
@@ -409,7 +417,7 @@ def set_ef_runtime(self, ef_runtime: int):
         self._ef_runtime = ef_runtime
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     @property
     def hybrid_policy(self) -> Optional[str]:
@@ -556,7 +564,7 @@ def __init__(
 
         # Initialize the base query
         super().__init__("*")
-        self.__query_string = None
+        self._built_query_string = None
 
         if epsilon is not None:
             self.set_epsilon(epsilon)
@@ -615,7 +623,7 @@ def set_distance_threshold(self, distance_threshold: float):
         self._distance_threshold = distance_threshold
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def set_epsilon(self, epsilon: float):
         """Set the epsilon parameter for the range query.
@@ -635,7 +643,7 @@ def set_epsilon(self, epsilon: float):
         self._epsilon = epsilon
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def set_hybrid_policy(self, hybrid_policy: str):
         """Set the hybrid policy for the query.
@@ -655,7 +663,7 @@ def set_hybrid_policy(self, hybrid_policy: str):
             )
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def set_batch_size(self, batch_size: int):
         """Set the batch size for the query.
@@ -674,7 +682,7 @@ def set_batch_size(self, batch_size: int):
         self._batch_size = batch_size
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def set_ef_runtime(self, ef_runtime: int):
         """Set the EF_RUNTIME parameter for the query.
@@ -694,7 +702,7 @@ def set_ef_runtime(self, ef_runtime: int):
         self._ef_runtime = ef_runtime
 
         # Invalidate the query string
-        self.__query_string = None
+        self._built_query_string = None
 
     def _build_query_string(self) -> str:
         """Build the full query string for vector range queries with optional filtering"""
@@ -708,6 +716,9 @@ def _build_query_string(self) -> str:
         if self._epsilon is not None:
             attr_parts.append(f"$EPSILON: {self._epsilon}")
 
+        if self._ef_runtime is not None:
+            attr_parts.append(f"$EF_RUNTIME: {self._ef_runtime}")
+
         # Add query attributes section
         attr_section = f"=>{{{'; '.join(attr_parts)}}}"
 
@@ -782,10 +793,6 @@ def params(self) -> Dict[str, Any]:
             self.DISTANCE_THRESHOLD_PARAM: self._distance_threshold,
         }
 
-        # Add EPSILON parameter if specified
-        if self._epsilon is not None:
-            params[self.EPSILON_PARAM] = self._epsilon
-
         # Add hybrid policy and batch size as query parameters (not in query string)
         if self._hybrid_policy is not None:
             params[self.HYBRID_POLICY_PARAM] = self._hybrid_policy.value
@@ -795,10 +802,6 @@ def params(self) -> Dict[str, Any]:
             ):
                 params[self.BATCH_SIZE_PARAM] = self._batch_size
 
-        # Add EF_RUNTIME parameter if specified
-        if self._ef_runtime is not None:
-            params[self.EF_RUNTIME_PARAM] = self._ef_runtime
-
         return params
 
 
@@ -894,7 +897,7 @@ def __init__(
 
         # Initialize the base query
         super().__init__("*")
-        self.__query_string = None
+        self._built_query_string = None
 
         # handle query settings
         self.scorer(text_scorer)

tests/unit/test_query_types.py

@@ -535,7 +535,7 @@ def test_vector_range_query_construction():
     assert "$YIELD_DISTANCE_AS: vector_distance" in query_string
     assert "$EPSILON: 0.05" in query_string
     assert epsilon_query.epsilon == 0.05
-    assert epsilon_query.params.get("EPSILON") == 0.05
+    assert "EPSILON" not in epsilon_query.params
 
     # Range query with hybrid policy
     hybrid_query = VectorRangeQuery(
@@ -571,6 +571,20 @@ def test_vector_range_query_construction():
     assert batch_query.params["HYBRID_POLICY"] == "BATCHES"
     assert batch_query.params["BATCH_SIZE"] == 50
 
+    # Range query with ef_runtime
+    ef_runtime_query = VectorRangeQuery(
+        vector=[0.1, 0.1, 0.5],
+        vector_field_name="user_embedding",
+        return_fields=["user", "credit_score"],
+        distance_threshold=0.2,
+        ef_runtime=100,
+    )
+
+    # EF_RUNTIME should be in the query string, not params
+    query_string = str(ef_runtime_query)
+    assert "$EF_RUNTIME: 100" in query_string
+    assert "EF_RUNTIME" not in ef_runtime_query.params
+
 
 def test_vector_range_query_setter_methods():
     """Unit test: Test setter methods for VectorRangeQuery parameters."""
@@ -594,6 +608,11 @@ def test_vector_range_query_setter_methods():
     assert query.epsilon == 0.1
     assert "$EPSILON: 0.1" in str(query)
 
+    # Set ef_runtime
+    query.set_ef_runtime(100)
+    assert query.ef_runtime == 100
+    assert "$EF_RUNTIME: 100" in str(query)
+
     # Set hybrid policy
     query.set_hybrid_policy("BATCHES")
     assert query.hybrid_policy == "BATCHES"
@@ -635,6 +654,13 @@ def test_vector_range_query_error_handling():
     with pytest.raises(ValueError, match="batch_size must be positive"):
         query.set_batch_size(-10)
 
+    # Test invalid ef_runtime
+    with pytest.raises(TypeError, match="ef_runtime must be an integer"):
+        query.set_ef_runtime("hey")  # type: ignore
+
+    with pytest.raises(ValueError, match="ef_runtime must be positive"):
+        query.set_ef_runtime(-10)
+
 
 def test_vector_query_ef_runtime():
     """Test that VectorQuery correctly handles EF_RUNTIME parameter."""
@@ -646,10 +672,10 @@ def test_vector_query_ef_runtime():
 
     # Check query string
     query_string = str(vector_query)
-    assert "EF_RUNTIME 100" in query_string
+    assert f"{VectorQuery.EF_RUNTIME} ${VectorQuery.EF_RUNTIME_PARAM}" in query_string
 
     # Check params dictionary
-    assert vector_query.params.get("EF_RUNTIME") == 100
+    assert vector_query.params.get(VectorQuery.EF_RUNTIME_PARAM) == 100
 
     # Test with different value
     vector_query = VectorQuery([0.1, 0.2, 0.3, 0.4], "vector_field", ef_runtime=50)
@@ -659,7 +685,7 @@ def test_vector_query_ef_runtime():
 
     # Check query string
     query_string = str(vector_query)
-    assert "EF_RUNTIME 50" in query_string
+    assert "EF_RUNTIME $EF" in query_string
 
 
 def test_vector_query_set_ef_runtime():
@@ -669,7 +695,9 @@ def test_vector_query_set_ef_runtime():
 
     # Initially no ef_runtime
     assert vector_query.ef_runtime is None
-    assert "EF_RUNTIME" not in str(vector_query)
+    assert f"{VectorQuery.EF_RUNTIME} ${VectorQuery.EF_RUNTIME_PARAM}" not in str(
+        vector_query
+    )
 
     # Set ef_runtime
     vector_query.set_ef_runtime(200)
@@ -679,17 +707,17 @@ def test_vector_query_set_ef_runtime():
 
     # Check query string
     query_string = str(vector_query)
-    assert "EF_RUNTIME 200" in query_string
+    assert f"{VectorQuery.EF_RUNTIME} ${VectorQuery.EF_RUNTIME_PARAM}" in query_string
 
     # Check params dictionary
-    assert vector_query.params.get("EF_RUNTIME") == 200
+    assert vector_query.params.get(VectorQuery.EF_RUNTIME_PARAM) == 200
 
 
 def test_vector_query_invalid_ef_runtime():
     """Test error handling for invalid EF_RUNTIME values."""
     # Test with invalid ef_runtime type
     with pytest.raises(TypeError, match="ef_runtime must be an integer"):
-        VectorQuery([0.1, 0.2, 0.3, 0.4], "vector_field", ef_runtime="100")
+        VectorQuery([0.1, 0.2, 0.3, 0.4], "vector_field", ef_runtime="hey")  # type: ignore
 
     # Test with invalid ef_runtime value
     with pytest.raises(ValueError, match="ef_runtime must be positive"):
@@ -703,7 +731,7 @@ def test_vector_query_invalid_ef_runtime():
 
     # Test with invalid ef_runtime type
     with pytest.raises(TypeError, match="ef_runtime must be an integer"):
-        vector_query.set_ef_runtime("100")
+        vector_query.set_ef_runtime("hey")  # type: ignore
 
     # Test with invalid ef_runtime value
     with pytest.raises(ValueError, match="ef_runtime must be positive"):
@@ -724,11 +752,12 @@ def test_vector_range_query_ef_runtime():
     assert range_query.ef_runtime == 100
     assert range_query.distance_threshold == 0.3
 
-    # EF_RUNTIME should be in params but not in query string (like hybrid_policy)
-    assert "EF_RUNTIME" not in str(range_query)
-    assert range_query.params.get("EF_RUNTIME") == 100
+    # EF_RUNTIME should not be in params and should be in query string
+    assert f"${VectorRangeQuery.EF_RUNTIME}: 100" in str(range_query)
+    assert VectorRangeQuery.EF_RUNTIME_PARAM not in range_query.params
 
     # Test setting ef_runtime
     range_query.set_ef_runtime(150)
     assert range_query.ef_runtime == 150
-    assert range_query.params.get("EF_RUNTIME") == 150
+    assert f"${VectorRangeQuery.EF_RUNTIME}: 150" in str(range_query)
+    assert VectorRangeQuery.EF_RUNTIME_PARAM not in range_query.params