Skip to content

Remove non-TEXT fields from the spec's keys dictionary - [MOD-13150]#7871

Merged
GuyAv46 merged 12 commits intomasterfrom
guyav-deprecate_fields_formatting
Dec 24, 2025
Merged

Remove non-TEXT fields from the spec's keys dictionary - [MOD-13150]#7871
GuyAv46 merged 12 commits intomasterfrom
guyav-deprecate_fields_formatting

Conversation

@GuyAv46
Copy link
Collaborator

@GuyAv46 GuyAv46 commented Dec 23, 2025

Describe the changes in the pull request

Today, when we want to get a non-text field index, we:

  1. Get the field spec by its name from the index spec.
  2. Generate a formatted string from the field name
  3. Look up the object in keysDict

In this PR, we move these objects from keysDict and set them directly in the field spec. This eliminates the need for steps 2 and 3.

In a following PR, we will simplify keysDict, specifically the keys formatting, which, after this PR, is no longer required.

Remove non-text fields from the spec's keysDict, which should only be used for text inverted indexes.

Deprecating IndexSpec_GetFormattedKey - should only be used to upgrade legacy indexes

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

Note

Shifts ownership of non-TEXT indexes from the global keysDict to per-field members on FieldSpec, simplifying lookups and lifecycle.

  • Introduces per-field storage: fs->tree (numeric/geo), fs->tagOpts.tagIndex, fs->vectorOpts.vecSimIndex, fs->geometryOpts.geometryIndex; updates FieldSpec_Cleanup to free them
  • Replaces key-based open helpers with field-based APIs: openNumericOrGeoIndex(...), TagIndex_Open(FieldSpec*, ...), openVectorIndex(FieldSpec*, ...), OpenGeometryIndex(FieldSpec*, ...)
  • Deprecates formatted key usage: adds IndexSpec_LegacyGetFormattedKey(...), renames helpers to HiddenString_LegacyDropFromKeySpace and Redis_LegacyDeleteKey; use legacy paths only for dropping/upgrading old indexes
  • Updates debug commands, GC, query, indexers, and info reporting to use new field-based access; removes geometry/numeric/tag key formatting and related dict entries
  • Adjusts tests and minor types (e.g., NumericRangeTree typedef) to new interfaces and lazy index creation behavior

Written by Cursor Bugbot for commit 3d02cf2. This will update automatically on new commits. Configure here.

@GuyAv46 GuyAv46 requested a review from ofiryanai December 23, 2025 16:37
@GuyAv46 GuyAv46 marked this pull request as ready for review December 23, 2025 16:37
@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

❌ Patch coverage is 97.05882% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.23%. Comparing base (e2fdca9) to head (3d02cf2).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/spec.c 85.71% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7871      +/-   ##
==========================================
+ Coverage   84.09%   84.23%   +0.13%     
==========================================
  Files         360      360              
  Lines       56289    56225      -64     
  Branches    15717    15717              
==========================================
+ Hits        47339    47362      +23     
+ Misses       8734     8647      -87     
  Partials      216      216              
Flag Coverage Δ
flow 85.23% <96.32%> (+0.18%) ⬆️
unit 52.04% <36.76%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ofiryanai
ofiryanai previously approved these changes Dec 24, 2025
Copy link
Contributor

@ofiryanai ofiryanai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few shallow comments, all in all good refactor

Copy link
Contributor

@ofiryanai ofiryanai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GuyAv46 GuyAv46 added this pull request to the merge queue Dec 24, 2025
} geometryOpts;
};

// TODO: Move into union above when we stop supporting multi-type fields
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GuyAv46 You mean fields with option FieldSpec_Dynamic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Referring here to the text-only data and the numeric pointer

Merged via the queue into master with commit 5668dd1 Dec 24, 2025
32 checks passed
@GuyAv46 GuyAv46 deleted the guyav-deprecate_fields_formatting branch December 24, 2025 13:11
@redisearch-backport-pull-request
Copy link
Contributor

Backport failed for 8.2, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.2
git worktree add -d .worktree/backport-7871-to-8.2 origin/8.2
cd .worktree/backport-7871-to-8.2
git switch --create backport-7871-to-8.2
git cherry-pick -x 5668dd1a51a6fd393c4f2721853ebd46528ff4e5

@redisearch-backport-pull-request
Copy link
Contributor

Backport failed for 8.4, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.4
git worktree add -d .worktree/backport-7871-to-8.4 origin/8.4
cd .worktree/backport-7871-to-8.4
git switch --create backport-7871-to-8.4
git cherry-pick -x 5668dd1a51a6fd393c4f2721853ebd46528ff4e5

@GuyAv46 GuyAv46 mentioned this pull request Dec 24, 2025
2 tasks
GuyAv46 added a commit that referenced this pull request Dec 24, 2025
…7871)

* move fields outside keysDict

* remove dead defs

* fix memory consumption test helper

* test fixes

* wip

* reverting unnecessary changes

* reverting unnecessary changes

* fix memory calculations

* fix memory calculations

* small improvement

* tidy ups

* review fixes

(cherry picked from commit 5668dd1)
GuyAv46 added a commit that referenced this pull request Dec 24, 2025
…7871)

* move fields outside keysDict

* remove dead defs

* fix memory consumption test helper

* test fixes

* wip

* reverting unnecessary changes

* reverting unnecessary changes

* fix memory calculations

* fix memory calculations

* small improvement

* tidy ups

* review fixes

(cherry picked from commit 5668dd1)
github-merge-queue bot pushed a commit that referenced this pull request Dec 28, 2025
…3150] (#7887)

* Remove non-TEXT fields from the spec's keys dictionary - [MOD-13150] (#7871)

* move fields outside keysDict

* remove dead defs

* fix memory consumption test helper

* test fixes

* wip

* reverting unnecessary changes

* reverting unnecessary changes

* fix memory calculations

* fix memory calculations

* small improvement

* tidy ups

* review fixes

(cherry picked from commit 5668dd1)

* fixes for 8.4
github-merge-queue bot pushed a commit that referenced this pull request Dec 29, 2025
…3150] (#7886)

* Remove non-TEXT fields from the spec's keys dictionary - [MOD-13150] (#7871)

* move fields outside keysDict

* remove dead defs

* fix memory consumption test helper

* test fixes

* wip

* reverting unnecessary changes

* reverting unnecessary changes

* fix memory calculations

* fix memory calculations

* small improvement

* tidy ups

* review fixes

(cherry picked from commit 5668dd1)

* fixes for 8.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants