From e601bd1702a4459ba967a191a42896a99b512c2e Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Mon, 29 Sep 2025 21:41:04 -0700 Subject: [PATCH 1/5] feat: add support for multiple field sorting specifications (#373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhanced sort_by parameter to accept multiple formats: - Single field: sort_by="price" - Field with direction: sort_by=("price", "DESC") - Multiple fields: sort_by=["price", ("rating", "DESC")] Note: Redis Search only supports single-field sorting, so only the first field is used for actual sorting. A warning is logged when multiple fields are specified. This provides a flexible API for future enhancements while working within Redis's current limitations. Changes: - Added SortSpec type alias for flexible sort specifications - Implemented _parse_sort_spec() static method to normalize sort inputs - Overrode sort_by() method in BaseQuery with validation and warnings - Updated type hints for FilterQuery, VectorQuery, VectorRangeQuery, TextQuery - Added comprehensive unit tests for multiple field sorting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- redisvl/query/query.py | 187 +++++++++++++++++++++-- tests/unit/test_multi_field_sorting.py | 199 +++++++++++++++++++++++++ 2 files changed, 375 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_multi_field_sorting.py diff --git a/redisvl/query/query.py b/redisvl/query/query.py index d2eb5ee6..9ee66173 100644 --- a/redisvl/query/query.py +++ b/redisvl/query/query.py @@ -5,12 +5,22 @@ from redisvl.query.filter import FilterExpression from redisvl.redis.utils import array_to_buffer +from redisvl.utils.log import get_logger from redisvl.utils.token_escaper import TokenEscaper from redisvl.utils.utils import denorm_cosine_distance, lazy_import +logger = get_logger(__name__) + nltk = lazy_import("nltk") nltk_stopwords = lazy_import("nltk.corpus.stopwords") +# Type alias for sort specification +# Can be: +# - str: single field name (ASC by default) +# - Tuple[str, str]: (field_name, direction) +# - List: list of field names or tuples +SortSpec = Union[str, Tuple[str, str], List[Union[str, Tuple[str, str]]]] + class BaseQuery(RedisQuery): """ @@ -58,6 +68,144 @@ def _build_query_string(self) -> str: """Build the full Redis query string.""" raise NotImplementedError("Must be implemented by subclasses") + @staticmethod + def _parse_sort_spec(sort_spec: Optional[SortSpec]) -> List[Tuple[str, bool]]: + """Parse sort specification into list of (field, ascending) tuples. + + Args: + sort_spec: Sort specification in various formats: + - str: single field name (defaults to ASC) + - Tuple[str, str]: (field_name, "ASC"|"DESC") + - List: list of strings or tuples + + Returns: + List of (field_name, ascending) tuples where ascending is a boolean. + + Raises: + TypeError: If sort_spec is not a valid type. + ValueError: If direction is not "ASC" or "DESC". + + Examples: + >>> BaseQuery._parse_sort_spec("price") + [("price", True)] + >>> BaseQuery._parse_sort_spec(("price", "DESC")) + [("price", False)] + >>> BaseQuery._parse_sort_spec(["price", ("rating", "DESC")]) + [("price", True), ("rating", False)] + """ + if sort_spec is None or sort_spec == []: + return [] + + result: List[Tuple[str, bool]] = [] + + # Single field as string + if isinstance(sort_spec, str): + result.append((sort_spec, True)) # Default to ASC + + # Single field as tuple + elif isinstance(sort_spec, tuple): + if len(sort_spec) != 2: + raise ValueError( + f"Sort tuple must have exactly 2 elements (field, direction), got {len(sort_spec)}" + ) + field, direction = sort_spec + if not isinstance(field, str): + raise TypeError(f"Field name must be a string, got {type(field)}") + if not isinstance(direction, str): + raise TypeError(f"Direction must be a string, got {type(direction)}") + + direction_upper = direction.upper() + if direction_upper not in ("ASC", "DESC"): + raise ValueError( + f"Sort direction must be 'ASC' or 'DESC', got '{direction}'" + ) + + result.append((field, direction_upper == "ASC")) + + # Multiple fields as list + elif isinstance(sort_spec, list): + for item in sort_spec: + # Recursively parse each item + parsed = BaseQuery._parse_sort_spec(item) + result.extend(parsed) + + else: + raise TypeError( + f"sort_by must be a string, tuple, or list, got {type(sort_spec)}" + ) + + return result + + def sort_by( + self, sort_spec: Optional[SortSpec] = None, asc: bool = True + ) -> "BaseQuery": + """Set the sort order for query results. + + This method supports sorting by single or multiple fields. Note that Redis Search + natively supports only a single SORTBY field. When multiple fields are specified, + only the FIRST field is used for the Redis SORTBY clause. + + Args: + sort_spec: Sort specification in various formats: + - str: single field name + - Tuple[str, str]: (field_name, "ASC"|"DESC") + - List: list of field names or tuples + asc: Default sort direction when not specified (only used when sort_spec is a string). + Defaults to True (ascending). + + Returns: + self: Returns the query object for method chaining. + + Raises: + TypeError: If sort_spec is not a valid type. + ValueError: If direction is not "ASC" or "DESC". + + Examples: + >>> query.sort_by("price") # Single field, ascending + >>> query.sort_by(("price", "DESC")) # Single field, descending + >>> query.sort_by(["price", "rating"]) # Multiple fields (only first used) + >>> query.sort_by([("price", "DESC"), ("rating", "ASC")]) + + Note: + When multiple fields are specified, only the first field is used for sorting + in Redis. Future versions may support multi-field sorting through post-query + sorting in Python. + """ + if sort_spec is None or sort_spec == []: + # No sorting + self._sortby = None + return self + + # Handle backward compatibility: if sort_spec is a string and asc is specified + # treat it as the old (field, asc) format + parsed: List[Tuple[str, bool]] + if isinstance(sort_spec, str) and asc is not True: + # Old API: query.sort_by("field", asc=False) + parsed = [(sort_spec, asc)] + else: + # New API: parse the sort_spec + parsed = self._parse_sort_spec(sort_spec) + + if not parsed: + self._sortby = None + return self + + # Use the first field for Redis SORTBY + first_field, first_asc = parsed[0] + + # Log warning if multiple fields specified + if len(parsed) > 1: + logger.warning( + f"Multiple sort fields specified: {[f[0] for f in parsed]}. " + f"Redis Search only supports single-field sorting. Using first field: '{first_field}'. " + "Additional fields are ignored." + ) + + # Call parent's sort_by with the first field + super().sort_by(first_field, asc=first_asc) + + return self + def set_filter( self, filter_expression: Optional[Union[str, FilterExpression]] = None ): @@ -170,7 +318,7 @@ def __init__( return_fields: Optional[List[str]] = None, num_results: int = 10, dialect: int = 2, - sort_by: Optional[str] = None, + sort_by: Optional[SortSpec] = None, in_order: bool = False, params: Optional[Dict[str, Any]] = None, ): @@ -182,7 +330,12 @@ def __init__( return_fields (Optional[List[str]], optional): The fields to return. num_results (Optional[int], optional): The number of results to return. Defaults to 10. dialect (int, optional): The query dialect. Defaults to 2. - sort_by (Optional[str], optional): The field to order the results by. Defaults to None. + sort_by (Optional[SortSpec], optional): The field(s) to order the results by. Can be: + - str: single field name (e.g., "price") + - Tuple[str, str]: (field_name, "ASC"|"DESC") (e.g., ("price", "DESC")) + - List: list of fields or tuples (e.g., ["price", ("rating", "DESC")]) + Note: Redis Search only supports single-field sorting, so only the first field is used. + Defaults to None. in_order (bool, optional): Requires the terms in the field to have the same order as the terms in the query filter. Defaults to False. params (Optional[Dict[str, Any]], optional): The parameters for the query. Defaults to None. @@ -292,7 +445,7 @@ def __init__( num_results: int = 10, return_score: bool = True, dialect: int = 2, - sort_by: Optional[str] = None, + sort_by: Optional[SortSpec] = None, in_order: bool = False, hybrid_policy: Optional[str] = None, batch_size: Optional[int] = None, @@ -319,8 +472,12 @@ def __init__( distance. Defaults to True. dialect (int, optional): The RediSearch query dialect. Defaults to 2. - sort_by (Optional[str]): The field to order the results by. Defaults - to None. Results will be ordered by vector distance. + sort_by (Optional[SortSpec]): The field(s) to order the results by. Can be: + - str: single field name + - Tuple[str, str]: (field_name, "ASC"|"DESC") + - List: list of fields or tuples + Note: Only the first field is used for Redis sorting. + Defaults to None. Results will be ordered by vector distance. in_order (bool): Requires the terms in the field to have the same order as the terms in the query filter, regardless of the offsets between them. Defaults to False. @@ -543,7 +700,7 @@ def __init__( num_results: int = 10, return_score: bool = True, dialect: int = 2, - sort_by: Optional[str] = None, + sort_by: Optional[SortSpec] = None, in_order: bool = False, hybrid_policy: Optional[str] = None, batch_size: Optional[int] = None, @@ -576,8 +733,12 @@ def __init__( distance. Defaults to True. dialect (int, optional): The RediSearch query dialect. Defaults to 2. - sort_by (Optional[str]): The field to order the results by. Defaults - to None. Results will be ordered by vector distance. + sort_by (Optional[SortSpec]): The field(s) to order the results by. Can be: + - str: single field name + - Tuple[str, str]: (field_name, "ASC"|"DESC") + - List: list of fields or tuples + Note: Only the first field is used for Redis sorting. + Defaults to None. Results will be ordered by vector distance. in_order (bool): Requires the terms in the field to have the same order as the terms in the query filter, regardless of the offsets between them. Defaults to False. @@ -863,7 +1024,7 @@ def __init__( num_results: int = 10, return_score: bool = True, dialect: int = 2, - sort_by: Optional[str] = None, + sort_by: Optional[SortSpec] = None, in_order: bool = False, params: Optional[Dict[str, Any]] = None, stopwords: Optional[Union[str, Set[str]]] = "english", @@ -887,8 +1048,12 @@ def __init__( Defaults to True. dialect (int, optional): The RediSearch query dialect. Defaults to 2. - sort_by (Optional[str]): The field to order the results by. Defaults - to None. Results will be ordered by text score. + sort_by (Optional[SortSpec]): The field(s) to order the results by. Can be: + - str: single field name + - Tuple[str, str]: (field_name, "ASC"|"DESC") + - List: list of fields or tuples + Note: Only the first field is used for Redis sorting. + Defaults to None. Results will be ordered by text score. in_order (bool): Requires the terms in the field to have the same order as the terms in the query filter, regardless of the offsets between them. Defaults to False. diff --git a/tests/unit/test_multi_field_sorting.py b/tests/unit/test_multi_field_sorting.py new file mode 100644 index 00000000..ab077043 --- /dev/null +++ b/tests/unit/test_multi_field_sorting.py @@ -0,0 +1,199 @@ +"""Unit tests for multiple field sorting functionality (issue #373).""" + +import pytest + +from redisvl.query import CountQuery, FilterQuery, TextQuery, VectorQuery +from redisvl.query.filter import Tag + + +class TestMultiFieldSortingFilterQuery: + """Test multiple field sorting in FilterQuery.""" + + def test_sort_by_single_field_string_backward_compat(self): + """Test backward compatibility with single string sort_by.""" + query = FilterQuery(sort_by="price") + + # Should work as before - single field, default ASC + assert query._sortby is not None + # The _sortby object should have the field set + assert "price" in str(query._sortby.args) + + def test_sort_by_single_field_with_direction(self): + """Test single field with explicit direction as tuple.""" + query = FilterQuery(sort_by=("price", "DESC")) + + assert query._sortby is not None + args_str = str(query._sortby.args) + assert "price" in args_str + assert "DESC" in args_str + + def test_sort_by_multiple_fields_list_of_strings(self): + """Test multiple fields as list of strings (all ASC by default). + + Note: Only the first field is actually used in Redis sorting. + """ + query = FilterQuery(sort_by=["price", "rating", "age"]) + + assert query._sortby is not None + # Only the first field should be in the sortby + args_str = str(query._sortby.args) + assert "price" in args_str + assert "ASC" in args_str + # Additional fields are not included in Redis SORTBY + assert "rating" not in args_str + assert "age" not in args_str + + def test_sort_by_multiple_fields_list_of_tuples(self): + """Test multiple fields as list of tuples with directions. + + Note: Only the first field is actually used in Redis sorting. + """ + query = FilterQuery( + sort_by=[("price", "DESC"), ("rating", "ASC"), ("age", "DESC")] + ) + + assert query._sortby is not None + args_str = str(query._sortby.args) + # Only the first field should be in the sortby + assert "price" in args_str + assert "DESC" in args_str + # Additional fields are not included + assert "rating" not in args_str + assert "age" not in args_str + + def test_sort_by_multiple_fields_mixed_formats(self): + """Test multiple fields with mixed formats (string and tuple). + + Note: Only the first field is actually used in Redis sorting. + """ + query = FilterQuery(sort_by=["price", ("rating", "DESC")]) + + assert query._sortby is not None + args_str = str(query._sortby.args) + # Only the first field (price with default ASC) should be in sortby + assert "price" in args_str + assert "ASC" in args_str + # Second field is not included + assert "rating" not in args_str + + def test_sort_by_invalid_direction(self): + """Test that invalid sort direction raises ValueError.""" + with pytest.raises(ValueError, match="Sort direction must be 'ASC' or 'DESC'"): + FilterQuery(sort_by=[("price", "INVALID")]) + + def test_sort_by_invalid_type(self): + """Test that invalid sort_by type raises TypeError.""" + with pytest.raises(TypeError): + FilterQuery(sort_by=123) + + def test_sort_by_empty_list(self): + """Test that empty list is handled gracefully.""" + query = FilterQuery(sort_by=[]) + # Should not set sortby + assert query._sortby is None + + def test_sort_by_none(self): + """Test that None is handled gracefully (no sorting).""" + query = FilterQuery(sort_by=None) + assert query._sortby is None + + +class TestMultiFieldSortingVectorQuery: + """Test multiple field sorting in VectorQuery.""" + + def test_vector_query_default_sorts_by_distance(self): + """Test that VectorQuery defaults to sorting by vector_distance.""" + query = VectorQuery(vector=[0.1, 0.2, 0.3], vector_field_name="embedding") + + assert query._sortby is not None + assert "vector_distance" in str(query._sortby.args) + + def test_vector_query_custom_sort_single_field(self): + """Test VectorQuery with custom single field sort.""" + query = VectorQuery( + vector=[0.1, 0.2, 0.3], vector_field_name="embedding", sort_by="price" + ) + + assert query._sortby is not None + assert "price" in str(query._sortby.args) + + def test_vector_query_custom_sort_multiple_fields(self): + """Test VectorQuery with multiple field sort. + + Note: Only the first field is actually used in Redis sorting. + """ + query = VectorQuery( + vector=[0.1, 0.2, 0.3], + vector_field_name="embedding", + sort_by=[("price", "DESC"), "rating"], + ) + + assert query._sortby is not None + args_str = str(query._sortby.args) + # Only the first field should be used + assert "price" in args_str + assert "DESC" in args_str + assert "rating" not in args_str + + +class TestMultiFieldSortingTextQuery: + """Test multiple field sorting in TextQuery.""" + + def test_text_query_single_field_sort(self): + """Test TextQuery with single field sort.""" + query = TextQuery( + text="example", text_field_name="content", sort_by="timestamp" + ) + + assert query._sortby is not None + assert "timestamp" in str(query._sortby.args) + + def test_text_query_multiple_field_sort(self): + """Test TextQuery with multiple field sort. + + Note: Only the first field is actually used in Redis sorting. + """ + query = TextQuery( + text="example", + text_field_name="content", + sort_by=[("relevance", "DESC"), ("timestamp", "ASC")], + ) + + assert query._sortby is not None + args_str = str(query._sortby.args) + # Only the first field should be used + assert "relevance" in args_str + assert "DESC" in args_str + assert "timestamp" not in args_str + + +class TestSortByMethodChaining: + """Test sort_by method can be called after initialization.""" + + def test_filter_query_sort_by_chaining(self): + """Test calling sort_by() method after FilterQuery initialization. + + Note: Only the first field is actually used in Redis sorting. + """ + query = FilterQuery() + query.sort_by([("price", "DESC"), "rating"]) + + assert query._sortby is not None + args_str = str(query._sortby.args) + # Only the first field should be used + assert "price" in args_str + assert "DESC" in args_str + assert "rating" not in args_str + + def test_sort_by_replacement(self): + """Test that calling sort_by() replaces previous sort.""" + query = FilterQuery(sort_by="price") + + # Replace with new sort + query.sort_by([("rating", "DESC")]) + + # Should only have rating now (price replaced) + args_str = str(query._sortby.args) + assert "rating" in args_str + assert "DESC" in args_str + assert "price" not in args_str From 122c7ee9b2afe3896670be34d405a5248d70dfc9 Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Mon, 29 Sep 2025 21:47:41 -0700 Subject: [PATCH 2/5] test: add comprehensive integration tests for multi-field sorting (#373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added integration tests using TestContainers to verify multiple field sorting behavior with real Redis Search instances. Tests cover: - FilterQuery: single/multiple field sorting with various formats - VectorQuery: custom sorting vs default vector distance sorting - TextQuery: sorting text search results by custom fields - Edge cases: invalid inputs, empty lists, None values - Backward compatibility: existing sort_by usage patterns - Warning verification: logs when multiple fields specified All 17 integration tests validate that: 1. Single field sorting works in ASC/DESC modes 2. Multiple field specifications are accepted but only first field used 3. Warnings are logged when multiple fields provided 4. Filter expressions work correctly with sorting 5. Old-style API calls remain functional Test data: 8 product records with varied price, rating, and stock values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../test_multi_field_sorting_integration.py | 408 ++++++++++++++++++ 1 file changed, 408 insertions(+) create mode 100644 tests/integration/test_multi_field_sorting_integration.py diff --git a/tests/integration/test_multi_field_sorting_integration.py b/tests/integration/test_multi_field_sorting_integration.py new file mode 100644 index 00000000..ae614ba7 --- /dev/null +++ b/tests/integration/test_multi_field_sorting_integration.py @@ -0,0 +1,408 @@ +"""Integration tests for multiple field sorting feature (issue #373).""" + +import pytest + +from redisvl.index import SearchIndex +from redisvl.query import FilterQuery, TextQuery, VectorQuery +from redisvl.redis.utils import array_to_buffer +from redisvl.schema import IndexSchema + + +@pytest.fixture +def products_schema(): + """Create a schema for product data with multiple sortable fields.""" + return IndexSchema.from_dict( + { + "index": { + "name": "products_test", + "prefix": "product:", + "storage_type": "hash", + }, + "fields": [ + {"name": "name", "type": "text"}, + {"name": "category", "type": "tag"}, + {"name": "price", "type": "numeric"}, + {"name": "rating", "type": "numeric"}, + {"name": "stock", "type": "numeric"}, + {"name": "brand", "type": "tag"}, + { + "name": "embedding", + "type": "vector", + "attrs": { + "dims": 3, + "algorithm": "flat", + "distance_metric": "cosine", + }, + }, + ], + } + ) + + +@pytest.fixture +def products_index(products_schema, client): + """Create and populate a products index with test data.""" + index = SearchIndex(schema=products_schema, redis_client=client) + index.create(overwrite=True, drop=True) + + # Load test product data with varied values for sorting + products = [ + { + "name": "Laptop Pro", + "category": "electronics", + "price": 1299.99, + "rating": 4.5, + "stock": 15, + "brand": "TechCorp", + "embedding": [0.1, 0.2, 0.3], + }, + { + "name": "Wireless Mouse", + "category": "electronics", + "price": 29.99, + "rating": 4.8, + "stock": 100, + "brand": "TechCorp", + "embedding": [0.2, 0.3, 0.4], + }, + { + "name": "USB Cable", + "category": "electronics", + "price": 9.99, + "rating": 4.2, + "stock": 250, + "brand": "GenericBrand", + "embedding": [0.3, 0.4, 0.5], + }, + { + "name": "Gaming Keyboard", + "category": "electronics", + "price": 149.99, + "rating": 4.9, + "stock": 45, + "brand": "TechCorp", + "embedding": [0.1, 0.3, 0.5], + }, + { + "name": "Monitor 27inch", + "category": "electronics", + "price": 349.99, + "rating": 4.6, + "stock": 30, + "brand": "DisplayCo", + "embedding": [0.2, 0.2, 0.4], + }, + { + "name": "Desk Chair", + "category": "furniture", + "price": 199.99, + "rating": 4.3, + "stock": 20, + "brand": "FurniturePlus", + "embedding": [0.5, 0.1, 0.2], + }, + { + "name": "Standing Desk", + "category": "furniture", + "price": 499.99, + "rating": 4.7, + "stock": 12, + "brand": "FurniturePlus", + "embedding": [0.4, 0.2, 0.1], + }, + { + "name": "Office Lamp", + "category": "furniture", + "price": 39.99, + "rating": 4.4, + "stock": 75, + "brand": "LightCo", + "embedding": [0.3, 0.3, 0.3], + }, + ] + + # Preprocess function to convert embeddings to bytes for HASH storage + def preprocess(item): + """Convert embedding list to bytes for HASH storage.""" + item_copy = item.copy() + item_copy["embedding"] = array_to_buffer(item["embedding"], "float32") + return item_copy + + # Insert products with preprocessing + for i, product in enumerate(products): + index.load([product], keys=[f"product:{i}"], preprocess=preprocess) + + yield index + + # Cleanup + index.delete(drop=True) + + +class TestMultiFieldSortingFilterQuery: + """Integration tests for FilterQuery with multiple field sorting.""" + + def test_single_field_sort_ascending(self, products_index): + """Test sorting by single field in ascending order.""" + query = FilterQuery(sort_by="price", num_results=10) + results = products_index.query(query) + + assert len(results) > 0 + # Verify results are sorted by price ascending + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices), "Results should be sorted by price ascending" + assert prices[0] == 9.99 # USB Cable (cheapest) + assert prices[-1] == 1299.99 # Laptop Pro (most expensive) + + def test_single_field_sort_descending_tuple(self, products_index): + """Test sorting by single field in descending order using tuple format.""" + query = FilterQuery(sort_by=("rating", "DESC"), num_results=10) + results = products_index.query(query) + + assert len(results) > 0 + # Verify results are sorted by rating descending + ratings = [float(doc["rating"]) for doc in results] + assert ratings == sorted( + ratings, reverse=True + ), "Results should be sorted by rating descending" + assert ratings[0] == 4.9 # Gaming Keyboard (highest rating) + + def test_multiple_fields_uses_first_only(self, products_index, caplog): + """Test that multiple fields logs warning and uses only first field.""" + import logging + + caplog.set_level(logging.WARNING) + + query = FilterQuery( + sort_by=[("price", "ASC"), ("rating", "DESC"), "stock"], num_results=10 + ) + results = products_index.query(query) + + # Check that warning was logged + assert any( + "Multiple sort fields specified" in record.message + for record in caplog.records + ), "Should log warning about multiple fields" + assert any( + "Using first field: 'price'" in record.message for record in caplog.records + ), "Should indicate which field is being used" + + # Verify only first field (price) is used for sorting + assert len(results) > 0 + prices = [float(doc["price"]) for doc in results] + assert prices == sorted( + prices + ), "Results should be sorted by price (first field)" + + def test_multiple_fields_mixed_format(self, products_index, caplog): + """Test multiple fields with mixed string and tuple format.""" + import logging + + caplog.set_level(logging.WARNING) + + query = FilterQuery(sort_by=["stock", ("price", "DESC")], num_results=10) + results = products_index.query(query) + + # Check warning + assert any( + "Multiple sort fields specified" in record.message + for record in caplog.records + ) + + # Verify first field (stock) is used - ascending order + assert len(results) > 0 + stock = [int(doc["stock"]) for doc in results] + assert stock == sorted(stock), "Results should be sorted by stock ascending" + + def test_sort_with_filter_expression(self, products_index): + """Test sorting combined with filter expression.""" + from redisvl.query.filter import Tag + + category_filter = Tag("category") == "electronics" + query = FilterQuery( + filter_expression=category_filter, sort_by=("price", "DESC"), num_results=10 + ) + results = products_index.query(query) + + # All results should be electronics + assert all(doc["category"] == "electronics" for doc in results) + + # Should be sorted by price descending + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices, reverse=True) + + +class TestMultiFieldSortingVectorQuery: + """Integration tests for VectorQuery with multiple field sorting.""" + + def test_vector_query_default_sort(self, products_index): + """Test that VectorQuery defaults to sorting by vector distance.""" + query = VectorQuery( + vector=[0.1, 0.2, 0.3], + vector_field_name="embedding", + num_results=5, + ) + results = products_index.query(query) + + assert len(results) > 0 + # Should be sorted by vector_distance (ascending) + distances = [float(doc["vector_distance"]) for doc in results] + assert distances == sorted( + distances + ), "Results should be sorted by vector distance" + + def test_vector_query_custom_sort_single_field(self, products_index): + """Test VectorQuery with custom single field sort.""" + query = VectorQuery( + vector=[0.1, 0.2, 0.3], + vector_field_name="embedding", + return_fields=["name", "price", "rating"], + sort_by=("price", "ASC"), + num_results=5, + ) + results = products_index.query(query) + + assert len(results) > 0 + # Should be sorted by price, not vector distance + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices), "Results should be sorted by price" + + def test_vector_query_multiple_fields_warning(self, products_index, caplog): + """Test that VectorQuery with multiple sort fields logs warning.""" + import logging + + caplog.set_level(logging.WARNING) + + query = VectorQuery( + vector=[0.1, 0.2, 0.3], + vector_field_name="embedding", + return_fields=["name", "price", "rating"], + sort_by=[("rating", "DESC"), "price"], + num_results=5, + ) + results = products_index.query(query) + + # Check warning + assert any( + "Multiple sort fields specified" in record.message + for record in caplog.records + ) + assert any( + "Using first field: 'rating'" in record.message for record in caplog.records + ) + + # Verify first field (rating) is used + assert len(results) > 0 + ratings = [float(doc["rating"]) for doc in results] + assert ratings == sorted(ratings, reverse=True) + + +class TestMultiFieldSortingTextQuery: + """Integration tests for TextQuery with multiple field sorting.""" + + def test_text_query_with_custom_sort(self, products_index): + """Test TextQuery with custom sort field.""" + query = TextQuery( + text="tech", + text_field_name="name", + return_fields=["name", "price", "rating"], + sort_by=("price", "DESC"), + num_results=10, + ) + results = products_index.query(query) + + # Results should contain "tech" in the name and be sorted by price descending + if len(results) > 0: + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices, reverse=True) + + def test_text_query_multiple_fields(self, products_index, caplog): + """Test TextQuery with multiple sort fields.""" + import logging + + caplog.set_level(logging.WARNING) + + query = TextQuery( + text="desk", + text_field_name="name", + return_fields=["name", "price", "rating"], + sort_by=[("price", "ASC"), ("rating", "DESC")], + num_results=10, + ) + results = products_index.query(query) + + # Check warning + assert any( + "Multiple sort fields specified" in record.message + for record in caplog.records + ) + + # Should use first field (price ASC) + if len(results) > 0: + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices) + + +class TestSortingEdgeCases: + """Test edge cases and error handling for sorting.""" + + def test_invalid_sort_direction(self): + """Test that invalid sort direction raises ValueError.""" + with pytest.raises(ValueError, match="Sort direction must be 'ASC' or 'DESC'"): + FilterQuery(sort_by=[("price", "INVALID")]) + + def test_invalid_sort_type(self): + """Test that invalid sort type raises TypeError.""" + with pytest.raises(TypeError): + FilterQuery(sort_by=123) + + def test_empty_sort_list(self, products_index): + """Test that empty sort list is handled gracefully.""" + query = FilterQuery(sort_by=[], num_results=10) + results = products_index.query(query) + + # Should work, just no specific sort order (Redis default) + assert len(results) > 0 + + def test_sort_by_none(self, products_index): + """Test that sort_by=None is handled gracefully.""" + query = FilterQuery(sort_by=None, num_results=10) + results = products_index.query(query) + + # Should work, just no specific sort order + assert len(results) > 0 + + +class TestBackwardCompatibility: + """Test backward compatibility with existing sort_by usage.""" + + def test_old_style_sort_by_field_only(self, products_index): + """Test old style sort_by with just field name (backward compatible).""" + query = FilterQuery(sort_by="price", num_results=10) + results = products_index.query(query) + + assert len(results) > 0 + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices) + + def test_old_style_sort_by_method_with_asc_param(self, products_index): + """Test old style query.sort_by(field, asc=False) still works.""" + query = FilterQuery() + query.sort_by("price", asc=False) + + results = products_index.query(query) + + assert len(results) > 0 + prices = [float(doc["price"]) for doc in results] + assert prices == sorted(prices, reverse=True) + + def test_method_chaining_replaces_sort(self, products_index): + """Test that calling sort_by() again replaces previous sort.""" + query = FilterQuery(sort_by="price") + query.sort_by(("rating", "DESC")) + + results = products_index.query(query) + + # Should be sorted by rating (the replacement), not price + assert len(results) > 0 + ratings = [float(doc["rating"]) for doc in results] + assert ratings == sorted(ratings, reverse=True) From 05be04a1edbe80392da40b3574b54eae5e2be5e1 Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Tue, 30 Sep 2025 09:51:57 -0700 Subject: [PATCH 3/5] fix: skip TextQuery tests on Redis < 7.0.0 (BM25STD not available) BM25STD scorer was introduced in Redis 7.0.0. These tests will be skipped on Redis 6.2.6-v9 with an informative message. --- .../integration/test_multi_field_sorting_integration.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/integration/test_multi_field_sorting_integration.py b/tests/integration/test_multi_field_sorting_integration.py index ae614ba7..286c6c50 100644 --- a/tests/integration/test_multi_field_sorting_integration.py +++ b/tests/integration/test_multi_field_sorting_integration.py @@ -6,6 +6,7 @@ from redisvl.query import FilterQuery, TextQuery, VectorQuery from redisvl.redis.utils import array_to_buffer from redisvl.schema import IndexSchema +from tests.conftest import skip_if_redis_version_below @pytest.fixture @@ -301,6 +302,10 @@ class TestMultiFieldSortingTextQuery: def test_text_query_with_custom_sort(self, products_index): """Test TextQuery with custom sort field.""" + skip_if_redis_version_below( + products_index.client, "7.0.0", "BM25STD scorer requires Redis 7.0+" + ) + query = TextQuery( text="tech", text_field_name="name", @@ -317,6 +322,10 @@ def test_text_query_with_custom_sort(self, products_index): def test_text_query_multiple_fields(self, products_index, caplog): """Test TextQuery with multiple sort fields.""" + skip_if_redis_version_below( + products_index.client, "7.0.0", "BM25STD scorer requires Redis 7.0+" + ) + import logging caplog.set_level(logging.WARNING) From 4543fa3917e94106e9cb1206199d632064572073 Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Tue, 30 Sep 2025 10:36:55 -0700 Subject: [PATCH 4/5] docs: align test comment with implementation warning message Update comment to use consistent terminology: 'Redis Search only supports single-field sorting' instead of 'Only the first field is actually used' for clarity and consistency with runtime warnings. --- tests/unit/test_multi_field_sorting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_multi_field_sorting.py b/tests/unit/test_multi_field_sorting.py index ab077043..795e410c 100644 --- a/tests/unit/test_multi_field_sorting.py +++ b/tests/unit/test_multi_field_sorting.py @@ -30,7 +30,7 @@ def test_sort_by_single_field_with_direction(self): def test_sort_by_multiple_fields_list_of_strings(self): """Test multiple fields as list of strings (all ASC by default). - Note: Only the first field is actually used in Redis sorting. + Note: Redis Search only supports single-field sorting. Only the first field is used. """ query = FilterQuery(sort_by=["price", "rating", "age"]) From 215fb699ff0a50dbf08f1c571f96c2c7786b9463 Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Tue, 30 Sep 2025 10:38:47 -0700 Subject: [PATCH 5/5] refactor: extract caplog assertion helper to reduce duplication Add assert_warning_logged helper function to reduce code duplication in warning assertion patterns across multiple test methods. --- .../test_multi_field_sorting_integration.py | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/tests/integration/test_multi_field_sorting_integration.py b/tests/integration/test_multi_field_sorting_integration.py index 286c6c50..a6bcde1f 100644 --- a/tests/integration/test_multi_field_sorting_integration.py +++ b/tests/integration/test_multi_field_sorting_integration.py @@ -9,6 +9,18 @@ from tests.conftest import skip_if_redis_version_below +def assert_warning_logged(caplog, message_fragment): + """Helper to assert that a warning containing the message fragment was logged. + + Args: + caplog: pytest caplog fixture + message_fragment: String fragment to search for in log messages + """ + assert any( + message_fragment in record.message for record in caplog.records + ), f"Expected warning containing '{message_fragment}' to be logged" + + @pytest.fixture def products_schema(): """Create a schema for product data with multiple sortable fields.""" @@ -179,13 +191,8 @@ def test_multiple_fields_uses_first_only(self, products_index, caplog): results = products_index.query(query) # Check that warning was logged - assert any( - "Multiple sort fields specified" in record.message - for record in caplog.records - ), "Should log warning about multiple fields" - assert any( - "Using first field: 'price'" in record.message for record in caplog.records - ), "Should indicate which field is being used" + assert_warning_logged(caplog, "Multiple sort fields specified") + assert_warning_logged(caplog, "Using first field: 'price'") # Verify only first field (price) is used for sorting assert len(results) > 0 @@ -204,10 +211,7 @@ def test_multiple_fields_mixed_format(self, products_index, caplog): results = products_index.query(query) # Check warning - assert any( - "Multiple sort fields specified" in record.message - for record in caplog.records - ) + assert_warning_logged(caplog, "Multiple sort fields specified") # Verify first field (stock) is used - ascending order assert len(results) > 0 @@ -283,13 +287,8 @@ def test_vector_query_multiple_fields_warning(self, products_index, caplog): results = products_index.query(query) # Check warning - assert any( - "Multiple sort fields specified" in record.message - for record in caplog.records - ) - assert any( - "Using first field: 'rating'" in record.message for record in caplog.records - ) + assert_warning_logged(caplog, "Multiple sort fields specified") + assert_warning_logged(caplog, "Using first field: 'rating'") # Verify first field (rating) is used assert len(results) > 0 @@ -340,10 +339,7 @@ def test_text_query_multiple_fields(self, products_index, caplog): results = products_index.query(query) # Check warning - assert any( - "Multiple sort fields specified" in record.message - for record in caplog.records - ) + assert_warning_logged(caplog, "Multiple sort fields specified") # Should use first field (price ASC) if len(results) > 0: