From bd084c03085649fadd61ad6b264262ec5bc63d13 Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 16:12:41 -0500 Subject: [PATCH 1/9] fix: (schema) ensure field modifiers follow canonical order for RediSearch parser Fix field modifier ordering to satisfy RediSearch parser requirements where INDEXEMPTY and INDEXMISSING must appear BEFORE SORTABLE in field definitions. This resolves index creation failures when using index_missing=True with sortable=True. --- redisvl/schema/fields.py | 97 +++-- ...est_field_modifier_ordering_integration.py | 396 ++++++++++++++++++ tests/unit/test_field_modifier_ordering.py | 382 +++++++++++++++++ 3 files changed, 851 insertions(+), 24 deletions(-) create mode 100644 tests/integration/test_field_modifier_ordering_integration.py create mode 100644 tests/unit/test_field_modifier_ordering.py diff --git a/redisvl/schema/fields.py b/redisvl/schema/fields.py index 3a3f2206..ad97362e 100644 --- a/redisvl/schema/fields.py +++ b/redisvl/schema/fields.py @@ -35,7 +35,7 @@ """ from enum import Enum -from typing import Any, Dict, Literal, Optional, Tuple, Type, Union +from typing import Any, Dict, List, Literal, Optional, Tuple, Type, Union from pydantic import BaseModel, Field, field_validator, model_validator from redis.commands.search.field import Field as RedisField @@ -97,6 +97,51 @@ class CompressionType(str, Enum): LeanVec8x8 = "LeanVec8x8" +### Helper Functions ### + + +def _normalize_field_modifiers( + field: RedisField, canonical_order: List[str], want_unf: bool = False +) -> None: + """Normalize field modifier ordering for RediSearch parser. + + RediSearch has a parser limitation (redis/redis#5177) where INDEXEMPTY and + INDEXMISSING must appear BEFORE SORTABLE in field definitions. This function + reorders field.args_suffix to match the canonical order while preserving + unknown modifiers at the start. + + Args: + field: Redis field object whose args_suffix will be normalized + canonical_order: List of modifiers in desired canonical order + want_unf: Whether UNF should be added after SORTABLE (default: False) + + Time Complexity: O(n + m) where n = len(field.args_suffix), m = len(canonical_order) + Space Complexity: O(n + m) + + Example: + >>> field = RedisTextField("title") + >>> field.args_suffix = ["SORTABLE", "INDEXMISSING"] + >>> _normalize_field_modifiers(field, ["INDEXEMPTY", "INDEXMISSING", "SORTABLE"]) + >>> field.args_suffix + ['INDEXMISSING', 'SORTABLE'] + """ + suffix_set = set(field.args_suffix) + known_set = set(canonical_order) + + # Preserve unknown modifiers in original order + new_suffix = [t for t in field.args_suffix if t not in known_set] + + # Add known modifiers in canonical order + for modifier in canonical_order: + if modifier in suffix_set: + new_suffix.append(modifier) + # Special case: UNF only appears with SORTABLE + if modifier == "SORTABLE" and want_unf and "UNF" not in suffix_set: + new_suffix.append("UNF") + + field.args_suffix = new_suffix + + ### Field Attributes ### @@ -290,7 +335,7 @@ def validate_svs_params(self): ): logger.warning( f"LeanVec compression selected without 'reduce'. " - f"Consider setting reduce={self.dims//2} for better performance" + f"Consider setting reduce={self.dims // 2} for better performance" ) if self.graph_max_degree and self.graph_max_degree < 32: @@ -371,16 +416,11 @@ def as_redis_field(self) -> RedisField: field = RedisTextField(name, **kwargs) - # Add UNF support (only when sortable) - # UNF must come before NOINDEX in the args_suffix - if self.attrs.unf and self.attrs.sortable: # type: ignore - if "NOINDEX" in field.args_suffix: - # Insert UNF before NOINDEX - noindex_idx = field.args_suffix.index("NOINDEX") - field.args_suffix.insert(noindex_idx, "UNF") - else: - # No NOINDEX, append normally - field.args_suffix.append("UNF") + # Normalize suffix ordering to satisfy RediSearch parser expectations. + # Canonical order: [INDEXEMPTY] [INDEXMISSING] [SORTABLE [UNF]] [NOINDEX] + canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "UNF", "NOINDEX"] + want_unf = self.attrs.unf and self.attrs.sortable # type: ignore + _normalize_field_modifiers(field, canonical_order, want_unf) return field @@ -416,7 +456,14 @@ def as_redis_field(self) -> RedisField: if self.attrs.no_index: # type: ignore kwargs["no_index"] = True - return RedisTagField(name, **kwargs) + field = RedisTagField(name, **kwargs) + + # Normalize suffix ordering to satisfy RediSearch parser expectations. + # Canonical order: [INDEXEMPTY] [INDEXMISSING] [SORTABLE] [NOINDEX] + canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "NOINDEX"] + _normalize_field_modifiers(field, canonical_order) + + return field class NumericField(BaseField): @@ -446,16 +493,11 @@ def as_redis_field(self) -> RedisField: field = RedisNumericField(name, **kwargs) - # Add UNF support (only when sortable) - # UNF must come before NOINDEX in the args_suffix - if self.attrs.unf and self.attrs.sortable: # type: ignore - if "NOINDEX" in field.args_suffix: - # Insert UNF before NOINDEX - noindex_idx = field.args_suffix.index("NOINDEX") - field.args_suffix.insert(noindex_idx, "UNF") - else: - # No NOINDEX, append normally - field.args_suffix.append("UNF") + # Normalize suffix ordering to satisfy RediSearch parser expectations. + # Canonical order: [INDEXEMPTY] [INDEXMISSING] [SORTABLE [UNF]] [NOINDEX] + canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "UNF", "NOINDEX"] + want_unf = self.attrs.unf and self.attrs.sortable # type: ignore + _normalize_field_modifiers(field, canonical_order, want_unf) return field @@ -485,7 +527,14 @@ def as_redis_field(self) -> RedisField: if self.attrs.no_index: # type: ignore kwargs["no_index"] = True - return RedisGeoField(name, **kwargs) + field = RedisGeoField(name, **kwargs) + + # Normalize suffix ordering to satisfy RediSearch parser expectations. + # Canonical order: [INDEXEMPTY] [INDEXMISSING] [SORTABLE] [NOINDEX] + canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "NOINDEX"] + _normalize_field_modifiers(field, canonical_order) + + return field class FlatVectorField(BaseField): diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py new file mode 100644 index 00000000..c75eea17 --- /dev/null +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -0,0 +1,396 @@ +""" +Integration tests for field modifier ordering against live Redis. + +These tests verify that indices can be created successfully with various +field modifier combinations and that the modifiers are correctly set in Redis. +""" + +import pytest +import redis + +from redisvl.index import SearchIndex +from redisvl.schema import IndexSchema + + +@pytest.fixture +def redis_client(): + """Fixture to provide a Redis client.""" + client = redis.Redis(host="localhost", port=6379, decode_responses=True) + yield client + # Cleanup is handled by individual tests + + +class TestTextFieldModifierOrderingIntegration: + """Integration tests for TextField modifier ordering.""" + + def test_textfield_sortable_and_index_missing(self, redis_client): + """Test TextField with sortable and index_missing creates successfully.""" + schema_dict = { + "index": { + "name": "test_text_sortable_missing", + "prefix": "text_sm", + "storage_type": "hash", + }, + "fields": [ + { + "name": "title", + "type": "text", + "attrs": {"sortable": True, "index_missing": True}, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "test_text_sortable_missing") + assert info is not None + + # Verify field attributes contain both SORTABLE and INDEXMISSING + attrs = info[7][0] # Get field attributes + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + finally: + index.delete(drop=True) + + def test_textfield_all_modifiers(self, redis_client): + """Test TextField with all modifiers.""" + schema_dict = { + "index": { + "name": "test_text_all_mods", + "prefix": "text_all", + "storage_type": "hash", + }, + "fields": [ + { + "name": "content", + "type": "text", + "attrs": { + "index_empty": True, + "index_missing": True, + "sortable": True, + "unf": True, + }, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "test_text_all_mods") + assert info is not None + + # Verify all modifiers are present + attrs = info[7][0] + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + assert "INDEXEMPTY" in attrs + assert "UNF" in attrs + finally: + index.delete(drop=True) + + +class TestTagFieldModifierOrderingIntegration: + """Integration tests for TagField modifier ordering.""" + + def test_tagfield_sortable_and_index_missing(self, redis_client): + """Test TagField with sortable and index_missing creates successfully.""" + schema_dict = { + "index": { + "name": "test_tag_sortable_missing", + "prefix": "tag_sm", + "storage_type": "hash", + }, + "fields": [ + { + "name": "tags", + "type": "tag", + "attrs": {"sortable": True, "index_missing": True}, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "test_tag_sortable_missing") + assert info is not None + + # Verify field attributes contain both SORTABLE and INDEXMISSING + attrs = info[7][0] + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + finally: + index.delete(drop=True) + + def test_tagfield_all_modifiers(self, redis_client): + """Test TagField with all modifiers.""" + schema_dict = { + "index": { + "name": "test_tag_all_mods", + "prefix": "tag_all", + "storage_type": "hash", + }, + "fields": [ + { + "name": "categories", + "type": "tag", + "attrs": { + "index_empty": True, + "index_missing": True, + "sortable": True, + }, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "test_tag_all_mods") + assert info is not None + + # Verify all modifiers are present + attrs = info[7][0] + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + assert "INDEXEMPTY" in attrs + finally: + index.delete(drop=True) + + +class TestGeoFieldModifierOrderingIntegration: + """Integration tests for GeoField modifier ordering.""" + + def test_geofield_sortable_and_index_missing(self, redis_client): + """Test GeoField with sortable and index_missing creates successfully.""" + schema_dict = { + "index": { + "name": "test_geo_sortable_missing", + "prefix": "geo_sm", + "storage_type": "hash", + }, + "fields": [ + { + "name": "location", + "type": "geo", + "attrs": {"sortable": True, "index_missing": True}, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "test_geo_sortable_missing") + assert info is not None + + # Verify field attributes contain both SORTABLE and INDEXMISSING + attrs = info[7][0] + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + finally: + index.delete(drop=True) + + +class TestNumericFieldModifierOrderingIntegration: + """Integration tests for NumericField modifier ordering.""" + + def test_numericfield_sortable_and_index_missing(self, redis_client): + """Test NumericField with sortable and index_missing creates successfully.""" + schema_dict = { + "index": { + "name": "test_numeric_sortable_missing", + "prefix": "num_sm", + "storage_type": "hash", + }, + "fields": [ + { + "name": "price", + "type": "numeric", + "attrs": {"sortable": True, "index_missing": True}, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command( + "FT.INFO", "test_numeric_sortable_missing" + ) + assert info is not None + + # Verify field attributes contain both SORTABLE and INDEXMISSING + attrs = info[7][0] + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + finally: + index.delete(drop=True) + + +class TestMultiFieldModifierOrderingIntegration: + """Integration tests with multiple fields and modifiers.""" + + def test_mixed_field_types_with_modifiers(self, redis_client): + """Test index with multiple field types all using modifiers.""" + schema_dict = { + "index": { + "name": "test_mixed_fields", + "prefix": "mixed", + "storage_type": "hash", + }, + "fields": [ + { + "name": "title", + "type": "text", + "attrs": {"sortable": True, "index_missing": True}, + }, + { + "name": "tags", + "type": "tag", + "attrs": {"sortable": True, "index_missing": True}, + }, + { + "name": "price", + "type": "numeric", + "attrs": {"sortable": True, "index_missing": True}, + }, + { + "name": "location", + "type": "geo", + "attrs": {"sortable": True, "index_missing": True}, + }, + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "test_mixed_fields") + assert info is not None + + # Verify all fields were created + attrs_list = info[7] + assert len(attrs_list) == 4 + + # Verify each field has the correct modifiers + for attrs in attrs_list: + assert "SORTABLE" in attrs + assert "INDEXMISSING" in attrs + finally: + index.delete(drop=True) + + +class TestMLPCommandsScenarioIntegration: + """Integration tests for the exact scenario from mlp_commands.txt.""" + + def test_mlp_commands_index_creation(self, redis_client): + """Test creating index matching mlp_commands.txt scenario. + + This test verifies that an index with INDEXMISSING SORTABLE UNF + modifiers can be created successfully with correct field ordering. + """ + schema_dict = { + "index": { + "name": "testidx", + "prefix": "test:", + "storage_type": "hash", + }, + "fields": [ + { + "name": "work_experience_summary", + "type": "text", + "attrs": {"index_missing": True, "sortable": True, "unf": True}, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + # This should succeed with correct modifier ordering + index.create(overwrite=True) + + # Verify index was created + info = redis_client.execute_command("FT.INFO", "testidx") + assert info is not None + + # Verify field attributes + attrs = info[7][0] + assert "INDEXMISSING" in attrs + assert "SORTABLE" in attrs + assert "UNF" in attrs + finally: + index.delete(drop=True) + + def test_indexmissing_enables_ismissing_query(self, redis_client): + """Test that INDEXMISSING enables ismissing() query function.""" + schema_dict = { + "index": { + "name": "test_ismissing", + "prefix": "ismiss:", + "storage_type": "hash", + }, + "fields": [ + { + "name": "optional_field", + "type": "text", + "attrs": {"index_missing": True}, + } + ], + } + + schema = IndexSchema.from_dict(schema_dict) + index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + + try: + index.create(overwrite=True) + + # Create documents with and without the field + redis_client.hset("ismiss:1", "optional_field", "has value") + redis_client.hset("ismiss:2", "other_field", "no optional_field") + redis_client.hset("ismiss:3", "optional_field", "also has value") + + # Query for missing fields + result = redis_client.execute_command( + "FT.SEARCH", "test_ismissing", "ismissing(@optional_field)", "DIALECT", "2" + ) + + # Should return 1 result (ismiss:2) + assert result[0] == 1 + assert "ismiss:2" in str(result) + + finally: + redis_client.delete("ismiss:1", "ismiss:2", "ismiss:3") + index.delete(drop=True) diff --git a/tests/unit/test_field_modifier_ordering.py b/tests/unit/test_field_modifier_ordering.py new file mode 100644 index 00000000..59ebdbe5 --- /dev/null +++ b/tests/unit/test_field_modifier_ordering.py @@ -0,0 +1,382 @@ +""" +Unit tests for field modifier ordering fix. + +Tests verify that field modifiers are generated in the correct order +to satisfy RediSearch parser requirements. The canonical order is: + [INDEXEMPTY] [INDEXMISSING] [SORTABLE [UNF]] [NOINDEX] + +This is required because RediSearch has a parser limitation (redis/redis#5177) +where INDEXEMPTY/INDEXMISSING must appear BEFORE SORTABLE in field definitions. +""" + +import pytest +from redis.commands.search.field import TextField as RedisTextField + +from redisvl.schema.fields import ( + GeoField, + NumericField, + TagField, + TextField, + _normalize_field_modifiers, +) + + +class TestTextFieldModifierOrdering: + """Test TextField generates modifiers in correct order.""" + + def test_sortable_and_index_missing_order(self): + """Test that INDEXMISSING comes before SORTABLE.""" + field = TextField(name="title", attrs={"sortable": True, "index_missing": True}) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # INDEXMISSING must come before SORTABLE + assert "INDEXMISSING" in suffix + assert "SORTABLE" in suffix + assert suffix.index("INDEXMISSING") < suffix.index("SORTABLE") + + def test_all_modifiers_order(self): + """Test canonical order with all modifiers.""" + field = TextField( + name="content", + attrs={ + "index_empty": True, + "index_missing": True, + "sortable": True, + "unf": True, + "no_index": False, + }, + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # Verify canonical order: INDEXEMPTY, INDEXMISSING, SORTABLE, UNF + assert suffix == ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "UNF"] + + def test_unf_only_with_sortable(self): + """Test that UNF only appears when sortable=True.""" + field = TextField(name="title", attrs={"sortable": True, "unf": True}) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + assert "UNF" in suffix + assert "SORTABLE" in suffix + assert suffix.index("SORTABLE") < suffix.index("UNF") + + def test_unf_ignored_without_sortable(self): + """Test that UNF is ignored when sortable=False.""" + field = TextField(name="description", attrs={"sortable": False, "unf": True}) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + assert "UNF" not in suffix + assert "SORTABLE" not in suffix + + +class TestNumericFieldModifierOrdering: + """Test NumericField generates modifiers in correct order.""" + + def test_sortable_and_index_missing_order(self): + """Test that INDEXMISSING comes before SORTABLE.""" + field = NumericField( + name="price", attrs={"sortable": True, "index_missing": True} + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # INDEXMISSING must come before SORTABLE + assert "INDEXMISSING" in suffix + assert "SORTABLE" in suffix + assert suffix.index("INDEXMISSING") < suffix.index("SORTABLE") + + def test_all_modifiers_order(self): + """Test canonical order with all modifiers.""" + field = NumericField( + name="score", + attrs={ + "index_missing": True, + "sortable": True, + "unf": True, + "no_index": False, + }, + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # Verify canonical order: INDEXMISSING, SORTABLE, UNF + assert suffix == ["INDEXMISSING", "SORTABLE", "UNF"] + + def test_unf_only_with_sortable(self): + """Test that UNF only appears when sortable=True.""" + field = NumericField(name="rating", attrs={"sortable": True, "unf": True}) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + assert "UNF" in suffix + assert "SORTABLE" in suffix + assert suffix.index("SORTABLE") < suffix.index("UNF") + + +class TestTagFieldModifierOrdering: + """Test TagField generates modifiers in correct order.""" + + def test_sortable_and_index_missing_order(self): + """Test that INDEXMISSING comes before SORTABLE.""" + field = TagField(name="tags", attrs={"sortable": True, "index_missing": True}) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # INDEXMISSING must come before SORTABLE + assert "INDEXMISSING" in suffix + assert "SORTABLE" in suffix + assert suffix.index("INDEXMISSING") < suffix.index("SORTABLE") + + def test_all_modifiers_order(self): + """Test canonical order with all modifiers.""" + field = TagField( + name="categories", + attrs={ + "index_empty": True, + "index_missing": True, + "sortable": True, + "no_index": False, + }, + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # Verify canonical order: INDEXEMPTY, INDEXMISSING, SORTABLE + assert suffix == ["INDEXEMPTY", "INDEXMISSING", "SORTABLE"] + + def test_index_empty_before_index_missing(self): + """Test that INDEXEMPTY comes before INDEXMISSING.""" + field = TagField( + name="status", attrs={"index_empty": True, "index_missing": True} + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + assert "INDEXEMPTY" in suffix + assert "INDEXMISSING" in suffix + assert suffix.index("INDEXEMPTY") < suffix.index("INDEXMISSING") + + +class TestGeoFieldModifierOrdering: + """Test GeoField generates modifiers in correct order.""" + + def test_sortable_and_index_missing_order(self): + """Test that INDEXMISSING comes before SORTABLE.""" + field = GeoField( + name="location", attrs={"sortable": True, "index_missing": True} + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # INDEXMISSING must come before SORTABLE + assert "INDEXMISSING" in suffix + assert "SORTABLE" in suffix + assert suffix.index("INDEXMISSING") < suffix.index("SORTABLE") + + def test_all_modifiers_order(self): + """Test canonical order with all modifiers.""" + field = GeoField( + name="coordinates", + attrs={ + "index_missing": True, + "sortable": True, + "no_index": False, + }, + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # Verify canonical order: INDEXMISSING, SORTABLE + assert suffix == ["INDEXMISSING", "SORTABLE"] + + +class TestModifierOrderingConsistency: + """Test that all field types follow the same ordering rules.""" + + def test_all_fields_index_missing_before_sortable(self): + """Test that all field types put INDEXMISSING before SORTABLE.""" + fields = [ + TextField(name="text", attrs={"sortable": True, "index_missing": True}), + NumericField( + name="numeric", attrs={"sortable": True, "index_missing": True} + ), + TagField(name="tag", attrs={"sortable": True, "index_missing": True}), + GeoField(name="geo", attrs={"sortable": True, "index_missing": True}), + ] + + for field in fields: + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + assert ( + "INDEXMISSING" in suffix + ), f"{field.__class__.__name__} missing INDEXMISSING" + assert "SORTABLE" in suffix, f"{field.__class__.__name__} missing SORTABLE" + assert suffix.index("INDEXMISSING") < suffix.index( + "SORTABLE" + ), f"{field.__class__.__name__} has wrong order" + + def test_noindex_comes_last(self): + """Test that NOINDEX always comes last.""" + fields = [ + TextField(name="text", attrs={"sortable": True, "no_index": True}), + NumericField(name="numeric", attrs={"sortable": True, "no_index": True}), + TagField(name="tag", attrs={"sortable": True, "no_index": True}), + GeoField(name="geo", attrs={"sortable": True, "no_index": True}), + ] + + for field in fields: + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + if "NOINDEX" in suffix: + assert ( + suffix[-1] == "NOINDEX" + ), f"{field.__class__.__name__} NOINDEX not at end" + + +class TestNormalizeFieldModifiersHelper: + """Test the _normalize_field_modifiers helper function.""" + + def test_basic_reordering(self): + """Test basic reordering of INDEXMISSING and SORTABLE.""" + field = RedisTextField("test") + field.args_suffix = ["SORTABLE", "INDEXMISSING"] + canonical_order = ["INDEXMISSING", "SORTABLE"] + + _normalize_field_modifiers(field, canonical_order) + + assert field.args_suffix == ["INDEXMISSING", "SORTABLE"] + + def test_preserves_unknown_modifiers(self): + """Test that unknown modifiers are preserved at the start.""" + field = RedisTextField("test") + field.args_suffix = ["CUSTOM", "SORTABLE", "INDEXMISSING"] + canonical_order = ["INDEXMISSING", "SORTABLE"] + + _normalize_field_modifiers(field, canonical_order) + + assert field.args_suffix == ["CUSTOM", "INDEXMISSING", "SORTABLE"] + + def test_unf_added_with_sortable(self): + """Test that UNF is added when want_unf=True and SORTABLE is present.""" + field = RedisTextField("test") + field.args_suffix = ["SORTABLE"] + canonical_order = ["SORTABLE", "UNF"] + + _normalize_field_modifiers(field, canonical_order, want_unf=True) + + assert field.args_suffix == ["SORTABLE", "UNF"] + + def test_unf_not_duplicated(self): + """Test that UNF is not duplicated if already present.""" + field = RedisTextField("test") + field.args_suffix = ["SORTABLE", "UNF"] + canonical_order = ["SORTABLE", "UNF"] + + _normalize_field_modifiers(field, canonical_order, want_unf=True) + + assert field.args_suffix == ["SORTABLE", "UNF"] + + def test_unf_not_added_without_sortable(self): + """Test that UNF is not added if SORTABLE is not present.""" + field = RedisTextField("test") + field.args_suffix = ["INDEXMISSING"] + canonical_order = ["INDEXMISSING", "SORTABLE", "UNF"] + + _normalize_field_modifiers(field, canonical_order, want_unf=True) + + assert field.args_suffix == ["INDEXMISSING"] + + def test_all_modifiers_canonical_order(self): + """Test canonical order with all modifiers.""" + field = RedisTextField("test") + field.args_suffix = ["NOINDEX", "UNF", "SORTABLE", "INDEXMISSING", "INDEXEMPTY"] + canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "UNF", "NOINDEX"] + + _normalize_field_modifiers(field, canonical_order, want_unf=True) + + assert field.args_suffix == [ + "INDEXEMPTY", + "INDEXMISSING", + "SORTABLE", + "UNF", + "NOINDEX", + ] + + def test_empty_suffix(self): + """Test with empty args_suffix.""" + field = RedisTextField("test") + field.args_suffix = [] + canonical_order = ["INDEXMISSING", "SORTABLE"] + + _normalize_field_modifiers(field, canonical_order) + + assert field.args_suffix == [] + + def test_only_unknown_modifiers(self): + """Test with only unknown modifiers.""" + field = RedisTextField("test") + field.args_suffix = ["CUSTOM1", "CUSTOM2"] + canonical_order = ["INDEXMISSING", "SORTABLE"] + + _normalize_field_modifiers(field, canonical_order) + + assert field.args_suffix == ["CUSTOM1", "CUSTOM2"] + + def test_multiple_unknown_modifiers_preserved_order(self): + """Test that multiple unknown modifiers preserve their original order.""" + field = RedisTextField("test") + field.args_suffix = ["CUSTOM1", "SORTABLE", "CUSTOM2", "INDEXMISSING"] + canonical_order = ["INDEXMISSING", "SORTABLE"] + + _normalize_field_modifiers(field, canonical_order) + + assert field.args_suffix == ["CUSTOM1", "CUSTOM2", "INDEXMISSING", "SORTABLE"] + + +class TestMLPCommandsScenario: + """Test the exact scenario from mlp_commands.txt.""" + + def test_work_experience_summary_field(self): + """Test TextField with INDEXMISSING SORTABLE UNF (mlp_commands.txt scenario).""" + field = TextField( + name="work_experience_summary", + attrs={"index_missing": True, "sortable": True, "unf": True}, + ) + redis_field = field.as_redis_field() + suffix = redis_field.args_suffix + + # Verify exact order from mlp_commands.txt + assert suffix == ["INDEXMISSING", "SORTABLE", "UNF"] + + def test_mlp_scenario_redis_args(self): + """Test that redis_args() produces correct command for mlp_commands.txt scenario.""" + field = TextField( + name="work_experience_summary", + attrs={"index_missing": True, "sortable": True, "unf": True}, + ) + redis_field = field.as_redis_field() + args = redis_field.redis_args() + + # Verify the args contain the field name and modifiers in correct order + assert "work_experience_summary" in args + assert "TEXT" in args + + # Find the position of TEXT and verify modifiers come after it + text_idx = args.index("TEXT") + remaining_args = args[text_idx + 1 :] + + # Verify INDEXMISSING comes before SORTABLE + if "INDEXMISSING" in remaining_args and "SORTABLE" in remaining_args: + assert remaining_args.index("INDEXMISSING") < remaining_args.index( + "SORTABLE" + ) + + # Verify SORTABLE comes before UNF + if "SORTABLE" in remaining_args and "UNF" in remaining_args: + assert remaining_args.index("SORTABLE") < remaining_args.index("UNF") From 221811ae553a339ffe51eac6abbaec449c4fe847 Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 16:31:23 -0500 Subject: [PATCH 2/9] Remove support for unknown attributes. --- redisvl/schema/fields.py | 12 ++++------ tests/unit/test_field_modifier_ordering.py | 26 ---------------------- 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/redisvl/schema/fields.py b/redisvl/schema/fields.py index ad97362e..69434b9d 100644 --- a/redisvl/schema/fields.py +++ b/redisvl/schema/fields.py @@ -107,8 +107,7 @@ def _normalize_field_modifiers( RediSearch has a parser limitation (redis/redis#5177) where INDEXEMPTY and INDEXMISSING must appear BEFORE SORTABLE in field definitions. This function - reorders field.args_suffix to match the canonical order while preserving - unknown modifiers at the start. + reorders field.args_suffix to match the canonical order. Args: field: Redis field object whose args_suffix will be normalized @@ -116,7 +115,7 @@ def _normalize_field_modifiers( want_unf: Whether UNF should be added after SORTABLE (default: False) Time Complexity: O(n + m) where n = len(field.args_suffix), m = len(canonical_order) - Space Complexity: O(n + m) + Space Complexity: O(n) Example: >>> field = RedisTextField("title") @@ -126,12 +125,9 @@ def _normalize_field_modifiers( ['INDEXMISSING', 'SORTABLE'] """ suffix_set = set(field.args_suffix) - known_set = set(canonical_order) - # Preserve unknown modifiers in original order - new_suffix = [t for t in field.args_suffix if t not in known_set] - - # Add known modifiers in canonical order + # Build new suffix with only known modifiers in canonical order + new_suffix = [] for modifier in canonical_order: if modifier in suffix_set: new_suffix.append(modifier) diff --git a/tests/unit/test_field_modifier_ordering.py b/tests/unit/test_field_modifier_ordering.py index 59ebdbe5..75b02037 100644 --- a/tests/unit/test_field_modifier_ordering.py +++ b/tests/unit/test_field_modifier_ordering.py @@ -252,15 +252,7 @@ def test_basic_reordering(self): assert field.args_suffix == ["INDEXMISSING", "SORTABLE"] - def test_preserves_unknown_modifiers(self): - """Test that unknown modifiers are preserved at the start.""" - field = RedisTextField("test") - field.args_suffix = ["CUSTOM", "SORTABLE", "INDEXMISSING"] - canonical_order = ["INDEXMISSING", "SORTABLE"] - - _normalize_field_modifiers(field, canonical_order) - assert field.args_suffix == ["CUSTOM", "INDEXMISSING", "SORTABLE"] def test_unf_added_with_sortable(self): """Test that UNF is added when want_unf=True and SORTABLE is present.""" @@ -318,25 +310,7 @@ def test_empty_suffix(self): assert field.args_suffix == [] - def test_only_unknown_modifiers(self): - """Test with only unknown modifiers.""" - field = RedisTextField("test") - field.args_suffix = ["CUSTOM1", "CUSTOM2"] - canonical_order = ["INDEXMISSING", "SORTABLE"] - - _normalize_field_modifiers(field, canonical_order) - - assert field.args_suffix == ["CUSTOM1", "CUSTOM2"] - - def test_multiple_unknown_modifiers_preserved_order(self): - """Test that multiple unknown modifiers preserve their original order.""" - field = RedisTextField("test") - field.args_suffix = ["CUSTOM1", "SORTABLE", "CUSTOM2", "INDEXMISSING"] - canonical_order = ["INDEXMISSING", "SORTABLE"] - - _normalize_field_modifiers(field, canonical_order) - assert field.args_suffix == ["CUSTOM1", "CUSTOM2", "INDEXMISSING", "SORTABLE"] class TestMLPCommandsScenario: From 4297f6a9c12a0b8f40313d9db3950d92d0aedd2c Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 16:32:00 -0500 Subject: [PATCH 3/9] Format code --- .../integration/test_field_modifier_ordering_integration.py | 6 +++++- tests/unit/test_field_modifier_ordering.py | 4 ---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py index c75eea17..5bc17e9a 100644 --- a/tests/integration/test_field_modifier_ordering_integration.py +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -384,7 +384,11 @@ def test_indexmissing_enables_ismissing_query(self, redis_client): # Query for missing fields result = redis_client.execute_command( - "FT.SEARCH", "test_ismissing", "ismissing(@optional_field)", "DIALECT", "2" + "FT.SEARCH", + "test_ismissing", + "ismissing(@optional_field)", + "DIALECT", + "2", ) # Should return 1 result (ismiss:2) diff --git a/tests/unit/test_field_modifier_ordering.py b/tests/unit/test_field_modifier_ordering.py index 75b02037..133a7a2d 100644 --- a/tests/unit/test_field_modifier_ordering.py +++ b/tests/unit/test_field_modifier_ordering.py @@ -252,8 +252,6 @@ def test_basic_reordering(self): assert field.args_suffix == ["INDEXMISSING", "SORTABLE"] - - def test_unf_added_with_sortable(self): """Test that UNF is added when want_unf=True and SORTABLE is present.""" field = RedisTextField("test") @@ -311,8 +309,6 @@ def test_empty_suffix(self): assert field.args_suffix == [] - - class TestMLPCommandsScenario: """Test the exact scenario from mlp_commands.txt.""" From f49148f6be9b286120c4e4cc538bafa3f7830b18 Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 16:33:16 -0500 Subject: [PATCH 4/9] Update docstrings --- redisvl/schema/fields.py | 2 +- tests/unit/test_field_modifier_ordering.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/redisvl/schema/fields.py b/redisvl/schema/fields.py index 69434b9d..b7af624c 100644 --- a/redisvl/schema/fields.py +++ b/redisvl/schema/fields.py @@ -105,7 +105,7 @@ def _normalize_field_modifiers( ) -> None: """Normalize field modifier ordering for RediSearch parser. - RediSearch has a parser limitation (redis/redis#5177) where INDEXEMPTY and + RediSearch has a parser limitation where INDEXEMPTY and INDEXMISSING must appear BEFORE SORTABLE in field definitions. This function reorders field.args_suffix to match the canonical order. diff --git a/tests/unit/test_field_modifier_ordering.py b/tests/unit/test_field_modifier_ordering.py index 133a7a2d..fad097fc 100644 --- a/tests/unit/test_field_modifier_ordering.py +++ b/tests/unit/test_field_modifier_ordering.py @@ -5,7 +5,7 @@ to satisfy RediSearch parser requirements. The canonical order is: [INDEXEMPTY] [INDEXMISSING] [SORTABLE [UNF]] [NOINDEX] -This is required because RediSearch has a parser limitation (redis/redis#5177) +This is required because RediSearch has a parser limitation where INDEXEMPTY/INDEXMISSING must appear BEFORE SORTABLE in field definitions. """ From 31ed67e96c852cf874a998a36b0a942a42ee710b Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 17:31:13 -0500 Subject: [PATCH 5/9] fix integration tests to use testcontainers fixtures --- ...est_field_modifier_ordering_integration.py | 199 ++++++++---------- 1 file changed, 83 insertions(+), 116 deletions(-) diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py index 5bc17e9a..27310bae 100644 --- a/tests/integration/test_field_modifier_ordering_integration.py +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -2,33 +2,24 @@ Integration tests for field modifier ordering against live Redis. These tests verify that indices can be created successfully with various -field modifier combinations and that the modifiers are correctly set in Redis. +field modifier combinations. The key test is that index.create() succeeds +without error - if the modifiers are in the wrong order, Redis will reject +the FT.CREATE command. """ -import pytest -import redis - from redisvl.index import SearchIndex from redisvl.schema import IndexSchema -@pytest.fixture -def redis_client(): - """Fixture to provide a Redis client.""" - client = redis.Redis(host="localhost", port=6379, decode_responses=True) - yield client - # Cleanup is handled by individual tests - - class TestTextFieldModifierOrderingIntegration: """Integration tests for TextField modifier ordering.""" - def test_textfield_sortable_and_index_missing(self, redis_client): + def test_textfield_sortable_and_index_missing(self, client, redis_url, worker_id): """Test TextField with sortable and index_missing creates successfully.""" schema_dict = { "index": { - "name": "test_text_sortable_missing", - "prefix": "text_sm", + "name": f"test_text_sortable_missing_{worker_id}", + "prefix": f"text_sm_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -41,28 +32,26 @@ def test_textfield_sortable_and_index_missing(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "test_text_sortable_missing") + info = client.execute_command( + "FT.INFO", f"test_text_sortable_missing_{worker_id}" + ) assert info is not None - - # Verify field attributes contain both SORTABLE and INDEXMISSING - attrs = info[7][0] # Get field attributes - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs finally: index.delete(drop=True) - def test_textfield_all_modifiers(self, redis_client): + def test_textfield_all_modifiers(self, client, redis_url, worker_id): """Test TextField with all modifiers.""" schema_dict = { "index": { - "name": "test_text_all_mods", - "prefix": "text_all", + "name": f"test_text_all_mods_{worker_id}", + "prefix": f"text_all_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -80,21 +69,15 @@ def test_textfield_all_modifiers(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "test_text_all_mods") + info = client.execute_command("FT.INFO", f"test_text_all_mods_{worker_id}") assert info is not None - - # Verify all modifiers are present - attrs = info[7][0] - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs - assert "INDEXEMPTY" in attrs - assert "UNF" in attrs finally: index.delete(drop=True) @@ -102,12 +85,12 @@ def test_textfield_all_modifiers(self, redis_client): class TestTagFieldModifierOrderingIntegration: """Integration tests for TagField modifier ordering.""" - def test_tagfield_sortable_and_index_missing(self, redis_client): + def test_tagfield_sortable_and_index_missing(self, client, redis_url, worker_id): """Test TagField with sortable and index_missing creates successfully.""" schema_dict = { "index": { - "name": "test_tag_sortable_missing", - "prefix": "tag_sm", + "name": f"test_tag_sortable_missing_{worker_id}", + "prefix": f"tag_sm_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -120,28 +103,26 @@ def test_tagfield_sortable_and_index_missing(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "test_tag_sortable_missing") + info = client.execute_command( + "FT.INFO", f"test_tag_sortable_missing_{worker_id}" + ) assert info is not None - - # Verify field attributes contain both SORTABLE and INDEXMISSING - attrs = info[7][0] - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs finally: index.delete(drop=True) - def test_tagfield_all_modifiers(self, redis_client): + def test_tagfield_all_modifiers(self, client, redis_url, worker_id): """Test TagField with all modifiers.""" schema_dict = { "index": { - "name": "test_tag_all_mods", - "prefix": "tag_all", + "name": f"test_tag_all_mods_{worker_id}", + "prefix": f"tag_all_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -158,20 +139,15 @@ def test_tagfield_all_modifiers(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "test_tag_all_mods") + info = client.execute_command("FT.INFO", f"test_tag_all_mods_{worker_id}") assert info is not None - - # Verify all modifiers are present - attrs = info[7][0] - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs - assert "INDEXEMPTY" in attrs finally: index.delete(drop=True) @@ -179,12 +155,12 @@ def test_tagfield_all_modifiers(self, redis_client): class TestGeoFieldModifierOrderingIntegration: """Integration tests for GeoField modifier ordering.""" - def test_geofield_sortable_and_index_missing(self, redis_client): + def test_geofield_sortable_and_index_missing(self, client, redis_url, worker_id): """Test GeoField with sortable and index_missing creates successfully.""" schema_dict = { "index": { - "name": "test_geo_sortable_missing", - "prefix": "geo_sm", + "name": f"test_geo_sortable_missing_{worker_id}", + "prefix": f"geo_sm_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -197,19 +173,17 @@ def test_geofield_sortable_and_index_missing(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "test_geo_sortable_missing") + info = client.execute_command( + "FT.INFO", f"test_geo_sortable_missing_{worker_id}" + ) assert info is not None - - # Verify field attributes contain both SORTABLE and INDEXMISSING - attrs = info[7][0] - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs finally: index.delete(drop=True) @@ -217,12 +191,14 @@ def test_geofield_sortable_and_index_missing(self, redis_client): class TestNumericFieldModifierOrderingIntegration: """Integration tests for NumericField modifier ordering.""" - def test_numericfield_sortable_and_index_missing(self, redis_client): + def test_numericfield_sortable_and_index_missing( + self, client, redis_url, worker_id + ): """Test NumericField with sortable and index_missing creates successfully.""" schema_dict = { "index": { - "name": "test_numeric_sortable_missing", - "prefix": "num_sm", + "name": f"test_numeric_sortable_missing_{worker_id}", + "prefix": f"num_sm_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -235,34 +211,30 @@ def test_numericfield_sortable_and_index_missing(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command( - "FT.INFO", "test_numeric_sortable_missing" + info = client.execute_command( + "FT.INFO", f"test_numeric_sortable_missing_{worker_id}" ) assert info is not None - - # Verify field attributes contain both SORTABLE and INDEXMISSING - attrs = info[7][0] - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs finally: index.delete(drop=True) class TestMultiFieldModifierOrderingIntegration: - """Integration tests with multiple fields and modifiers.""" + """Integration tests for multiple field types with modifiers.""" - def test_mixed_field_types_with_modifiers(self, redis_client): + def test_mixed_field_types_with_modifiers(self, client, redis_url, worker_id): """Test index with multiple field types all using modifiers.""" schema_dict = { "index": { - "name": "test_mixed_fields", - "prefix": "mixed", + "name": f"test_mixed_fields_{worker_id}", + "prefix": f"mixed_{worker_id}", "storage_type": "hash", }, "fields": [ @@ -290,45 +262,42 @@ def test_mixed_field_types_with_modifiers(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: + # This should succeed - if modifiers are in wrong order, it will fail index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "test_mixed_fields") + info = client.execute_command("FT.INFO", f"test_mixed_fields_{worker_id}") assert info is not None # Verify all fields were created attrs_list = info[7] assert len(attrs_list) == 4 - - # Verify each field has the correct modifiers - for attrs in attrs_list: - assert "SORTABLE" in attrs - assert "INDEXMISSING" in attrs finally: index.delete(drop=True) -class TestMLPCommandsScenarioIntegration: - """Integration tests for the exact scenario from mlp_commands.txt.""" +class TestMultipleCommandsScenarioIntegration: + """Integration tests for mlp_commands.txt scenario.""" - def test_mlp_commands_index_creation(self, redis_client): - """Test creating index matching mlp_commands.txt scenario. + def test_mlp_commands_index_creation(self, client, redis_url, worker_id): + """Test creating index with INDEXMISSING SORTABLE UNF modifiers. - This test verifies that an index with INDEXMISSING SORTABLE UNF - modifiers can be created successfully with correct field ordering. + This test verifies that an index with all three modifiers + (INDEXMISSING, SORTABLE, UNF) can be created successfully with + correct field ordering. """ schema_dict = { "index": { - "name": "testidx", - "prefix": "test:", + "name": f"testidx_{worker_id}", + "prefix": f"test_{worker_id}:", "storage_type": "hash", }, "fields": [ { - "name": "work_experience_summary", + "name": "description", "type": "text", "attrs": {"index_missing": True, "sortable": True, "unf": True}, } @@ -336,30 +305,24 @@ def test_mlp_commands_index_creation(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: # This should succeed with correct modifier ordering index.create(overwrite=True) # Verify index was created - info = redis_client.execute_command("FT.INFO", "testidx") + info = client.execute_command("FT.INFO", f"testidx_{worker_id}") assert info is not None - - # Verify field attributes - attrs = info[7][0] - assert "INDEXMISSING" in attrs - assert "SORTABLE" in attrs - assert "UNF" in attrs finally: index.delete(drop=True) - def test_indexmissing_enables_ismissing_query(self, redis_client): + def test_indexmissing_enables_ismissing_query(self, client, redis_url, worker_id): """Test that INDEXMISSING enables ismissing() query function.""" schema_dict = { "index": { - "name": "test_ismissing", - "prefix": "ismiss:", + "name": f"test_ismissing_{worker_id}", + "prefix": f"ismiss_{worker_id}:", "storage_type": "hash", }, "fields": [ @@ -372,29 +335,33 @@ def test_indexmissing_enables_ismissing_query(self, redis_client): } schema = IndexSchema.from_dict(schema_dict) - index = SearchIndex(schema=schema, redis_url="redis://localhost:6379") + index = SearchIndex(schema=schema, redis_url=redis_url) try: index.create(overwrite=True) # Create documents with and without the field - redis_client.hset("ismiss:1", "optional_field", "has value") - redis_client.hset("ismiss:2", "other_field", "no optional_field") - redis_client.hset("ismiss:3", "optional_field", "also has value") + client.hset(f"ismiss_{worker_id}:1", "optional_field", "has value") + client.hset(f"ismiss_{worker_id}:2", "other_field", "no optional_field") + client.hset(f"ismiss_{worker_id}:3", "optional_field", "also has value") # Query for missing fields - result = redis_client.execute_command( + result = client.execute_command( "FT.SEARCH", - "test_ismissing", + f"test_ismissing_{worker_id}", "ismissing(@optional_field)", "DIALECT", "2", ) - # Should return 1 result (ismiss:2) + # Should return 1 result (ismiss_{worker_id}:2) assert result[0] == 1 - assert "ismiss:2" in str(result) + assert f"ismiss_{worker_id}:2" in str(result) finally: - redis_client.delete("ismiss:1", "ismiss:2", "ismiss:3") + client.delete( + f"ismiss_{worker_id}:1", + f"ismiss_{worker_id}:2", + f"ismiss_{worker_id}:3", + ) index.delete(drop=True) From f65a51b4f643c7defcd76b5300dc81a177176cff Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 17:44:10 -0500 Subject: [PATCH 6/9] Remove index_empty for geo and numerics tags and update tests --- redisvl/schema/fields.py | 10 ++- ...est_field_modifier_ordering_integration.py | 80 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/redisvl/schema/fields.py b/redisvl/schema/fields.py index b7af624c..74717acc 100644 --- a/redisvl/schema/fields.py +++ b/redisvl/schema/fields.py @@ -490,8 +490,9 @@ def as_redis_field(self) -> RedisField: field = RedisNumericField(name, **kwargs) # Normalize suffix ordering to satisfy RediSearch parser expectations. - # Canonical order: [INDEXEMPTY] [INDEXMISSING] [SORTABLE [UNF]] [NOINDEX] - canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "UNF", "NOINDEX"] + # Canonical order: [INDEXMISSING] [SORTABLE [UNF]] [NOINDEX] + # Note: INDEXEMPTY is not supported for NUMERIC fields + canonical_order = ["INDEXMISSING", "SORTABLE", "UNF", "NOINDEX"] want_unf = self.attrs.unf and self.attrs.sortable # type: ignore _normalize_field_modifiers(field, canonical_order, want_unf) @@ -526,8 +527,9 @@ def as_redis_field(self) -> RedisField: field = RedisGeoField(name, **kwargs) # Normalize suffix ordering to satisfy RediSearch parser expectations. - # Canonical order: [INDEXEMPTY] [INDEXMISSING] [SORTABLE] [NOINDEX] - canonical_order = ["INDEXEMPTY", "INDEXMISSING", "SORTABLE", "NOINDEX"] + # Canonical order: [INDEXMISSING] [SORTABLE] [NOINDEX] + # Note: INDEXEMPTY is not supported for GEO fields + canonical_order = ["INDEXMISSING", "SORTABLE", "NOINDEX"] _normalize_field_modifiers(field, canonical_order) return field diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py index 27310bae..09749349 100644 --- a/tests/integration/test_field_modifier_ordering_integration.py +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -365,3 +365,83 @@ def test_indexmissing_enables_ismissing_query(self, client, redis_url, worker_id f"ismiss_{worker_id}:3", ) index.delete(drop=True) + + +class TestFieldTypeModifierSupport: + """Test that field types only support their documented modifiers.""" + + def test_numeric_field_does_not_support_index_empty( + self, client, redis_url, worker_id + ): + """Verify that NumericField does not have index_empty attribute. + + INDEXEMPTY is only supported for TEXT and TAG fields according to + RediSearch documentation. NumericFieldAttributes should not have + an index_empty attribute. + """ + import inspect + + from redisvl.schema.fields import NumericFieldAttributes + + # Verify NumericFieldAttributes doesn't have index_empty + attrs = inspect.signature(NumericFieldAttributes).parameters + assert ( + "index_empty" not in attrs + ), "NumericFieldAttributes should not have index_empty parameter" + + # Verify the attribute doesn't exist on the class + field_attrs = NumericFieldAttributes() + assert not hasattr( + field_attrs, "index_empty" + ), "NumericFieldAttributes should not have index_empty attribute" + + def test_geo_field_does_not_support_index_empty(self, client, redis_url, worker_id): + """Verify that GeoField does not have index_empty attribute. + + INDEXEMPTY is only supported for TEXT and TAG fields according to + RediSearch documentation. GeoFieldAttributes should not have + an index_empty attribute. + """ + import inspect + + from redisvl.schema.fields import GeoFieldAttributes + + # Verify GeoFieldAttributes doesn't have index_empty + attrs = inspect.signature(GeoFieldAttributes).parameters + assert ( + "index_empty" not in attrs + ), "GeoFieldAttributes should not have index_empty parameter" + + # Verify the attribute doesn't exist on the class + field_attrs = GeoFieldAttributes() + assert not hasattr( + field_attrs, "index_empty" + ), "GeoFieldAttributes should not have index_empty attribute" + + def test_text_field_supports_index_empty(self, client, redis_url, worker_id): + """Verify that TextField supports index_empty attribute. + + INDEXEMPTY is supported for TEXT fields according to RediSearch documentation. + """ + from redisvl.schema.fields import TextFieldAttributes + + # Verify TextFieldAttributes has index_empty + field_attrs = TextFieldAttributes(index_empty=True) + assert hasattr( + field_attrs, "index_empty" + ), "TextFieldAttributes should have index_empty attribute" + assert field_attrs.index_empty is True + + def test_tag_field_supports_index_empty(self, client, redis_url, worker_id): + """Verify that TagField supports index_empty attribute. + + INDEXEMPTY is supported for TAG fields according to RediSearch documentation. + """ + from redisvl.schema.fields import TagFieldAttributes + + # Verify TagFieldAttributes has index_empty + field_attrs = TagFieldAttributes(index_empty=True) + assert hasattr( + field_attrs, "index_empty" + ), "TagFieldAttributes should have index_empty attribute" + assert field_attrs.index_empty is True From 94e2ff29478f38983836faca722a958f2c1420dc Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Thu, 20 Nov 2025 18:13:49 -0500 Subject: [PATCH 7/9] test: fix integration test cleanup for CI --- ...est_field_modifier_ordering_integration.py | 82 +++++++++++++++---- 1 file changed, 68 insertions(+), 14 deletions(-) diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py index 09749349..3f6f8fa8 100644 --- a/tests/integration/test_field_modifier_ordering_integration.py +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -7,15 +7,34 @@ the FT.CREATE command. """ +import pytest + from redisvl.index import SearchIndex +from redisvl.redis.connection import RedisConnectionFactory from redisvl.schema import IndexSchema +MIN_SEARCH_VERSION_FOR_INDEXMISSING = 21000 # RediSearch 2.10.0+ + + +def skip_if_search_version_below_for_indexmissing(client) -> None: + """Skip tests that require INDEXMISSING/INDEXEMPTY if RediSearch is too old.""" + modules = RedisConnectionFactory.get_modules(client) + search_ver = modules.get("search", 0) + searchlight_ver = modules.get("searchlight", 0) + current_ver = max(search_ver, searchlight_ver) + if current_ver < MIN_SEARCH_VERSION_FOR_INDEXMISSING: + pytest.skip( + "INDEXMISSING/INDEXEMPTY require RediSearch 2.10+ " + f"(found module version {current_ver})" + ) + class TestTextFieldModifierOrderingIntegration: """Integration tests for TextField modifier ordering.""" def test_textfield_sortable_and_index_missing(self, client, redis_url, worker_id): """Test TextField with sortable and index_missing creates successfully.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_text_sortable_missing_{worker_id}", @@ -44,10 +63,14 @@ def test_textfield_sortable_and_index_missing(self, client, redis_url, worker_id ) assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early def test_textfield_all_modifiers(self, client, redis_url, worker_id): """Test TextField with all modifiers.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_text_all_mods_{worker_id}", @@ -79,7 +102,10 @@ def test_textfield_all_modifiers(self, client, redis_url, worker_id): info = client.execute_command("FT.INFO", f"test_text_all_mods_{worker_id}") assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early class TestTagFieldModifierOrderingIntegration: @@ -87,6 +113,7 @@ class TestTagFieldModifierOrderingIntegration: def test_tagfield_sortable_and_index_missing(self, client, redis_url, worker_id): """Test TagField with sortable and index_missing creates successfully.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_tag_sortable_missing_{worker_id}", @@ -115,10 +142,14 @@ def test_tagfield_sortable_and_index_missing(self, client, redis_url, worker_id) ) assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early def test_tagfield_all_modifiers(self, client, redis_url, worker_id): """Test TagField with all modifiers.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_tag_all_mods_{worker_id}", @@ -149,7 +180,10 @@ def test_tagfield_all_modifiers(self, client, redis_url, worker_id): info = client.execute_command("FT.INFO", f"test_tag_all_mods_{worker_id}") assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early class TestGeoFieldModifierOrderingIntegration: @@ -157,6 +191,7 @@ class TestGeoFieldModifierOrderingIntegration: def test_geofield_sortable_and_index_missing(self, client, redis_url, worker_id): """Test GeoField with sortable and index_missing creates successfully.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_geo_sortable_missing_{worker_id}", @@ -185,7 +220,10 @@ def test_geofield_sortable_and_index_missing(self, client, redis_url, worker_id) ) assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early class TestNumericFieldModifierOrderingIntegration: @@ -195,6 +233,7 @@ def test_numericfield_sortable_and_index_missing( self, client, redis_url, worker_id ): """Test NumericField with sortable and index_missing creates successfully.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_numeric_sortable_missing_{worker_id}", @@ -223,7 +262,10 @@ def test_numericfield_sortable_and_index_missing( ) assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early class TestMultiFieldModifierOrderingIntegration: @@ -231,6 +273,7 @@ class TestMultiFieldModifierOrderingIntegration: def test_mixed_field_types_with_modifiers(self, client, redis_url, worker_id): """Test index with multiple field types all using modifiers.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_mixed_fields_{worker_id}", @@ -276,7 +319,10 @@ def test_mixed_field_types_with_modifiers(self, client, redis_url, worker_id): attrs_list = info[7] assert len(attrs_list) == 4 finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early class TestMultipleCommandsScenarioIntegration: @@ -289,6 +335,7 @@ def test_mlp_commands_index_creation(self, client, redis_url, worker_id): (INDEXMISSING, SORTABLE, UNF) can be created successfully with correct field ordering. """ + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"testidx_{worker_id}", @@ -315,10 +362,14 @@ def test_mlp_commands_index_creation(self, client, redis_url, worker_id): info = client.execute_command("FT.INFO", f"testidx_{worker_id}") assert info is not None finally: - index.delete(drop=True) + try: + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early def test_indexmissing_enables_ismissing_query(self, client, redis_url, worker_id): """Test that INDEXMISSING enables ismissing() query function.""" + skip_if_search_version_below_for_indexmissing(client) schema_dict = { "index": { "name": f"test_ismissing_{worker_id}", @@ -359,12 +410,15 @@ def test_indexmissing_enables_ismissing_query(self, client, redis_url, worker_id assert f"ismiss_{worker_id}:2" in str(result) finally: - client.delete( - f"ismiss_{worker_id}:1", - f"ismiss_{worker_id}:2", - f"ismiss_{worker_id}:3", - ) - index.delete(drop=True) + try: + client.delete( + f"ismiss_{worker_id}:1", + f"ismiss_{worker_id}:2", + f"ismiss_{worker_id}:3", + ) + index.delete(drop=True) + except Exception: + pass # Index may not exist if test was skipped or failed early class TestFieldTypeModifierSupport: From 6342f2b1dd092600e32951b91b4b6a433759ba4d Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Fri, 21 Nov 2025 10:21:35 -0500 Subject: [PATCH 8/9] refactor: Improve field modifier test cleanup and naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two improvements to field modifier ordering integration tests: 1. Remove try/except cleanup pattern - Replaced try/finally/except blocks with direct cleanup calls - Affects 9 test methods across all field types - Cleanup failures now visible instead of silently ignored - Matches pattern used in other integration tests 2. Rename for open-source clarity - TestMultipleCommandsScenarioIntegration → TestFieldModifierIntegration - test_mlp_commands_index_creation → test_index_creation_with_multiple_modifiers - Use general naming instead of MLP-specific terminology These changes improve test maintainability and make failures more visible. --- ...est_field_modifier_ordering_integration.py | 242 ++++++++---------- 1 file changed, 107 insertions(+), 135 deletions(-) diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py index 3f6f8fa8..cea4d852 100644 --- a/tests/integration/test_field_modifier_ordering_integration.py +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -53,20 +53,17 @@ def test_textfield_sortable_and_index_missing(self, client, redis_url, worker_id schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) - - # Verify index was created - info = client.execute_command( - "FT.INFO", f"test_text_sortable_missing_{worker_id}" - ) - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) + + # Verify index was created + info = client.execute_command( + "FT.INFO", f"test_text_sortable_missing_{worker_id}" + ) + assert info is not None + + # Cleanup + index.delete(drop=True) def test_textfield_all_modifiers(self, client, redis_url, worker_id): """Test TextField with all modifiers.""" @@ -94,18 +91,15 @@ def test_textfield_all_modifiers(self, client, redis_url, worker_id): schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) - # Verify index was created - info = client.execute_command("FT.INFO", f"test_text_all_mods_{worker_id}") - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # Verify index was created + info = client.execute_command("FT.INFO", f"test_text_all_mods_{worker_id}") + assert info is not None + + # Cleanup + index.delete(drop=True) class TestTagFieldModifierOrderingIntegration: @@ -132,20 +126,17 @@ def test_tagfield_sortable_and_index_missing(self, client, redis_url, worker_id) schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) - - # Verify index was created - info = client.execute_command( - "FT.INFO", f"test_tag_sortable_missing_{worker_id}" - ) - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) + + # Verify index was created + info = client.execute_command( + "FT.INFO", f"test_tag_sortable_missing_{worker_id}" + ) + assert info is not None + + # Cleanup + index.delete(drop=True) def test_tagfield_all_modifiers(self, client, redis_url, worker_id): """Test TagField with all modifiers.""" @@ -172,18 +163,15 @@ def test_tagfield_all_modifiers(self, client, redis_url, worker_id): schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) + + # Verify index was created + info = client.execute_command("FT.INFO", f"test_tag_all_mods_{worker_id}") + assert info is not None - # Verify index was created - info = client.execute_command("FT.INFO", f"test_tag_all_mods_{worker_id}") - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # Cleanup + index.delete(drop=True) class TestGeoFieldModifierOrderingIntegration: @@ -210,20 +198,17 @@ def test_geofield_sortable_and_index_missing(self, client, redis_url, worker_id) schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) + + # Verify index was created + info = client.execute_command( + "FT.INFO", f"test_geo_sortable_missing_{worker_id}" + ) + assert info is not None - # Verify index was created - info = client.execute_command( - "FT.INFO", f"test_geo_sortable_missing_{worker_id}" - ) - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # Cleanup + index.delete(drop=True) class TestNumericFieldModifierOrderingIntegration: @@ -252,20 +237,17 @@ def test_numericfield_sortable_and_index_missing( schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) - # Verify index was created - info = client.execute_command( - "FT.INFO", f"test_numeric_sortable_missing_{worker_id}" - ) - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # Verify index was created + info = client.execute_command( + "FT.INFO", f"test_numeric_sortable_missing_{worker_id}" + ) + assert info is not None + + # Cleanup + index.delete(drop=True) class TestMultiFieldModifierOrderingIntegration: @@ -307,28 +289,25 @@ def test_mixed_field_types_with_modifiers(self, client, redis_url, worker_id): schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed - if modifiers are in wrong order, it will fail - index.create(overwrite=True) + # This should succeed - if modifiers are in wrong order, it will fail + index.create(overwrite=True) + + # Verify index was created + info = client.execute_command("FT.INFO", f"test_mixed_fields_{worker_id}") + assert info is not None - # Verify index was created - info = client.execute_command("FT.INFO", f"test_mixed_fields_{worker_id}") - assert info is not None + # Verify all fields were created + attrs_list = info[7] + assert len(attrs_list) == 4 - # Verify all fields were created - attrs_list = info[7] - assert len(attrs_list) == 4 - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # Cleanup + index.delete(drop=True) -class TestMultipleCommandsScenarioIntegration: - """Integration tests for mlp_commands.txt scenario.""" +class TestFieldModifierIntegration: + """Integration tests for complex field modifier combinations.""" - def test_mlp_commands_index_creation(self, client, redis_url, worker_id): + def test_index_creation_with_multiple_modifiers(self, client, redis_url, worker_id): """Test creating index with INDEXMISSING SORTABLE UNF modifiers. This test verifies that an index with all three modifiers @@ -354,18 +333,15 @@ def test_mlp_commands_index_creation(self, client, redis_url, worker_id): schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - # This should succeed with correct modifier ordering - index.create(overwrite=True) + # This should succeed with correct modifier ordering + index.create(overwrite=True) - # Verify index was created - info = client.execute_command("FT.INFO", f"testidx_{worker_id}") - assert info is not None - finally: - try: - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + # Verify index was created + info = client.execute_command("FT.INFO", f"testidx_{worker_id}") + assert info is not None + + # Cleanup + index.delete(drop=True) def test_indexmissing_enables_ismissing_query(self, client, redis_url, worker_id): """Test that INDEXMISSING enables ismissing() query function.""" @@ -388,37 +364,33 @@ def test_indexmissing_enables_ismissing_query(self, client, redis_url, worker_id schema = IndexSchema.from_dict(schema_dict) index = SearchIndex(schema=schema, redis_url=redis_url) - try: - index.create(overwrite=True) - - # Create documents with and without the field - client.hset(f"ismiss_{worker_id}:1", "optional_field", "has value") - client.hset(f"ismiss_{worker_id}:2", "other_field", "no optional_field") - client.hset(f"ismiss_{worker_id}:3", "optional_field", "also has value") - - # Query for missing fields - result = client.execute_command( - "FT.SEARCH", - f"test_ismissing_{worker_id}", - "ismissing(@optional_field)", - "DIALECT", - "2", - ) - - # Should return 1 result (ismiss_{worker_id}:2) - assert result[0] == 1 - assert f"ismiss_{worker_id}:2" in str(result) - - finally: - try: - client.delete( - f"ismiss_{worker_id}:1", - f"ismiss_{worker_id}:2", - f"ismiss_{worker_id}:3", - ) - index.delete(drop=True) - except Exception: - pass # Index may not exist if test was skipped or failed early + index.create(overwrite=True) + + # Create documents with and without the field + client.hset(f"ismiss_{worker_id}:1", "optional_field", "has value") + client.hset(f"ismiss_{worker_id}:2", "other_field", "no optional_field") + client.hset(f"ismiss_{worker_id}:3", "optional_field", "also has value") + + # Query for missing fields + result = client.execute_command( + "FT.SEARCH", + f"test_ismissing_{worker_id}", + "ismissing(@optional_field)", + "DIALECT", + "2", + ) + + # Should return 1 result (ismiss_{worker_id}:2) + assert result[0] == 1 + assert f"ismiss_{worker_id}:2" in str(result) + + # Cleanup + client.delete( + f"ismiss_{worker_id}:1", + f"ismiss_{worker_id}:2", + f"ismiss_{worker_id}:3", + ) + index.delete(drop=True) class TestFieldTypeModifierSupport: From 771a6ded66ac02da6a58272677d36b1657e83b64 Mon Sep 17 00:00:00 2001 From: Nitin Kanukolanu Date: Fri, 21 Nov 2025 10:44:40 -0500 Subject: [PATCH 9/9] refactor: Address code review feedback for field modifier ordering Address valid GitHub Copilot feedback: 1. Replace magic number index access in FT.INFO parsing - Changed info[7] to proper key-based lookup for 'attributes' - More resilient to Redis response format changes - Added helpful error message if key not found 2. Enhance time complexity documentation - Added detailed breakdown of O(n + m) complexity - Clarified set creation and lookup costs Rejected feedback about testing implementation details - unit tests appropriately test the _normalize_field_modifiers helper function directly, while integration tests verify end-to-end behavior. --- redisvl/schema/fields.py | 4 +++- .../test_field_modifier_ordering_integration.py | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/redisvl/schema/fields.py b/redisvl/schema/fields.py index 74717acc..b7492767 100644 --- a/redisvl/schema/fields.py +++ b/redisvl/schema/fields.py @@ -114,7 +114,9 @@ def _normalize_field_modifiers( canonical_order: List of modifiers in desired canonical order want_unf: Whether UNF should be added after SORTABLE (default: False) - Time Complexity: O(n + m) where n = len(field.args_suffix), m = len(canonical_order) + Time Complexity: O(n + m), where n = len(field.args_suffix), m = len(canonical_order). + - O(n) to create the set from field.args_suffix + - O(m) to iterate over canonical_order and perform set lookups (O(1) average case per lookup) Space Complexity: O(n) Example: diff --git a/tests/integration/test_field_modifier_ordering_integration.py b/tests/integration/test_field_modifier_ordering_integration.py index cea4d852..4fc1dabb 100644 --- a/tests/integration/test_field_modifier_ordering_integration.py +++ b/tests/integration/test_field_modifier_ordering_integration.py @@ -297,7 +297,13 @@ def test_mixed_field_types_with_modifiers(self, client, redis_url, worker_id): assert info is not None # Verify all fields were created - attrs_list = info[7] + # Find the 'attributes' key in the FT.INFO response (flat list format) + attrs_list = None + for i in range(0, len(info) - 1, 2): + if info[i] == b"attributes" or info[i] == "attributes": + attrs_list = info[i + 1] + break + assert attrs_list is not None, "'attributes' key not found in FT.INFO response" assert len(attrs_list) == 4 # Cleanup