Skip to content

Feature/alias as index attribute - #9

Closed
rileydes-improving wants to merge 13 commits into
mainfrom
feature/alias-as-index-attribute
Closed

Feature/alias as index attribute#9
rileydes-improving wants to merge 13 commits into
mainfrom
feature/alias-as-index-attribute

Conversation

@rileydes-improving

Copy link
Copy Markdown

Summary

Adds FT.ALIASADD, FT.ALIASDEL, and FT.ALIASUPDATE with RediSearch-compatible behavior. Also adds FT.ALIASLIST (not present in RediSearch) for listing all active aliases. Aliases live as a repeated field on the owning IndexSchema proto, persist in RDB as part of the index, and propagate cluster-wide through the existing MetadataManager / FT.INTERNAL_UPDATE path. No separate metadata type needed.

Changes

Proto & IndexSchema

  • Added repeated string aliases = 15 to IndexSchema in index_schema.proto
  • Extended IndexSchema class with GetAliases() / SetAliases() accessors and in-memory alias list

SchemaManager — Alias CRUD & Resolution

  • Added db_to_aliases_ (flat_hash_map<uint32_t, flat_hash_map<string, string>>) as a forward alias map (alias → index name) per database
  • Implemented AddAlias, RemoveAlias, UpdateAlias, and GetAllAliases
  • In coordinator mode: each alias operation fetches the owning index's proto from MetadataManager, mutates the aliases repeated field, normalizes defaults, and re-commits via CreateEntry, which triggers OnMetadataCallback cluster-wide
  • In standalone mode: directly mutates the in-memory alias map and IndexSchema's alias list under the mutex
  • GetIndexSchema falls through to alias resolution when direct name lookup misses
  • RemoveIndexSchemaInternal purges all aliases for a dropped index via EraseAliasesForIndex
  • OnMetadataCallback uses MessageDifferencer (ignoring aliases and stats fields) to detect alias-only changes: when structural fields are unchanged, it calls RebuildAliasMapsForIndex and SetAliases without tearing down the index
  • ComputeFingerprint clears the aliases field before hashing so alias mutations don't destabilize search consistency checks
  • OnFlushDBEnded in coordinator mode fetches the authoritative proto from MetadataManager (including aliases) when recreating indexes after FLUSHDB
  • OnSwapDB swaps alias maps in lockstep with index maps

FT.INTERNAL_UPDATE, Type Routing (Requested by Allen)

  • Extended FT.INTERNAL_UPDATE to accept optional keyword/count/args-style parameters after the required 4 arguments. Currently recognizes TYPE <1> <type_name> (defaults to kSchemaManagerMetadataTypeName for AOF backward compatibility).
  • ReconcileMetadata now skips replication for types not registered locally, preventing errors on nodes that don't recognize a metadata type.

Commands

  • FTAliasAddCmd, FTAliasDelCmd, FTAliasUpdateCmd in ft_alias_add.cc, ft_alias_del.cc, ft_alias_update.cc, registered in module_loader.cc with @search @write @fast ACL permissions
  • FTAliasListCmd in ft_alias_list.cc, returns all alias->index pairs for the selected database
  • In cluster (CME) mode, ALIASADD/ALIASUPDATE perform a consistency-check fanout (AliasExistsConsistencyCheckFanoutOperation). ALIASDEL uses AliasRemovedConsistencyCheckFanoutOperation to wait until the change is visible on all primaries.
  • MULTI/EXEC and Lua are rejected in CME mode to avoid local-only mutations without fanout
  • ft_dropindex.cc resolves an alias to its real index name before calling RemoveIndexSchema
  • ft_aggregate.cc uses the alias field when constructing its response

FT.INFO

  • ft_info_parser.cc calls GetAliases() on the IndexSchema and reports a sorted aliases array in FT.INFO output

Testing

  • Unit tests in ft_alias_commands_test.cc: argument count errors, happy paths, duplicate rejection, alias-to-alias rejection, self-referential alias, and ReplicateVerbatim behavior in both coordinator modes
  • Unit tests in schema_manager_alias_property_test.cc: property-based tests for add/remove/update round-trips, cross-database isolation, upsert semantics, FLUSHDB/SWAPDB behavior, alias-only metadata callback, and fingerprint stability
  • Unit tests in ft_info_test.cc: sorted aliases array and querying via alias still reports the real index name
  • Integration tests in test_ft_alias.py: all four commands, DROPINDEX cleanup, FT.SEARCH/FT.AGGREGATE via alias, FLUSHDB, SWAPDB, RDB persistence, wrong argument errors, and name-collision edge cases
  • Integration tests in test_ft_alias_cluster.py: ALIASADD, ALIASDEL, ALIASUPDATE, ALIASLIST, and DROPINDEX tombstoning all propagate to every cluster primary
  • Compatibility: alias-answers.pickle.gz added to the parametrized run in compatibility_test.py. Alias management commands are replayed inline to reconstruct alias state before dependent search/aggregate queries.

Architecture Difference vs. PR valkey-io#955

PR valkey-io#955 introduced a separate metadata type (vs_alias) with its own MetadataManager registration, OnAliasMetadataCallback, and dedicated RDB_SECTION_ALIAS_MAP section. This PR stores aliases directly as a repeated field on IndexSchema, treating alias mutations as proto updates through the existing vs_index_schema metadata pathway. That removes the need for a separate metadata type, a separate RDB section, and the ordering constraint (indexes must load before aliases). Alias changes propagate via the same CreateEntry call used for index creation. OnMetadataCallback detects alias-only changes via MessageDifferencer and applies them without index teardown.

Example

FT.CREATE my_idx ON HASH PREFIX 1 doc: SCHEMA category TAG
HSET doc:1 category books
FT.ALIASADD my_alias my_idx
FT.SEARCH my_alias "@category:{books}"

Returns 1 result (doc:1) via the alias without any change to the underlying index.

Add repeated string aliases field (tag 15) to the IndexSchema protobuf
message. Extend the C++ IndexSchema class with an aliases_ vector member,
SetAliases()/GetAliases() accessors, construction from proto, serialization
in ToProto(), and exposure in the FT.INFO response output.

Signed-off-by: Riley Des <riley.desserre@improving.com>
Add forward alias map (db_to_aliases_) with AddAlias, RemoveAlias,
UpdateAlias, and GetAllAliases methods supporting both standalone and
coordinator modes. Integrate alias resolution into GetIndexSchema so
existing commands transparently resolve aliases to real index names.

Handle alias map maintenance in OnMetadataCallback (alias-only changes
skip full index teardown via MessageDifferencer), OnFlushDBEnded,
OnSwapDB, OnLoadingEnded, LoadIndex, and RemoveIndexSchemaInternal.

Add NormalizeIndexSchemaProtoDefaults to prevent spurious proto diffs
from triggering unnecessary index rebuilds on alias-only metadata
updates.

Signed-off-by: Riley Des <riley.desserre@improving.com>
Extend FT.INTERNAL_UPDATE wire format with optional keyword/count/args
parsing. Emit TYPE keyword in ReplicateFTInternalUpdate and
CallFTInternalUpdateForReconciliation so replicas apply metadata to
the correct registered type.

Guard reconciliation replication behind type registration check to
avoid acting on unregistered metadata types. Add !Kcbbccc format
handler to test mock.

Signed-off-by: Riley Des <riley.desserre@improving.com>
…SLIST

Implement four new commands for index alias management with proper ACL
categories, arity validation, MULTI/EXEC rejection in CME mode, and
ValkeyModule_ReplicateVerbatim for standalone replication.

Add cluster consistency fanout operations that verify alias propagation
across all nodes before replying to the client. Register commands and
JSON specs in module_loader and CMakeLists.

Signed-off-by: Riley Des <riley.desserre@improving.com>
…REGATE

FT.DROPINDEX now resolves the user-supplied name to the real index name
via GetName() before calling RemoveIndexSchema and the drop fanout
operation, so dropping an index by alias works correctly.

FT.AGGREGATE response generation now uses the attribute alias_ field
instead of identifier_ for field names in the reply, matching expected
output when attributes have user-defined aliases.

Signed-off-by: Riley Des <riley.desserre@improving.com>
Add ft_alias_commands_test covering ALIASADD/DEL/UPDATE/LIST command
handlers with mock validation of arity, ACL, error conditions, and
cluster consistency fanout behavior.

Add schema_manager_alias_property_test with comprehensive coverage of
alias CRUD operations, collision detection, resolution, persistence
through RDB load, OnMetadataCallback alias-only fast path, FlushDB/
SwapDB alias map maintenance, and coordinator-mode proto round-trips.

Update ft_internal_update_test and metadata_manager_test for the new
TYPE keyword argument format.

Signed-off-by: Riley Des <riley.desserre@improving.com>
Add test_ft_alias.py with standalone integration tests covering alias
CRUD, resolution through FT.SEARCH/FT.AGGREGATE/FT.INFO/FT.DROPINDEX,
persistence across restarts, and error edge cases.

Add test_ft_alias_cluster.py with cluster-mode tests verifying alias
propagation, cross-node resolution, consistency after failover, and
MULTI/EXEC rejection in CME mode.

Add compatibility test infrastructure: generate_alias.py for golden
answer generation, alias-answers.pickle.gz baseline, and updated
data_sets.py and compatibility_test.py with alias test cases. Update
aggregate and text-search answer pickles for the alias_ field fix.

Signed-off-by: Riley Des <riley.desserre@improving.com>
Signed-off-by: Riley Des <riley.desserre@improving.com>
Signed-off-by: Riley Des <riley.desserre@improving.com>
Comment thread src/commands/ft_internal_update.cc
@AlexFilipImproving

Copy link
Copy Markdown

The code looks good, but for consistency with the rest of the codebase, can you combine the alias commands into a single ft_alias.cc file, like the other commands? That would be add, del, list and update. You can put the ft_alias_consistency.h file at the top so you don't have to include a separate .h and it will make it easier to search in the future.

Merge ft_alias_add.cc, ft_alias_del.cc, ft_alias_update.cc, and ft_alias_list.cc into one file. Extract shared logic into static helpers (RejectIfMultiExecInCme, FanoutAliasExists, FanoutAliasRemoved, ReplicateIfNeeded) to eliminate duplication.

Inline the consistency fanout classes from ft_alias_consistency.h
directly into ft_alias.cc and delete the header. Remove the coordinator-level unit tests (AliasConsistencyRequestTest, InfoResponseConsistencyTest) from the alias property test since they test cluster plumbing rather than alias behavior.

Signed-off-by: Riley Des <riley.desserre@improving.com>
Signed-off-by: Riley Des <riley.desserre@improving.com>
@rileydes-improving

Copy link
Copy Markdown
Author

The code looks good, but for consistency with the rest of the codebase, can you combine the alias commands into a single ft_alias.cc file, like the other commands? That would be add, del, list and update. You can put the ft_alias_consistency.h file at the top so you don't have to include a separate .h and it will make it easier to search in the future.

Made the change, all in one file now and centralized some of the functions.

Signed-off-by: Riley Des <riley.desserre@improving.com>
…sage

Signed-off-by: Riley Des <riley.desserre@improving.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants