Skip to content

Conversation

@CascadingRadium
Copy link
Member

@CascadingRadium CascadingRadium commented Dec 9, 2025

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors IndexInternalID usage by migrating from raw byte operations to dedicated API methods provided by bleve_index_api. The changes improve code maintainability and consistency by using proper API abstractions instead of direct byte manipulation.

Key changes:

  • Replaces byte comparison operations with IndexInternalID methods (Compare(), Equals())
  • Migrates ID construction from byte append operations to NewIndexInternalID() and NewIndexInternalIDFrom() functions
  • Removes internal helper functions (docNumberToBytes, docInternalToNumber) now provided by the API

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
search/searcher/search_numeric_range.go Updates term comparison to use idiomatic bytes.Equal()
search/searcher/search_disjunction_heap.go Migrates ID comparisons to use IndexInternalID.Equals() and Compare() methods, removes unused bytes import
search/scorer/scorer_term.go Updates ID assignment to use NewIndexInternalIDFrom(), includes minor comment formatting improvement
search/scorer/scorer_knn.go Updates ID assignment to use NewIndexInternalIDFrom()
index/scorch/snapshot_index_vr.go Migrates to use NewIndexInternalID(), ID.Value(), and ID.Compare() methods, removes unused bytes import
index/scorch/snapshot_index_tfr.go Migrates to use NewIndexInternalID(), ID.Value(), and ID.Compare() methods, removes unused bytes import
index/scorch/snapshot_index_doc.go Migrates to use NewIndexInternalID() and ID.Compare() methods, removes unused bytes import
index/scorch/snapshot_index.go Migrates to use ID.Value() method, removes helper functions now provided by API, removes unused imports, includes error message capitalization fixes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@abhinavdangeti abhinavdangeti added this to the v2.6.0 milestone Dec 9, 2025
@CascadingRadium CascadingRadium changed the title Refactor IndexInternalID usage Refactor IndexInternalID usage Dec 10, 2025
@Likith101
Copy link
Member

I remember reviewing this within the nested fields prs. Why does this require an additional commit? Especially considering these changes were first made to make it easier for nested fields and it also requires bleve_index_api changes as well?

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.

4 participants