Skip to content

Adding suffix 'field' to classes inheriting IndexBase #296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adding suffix 'field' to classes inheriting IndexBase #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c412ba
Adding suffix 'field' to classes inheriting IndexBase
Aug 8, 2025
339f848
DCO Remediation Commit for Aksha Thakkar <[email protected]>
Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/commands/filter_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ FilterParser::ParseNumericPredicate(const std::string& attribute_alias) {
pos_));
}
auto numeric_index =
dynamic_cast<const indexes::Numeric*>(index.value().get());
dynamic_cast<const indexes::NumericField*>(index.value().get());
return std::make_unique<query::NumericPredicate>(
numeric_index, attribute_alias, identifier, start, is_inclusive_start,
end, is_inclusive_end);
Expand All @@ -218,8 +218,8 @@ absl::StatusOr<absl::string_view> FilterParser::ParseTagString() {
}

absl::StatusOr<absl::flat_hash_set<absl::string_view>> FilterParser::ParseTags(
absl::string_view tag_string, indexes::Tag* tag_index) const {
return indexes::Tag::ParseSearchTags(tag_string, tag_index->GetSeparator());
absl::string_view tag_string, indexes::TagField* tag_index) const {
return indexes::TagField::ParseSearchTags(tag_string, tag_index->GetSeparator());
}

absl::StatusOr<std::unique_ptr<query::TagPredicate>>
Expand All @@ -233,7 +233,7 @@ FilterParser::ParseTagPredicate(const std::string& attribute_alias) {
auto identifier = index_schema_.GetIdentifier(attribute_alias).value();
filter_identifiers_.insert(identifier);

auto tag_index = dynamic_cast<indexes::Tag*>(index.value().get());
auto tag_index = dynamic_cast<indexes::TagField*>(index.value().get());
VMSDK_ASSIGN_OR_RETURN(auto tag_string, ParseTagString());
VMSDK_ASSIGN_OR_RETURN(auto parsed_tags, ParseTags(tag_string, tag_index));
return std::make_unique<query::TagPredicate>(
Expand Down
4 changes: 2 additions & 2 deletions src/commands/filter_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace valkey_search {
namespace indexes {
class Tag;
class TagField;
} // namespace indexes
struct FilterParseResults {
std::unique_ptr<query::Predicate> root_predicate;
Expand Down Expand Up @@ -61,7 +61,7 @@ class FilterParser {
absl::StatusOr<absl::string_view> ParseTagString();

absl::StatusOr<absl::flat_hash_set<absl::string_view>> ParseTags(
absl::string_view tag_string, indexes::Tag* tag_index) const;
absl::string_view tag_string, indexes::TagField* tag_index) const;
};

namespace options {
Expand Down
4 changes: 2 additions & 2 deletions src/coordinator/search_converter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ absl::StatusOr<std::unique_ptr<query::Predicate>> GRPCPredicateToPredicate(
auto identifier,
index_schema->GetIdentifier(predicate.tag().attribute_alias()));
attribute_identifiers.insert(identifier);
auto tag_index = dynamic_cast<indexes::Tag*>(index.get());
auto tag_index = dynamic_cast<indexes::TagField*>(index.get());
VMSDK_ASSIGN_OR_RETURN(
auto parsed_tags,
tag_index->ParseSearchTags(predicate.tag().raw_tag_string(),
Expand All @@ -70,7 +70,7 @@ absl::StatusOr<std::unique_ptr<query::Predicate>> GRPCPredicateToPredicate(
auto identifier,
index_schema->GetIdentifier(predicate.numeric().attribute_alias()));
attribute_identifiers.insert(identifier);
auto numeric_index = dynamic_cast<indexes::Numeric*>(index.get());
auto numeric_index = dynamic_cast<indexes::NumericField*>(index.get());
auto numeric_predicate = std::make_unique<query::NumericPredicate>(
numeric_index, predicate.numeric().attribute_alias(), identifier,
predicate.numeric().start(), predicate.numeric().is_inclusive_start(),
Expand Down
18 changes: 9 additions & 9 deletions src/index_schema.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ absl::StatusOr<std::shared_ptr<indexes::IndexBase>> IndexFactory(
const auto &index = attribute.index();
switch (index.index_type_case()) {
case data_model::Index::IndexTypeCase::kTagIndex: {
return std::make_shared<indexes::Tag>(index.tag_index());
return std::make_shared<indexes::TagField>(index.tag_index());
}
case data_model::Index::IndexTypeCase::kNumericIndex: {
return std::make_shared<indexes::Numeric>(index.numeric_index());
return std::make_shared<indexes::NumericField>(index.numeric_index());
}
case data_model::Index::IndexTypeCase::kVectorIndex: {
switch (index.vector_index().algorithm_case()) {
Expand All @@ -90,11 +90,11 @@ absl::StatusOr<std::shared_ptr<indexes::IndexBase>> IndexFactory(
VMSDK_ASSIGN_OR_RETURN(
auto index,
(iter.has_value())
? indexes::VectorHNSW<float>::LoadFromRDB(
? indexes::VectorHNSWField<float>::LoadFromRDB(
ctx, &index_schema->GetAttributeDataType(),
index.vector_index(), attribute.identifier(),
std::move(*iter))
: indexes::VectorHNSW<float>::Create(
: indexes::VectorHNSWField<float>::Create(
index.vector_index(), attribute.identifier(),
index_schema->GetAttributeDataType().ToProto()));
index_schema->SubscribeToVectorExternalizer(
Expand All @@ -115,11 +115,11 @@ absl::StatusOr<std::shared_ptr<indexes::IndexBase>> IndexFactory(
VMSDK_ASSIGN_OR_RETURN(
auto index,
(iter.has_value())
? indexes::VectorFlat<float>::LoadFromRDB(
? indexes::VectorFlatField<float>::LoadFromRDB(
ctx, &index_schema->GetAttributeDataType(),
index.vector_index(), attribute.identifier(),
std::move(*iter))
: indexes::VectorFlat<float>::Create(
: indexes::VectorFlatField<float>::Create(
index.vector_index(), attribute.identifier(),
index_schema->GetAttributeDataType().ToProto()));
index_schema->SubscribeToVectorExternalizer(
Expand Down Expand Up @@ -855,7 +855,7 @@ absl::Status IndexSchema::RDBSave(SafeRDB *rdb) const {
<< this->name_ << " attribute: " << attribute.first << " to RDB";
RDBChunkOutputStream key_to_id_chunked_out(rdb);
VMSDK_RETURN_IF_ERROR(
dynamic_cast<const indexes::VectorBase *>(
dynamic_cast<const indexes::VectorBaseField *>(
attribute.second.GetIndex().get())
->SaveTrackedKeys(std::move(key_to_id_chunked_out)))
<< "IO error while saving Key to ID mapping (index name: "
Expand Down Expand Up @@ -909,7 +909,7 @@ absl::StatusOr<std::shared_ptr<IndexSchema>> IndexSchema::LoadFromRDB(
"(index: %s, attribute: %s)",
index_schema->GetName(), attribute.alias()));
}
auto vector_index = dynamic_cast<indexes::VectorBase *>(index.get());
auto vector_index = dynamic_cast<indexes::VectorBaseField *>(index.get());
VMSDK_RETURN_IF_ERROR(vector_index->LoadTrackedKeys(
ctx, &index_schema->GetAttributeDataType(),
supplemental_iter.IterateChunks()));
Expand Down Expand Up @@ -1092,7 +1092,7 @@ size_t IndexSchema::GetMutatedRecordsSize() const {
}

void IndexSchema::SubscribeToVectorExternalizer(
absl::string_view attribute_identifier, indexes::VectorBase *vector_index) {
absl::string_view attribute_identifier, indexes::VectorBaseField *vector_index) {
vector_externalizer_subscriptions_[attribute_identifier] = vector_index;
}

Expand Down
4 changes: 2 additions & 2 deletions src/index_schema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class IndexSchema : public KeyspaceEventSubscription,
void MarkAsDestructing();
void ProcessMultiQueue();
void SubscribeToVectorExternalizer(absl::string_view attribute_identifier,
indexes::VectorBase *vector_index);
indexes::VectorBaseField *vector_index);

protected:
IndexSchema(ValkeyModuleCtx *ctx,
Expand Down Expand Up @@ -190,7 +190,7 @@ class IndexSchema : public KeyspaceEventSubscription,
};

vmsdk::MainThreadAccessGuard<std::optional<BackfillJob>> backfill_job_;
absl::flat_hash_map<std::string, indexes::VectorBase *>
absl::flat_hash_map<std::string, indexes::VectorBaseField *>
vector_externalizer_subscriptions_;
void VectorExternalizer(const InternedStringPtr &key,
absl::string_view attribute_identifier,
Expand Down
46 changes: 23 additions & 23 deletions src/indexes/numeric.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ std::optional<double> ParseNumber(absl::string_view data) {
}
} // namespace

Numeric::Numeric(const data_model::NumericIndex& numeric_index_proto)
NumericField::NumericField(const data_model::NumericIndex& numeric_index_proto)
: IndexBase(IndexerType::kNumeric) {
index_ = std::make_unique<BTreeNumericIndex>();
}

absl::StatusOr<bool> Numeric::AddRecord(const InternedStringPtr& key,
absl::string_view data) {
absl::StatusOr<bool> NumericField::AddRecord(const InternedStringPtr& key,
absl::string_view data) {
auto value = ParseNumber(data);
absl::MutexLock lock(&index_mutex_);
if (!value.has_value()) {
Expand All @@ -61,8 +61,8 @@ absl::StatusOr<bool> Numeric::AddRecord(const InternedStringPtr& key,
return true;
}

absl::StatusOr<bool> Numeric::ModifyRecord(const InternedStringPtr& key,
absl::string_view data) {
absl::StatusOr<bool> NumericField::ModifyRecord(const InternedStringPtr& key,
absl::string_view data) {
auto value = ParseNumber(data);
if (!value.has_value()) {
[[maybe_unused]] auto res =
Expand All @@ -81,8 +81,8 @@ absl::StatusOr<bool> Numeric::ModifyRecord(const InternedStringPtr& key,
return true;
}

absl::StatusOr<bool> Numeric::RemoveRecord(const InternedStringPtr& key,
DeletionType deletion_type) {
absl::StatusOr<bool> NumericField::RemoveRecord(const InternedStringPtr& key,
DeletionType deletion_type) {
absl::MutexLock lock(&index_mutex_);
if (deletion_type == DeletionType::kRecord) {
// If key is DELETED, remove it from untracked_keys_.
Expand All @@ -101,7 +101,7 @@ absl::StatusOr<bool> Numeric::RemoveRecord(const InternedStringPtr& key,
return true;
}

int Numeric::RespondWithInfo(ValkeyModuleCtx* ctx) const {
int NumericField::RespondWithInfo(ValkeyModuleCtx* ctx) const {
ValkeyModule_ReplyWithSimpleString(ctx, "type");
ValkeyModule_ReplyWithSimpleString(ctx, "NUMERIC");
ValkeyModule_ReplyWithSimpleString(ctx, "size");
Expand All @@ -111,19 +111,19 @@ int Numeric::RespondWithInfo(ValkeyModuleCtx* ctx) const {
return 4;
}

bool Numeric::IsTracked(const InternedStringPtr& key) const {
bool NumericField::IsTracked(const InternedStringPtr& key) const {
absl::MutexLock lock(&index_mutex_);
return tracked_keys_.contains(key);
}

std::unique_ptr<data_model::Index> Numeric::ToProto() const {
std::unique_ptr<data_model::Index> NumericField::ToProto() const {
auto index_proto = std::make_unique<data_model::Index>();
auto numeric_index = std::make_unique<data_model::NumericIndex>();
index_proto->set_allocated_numeric_index(numeric_index.release());
return index_proto;
}

const double* Numeric::GetValue(const InternedStringPtr& key) const {
const double* NumericField::GetValue(const InternedStringPtr& key) const {
// Note that the Numeric index is not mutated while the time sliced mutex is
// in a read mode and therefor it is safe to skip lock acquiring.
if (auto it = tracked_keys_.find(key); it != tracked_keys_.end()) {
Expand All @@ -132,7 +132,7 @@ const double* Numeric::GetValue(const InternedStringPtr& key) const {
return nullptr;
}

std::unique_ptr<Numeric::EntriesFetcher> Numeric::Search(
std::unique_ptr<NumericField::EntriesFetcher> NumericField::Search(
const query::NumericPredicate& predicate, bool negate) const {
EntriesRange entries_range;
const auto& btree = index_->GetBtree();
Expand All @@ -153,7 +153,7 @@ std::unique_ptr<Numeric::EntriesFetcher> Numeric::Search(
: btree.lower_bound(predicate.GetEnd());
;
additional_entries_range.second = btree.end();
return std::make_unique<Numeric::EntriesFetcher>(
return std::make_unique<NumericField::EntriesFetcher>(
entries_range, size + untracked_keys_.size(), additional_entries_range,
&untracked_keys_);
}
Expand All @@ -167,11 +167,11 @@ std::unique_ptr<Numeric::EntriesFetcher> Numeric::Search(
size_t size = index_->GetCount(predicate.GetStart(), predicate.GetEnd(),
predicate.IsStartInclusive(),
predicate.IsEndInclusive());
return std::make_unique<Numeric::EntriesFetcher>(entries_range, size);
return std::make_unique<NumericField::EntriesFetcher>(entries_range, size);
}

bool Numeric::EntriesFetcherIterator::NextKeys(
const Numeric::EntriesRange& range, BTreeNumericIndex::ConstIterator& iter,
bool NumericField::EntriesFetcherIterator::NextKeys(
const NumericField::EntriesRange& range, BTreeNumericIndex::ConstIterator& iter,
std::optional<InternedStringSet::const_iterator>& keys_iter) {
while (iter != range.second) {
if (!keys_iter.has_value()) {
Expand All @@ -188,7 +188,7 @@ bool Numeric::EntriesFetcherIterator::NextKeys(
return false;
}

Numeric::EntriesFetcherIterator::EntriesFetcherIterator(
NumericField::EntriesFetcherIterator::EntriesFetcherIterator(
const EntriesRange& entries_range,
const std::optional<EntriesRange>& additional_entries_range,
const InternedStringSet* untracked_keys)
Expand All @@ -201,7 +201,7 @@ Numeric::EntriesFetcherIterator::EntriesFetcherIterator(
}
}

bool Numeric::EntriesFetcherIterator::Done() const {
bool NumericField::EntriesFetcherIterator::Done() const {
return entries_iter_ == entries_range_.second &&
(!additional_entries_range_.has_value() ||
additional_entries_iter_ ==
Expand All @@ -211,7 +211,7 @@ bool Numeric::EntriesFetcherIterator::Done() const {
untracked_keys_iter_ == untracked_keys_->end()));
}

void Numeric::EntriesFetcherIterator::Next() {
void NumericField::EntriesFetcherIterator::Next() {
if (NextKeys(entries_range_, entries_iter_, entry_keys_iter_)) {
return;
}
Expand All @@ -229,7 +229,7 @@ void Numeric::EntriesFetcherIterator::Next() {
}
}

const InternedStringPtr& Numeric::EntriesFetcherIterator::operator*() const {
const InternedStringPtr& NumericField::EntriesFetcherIterator::operator*() const {
if (entries_iter_ != entries_range_.second) {
DCHECK(entry_keys_iter_ != entries_iter_->second.end());
return *entry_keys_iter_.value();
Expand All @@ -245,16 +245,16 @@ const InternedStringPtr& Numeric::EntriesFetcherIterator::operator*() const {
return *untracked_keys_iter_.value();
}

size_t Numeric::EntriesFetcher::Size() const { return size_; }
size_t NumericField::EntriesFetcher::Size() const { return size_; }

std::unique_ptr<EntriesFetcherIteratorBase> Numeric::EntriesFetcher::Begin() {
std::unique_ptr<EntriesFetcherIteratorBase> NumericField::EntriesFetcher::Begin() {
auto itr = std::make_unique<EntriesFetcherIterator>(
entries_range_, additional_entries_range_, untracked_keys_);
itr->Next();
return itr;
}

uint64_t Numeric::GetRecordCount() const {
uint64_t NumericField::GetRecordCount() const {
absl::MutexLock lock(&index_mutex_);
return tracked_keys_.size();
}
Expand Down
6 changes: 3 additions & 3 deletions src/indexes/numeric.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ class BTreeNumeric {
utils::SegmentTree segment_tree_;
};

class Numeric : public IndexBase {
class NumericField : public IndexBase {
public:
explicit Numeric(const data_model::NumericIndex& numeric_index_proto);
explicit NumericField(const data_model::NumericIndex& numeric_index_proto);
absl::StatusOr<bool> AddRecord(const InternedStringPtr& key,
absl::string_view data) override
ABSL_LOCKS_EXCLUDED(index_mutex_);
Expand Down Expand Up @@ -126,7 +126,7 @@ class Numeric : public IndexBase {

private:
static bool NextKeys(
const Numeric::EntriesRange& range,
const NumericField::EntriesRange& range,
BTreeNumericIndex::ConstIterator& iter,
std::optional<InternedStringSet::const_iterator>& keys_iter);
const EntriesRange& entries_range_;
Expand Down
Loading
Loading