Repository navigation
refactor vcache and move loader to bsup/loader - #7428
Merged
Merged
Conversation
nwt
approved these changes
Oct 8, 2026
| // the super.Context in use is passed in for each vector constructed from | ||
| // its in-memory, sctx-independent shadow. This way one shadow can be used | ||
| // across multiple queries with different sctx. | ||
| type Frame struct { |
Collaborator
There was a problem hiding this comment.
Nit: I might not be seeing the big picture here but I think calling this FrameLoader would be clearer. Right you've got
loader := loader.NewFrame(colFrame)which feels off because it looks like you're creating a frame and assigning it to loader. I think this looks more natural:
loader := loader.NewFrameLoader(colFrame)| // storage and cached in memory so that subsequent calls run from memory. | ||
| // The vectors returned will have types from the provided sctx. Multiple | ||
| // Fetch calls to the same object may run concurrently. | ||
| func (f *Frame) Fetch(sctx *super.Context, projection field.Projection) (vector.Any, error) { |
Collaborator
There was a problem hiding this comment.
Nit: Feels like this should be called Load.
Comment on lines
+33
to
+35
| loader := &loader{cctx, sctx, f.frame.DataReader()} | ||
| f.root = newShadow(cctx, f.frame.Root()) | ||
| f.root.unmarshal(cctx, projection) |
Collaborator
There was a problem hiding this comment.
Nit: Do this in the same order as in FetrchUnordered.
Suggested change
| loader := &loader{cctx, sctx, f.frame.DataReader()} | |
| f.root = newShadow(cctx, f.frame.Root()) | |
| f.root.unmarshal(cctx, projection) | |
| f.root = newShadow(cctx, f.frame.Root()) | |
| f.root.unmarshal(cctx, projection) | |
| loader := &loader{cctx, sctx, f.frame.DataReader()} |
This moves the bsup loader code below the bsup package where it belongs and refactors the design so that column frames are exposed as one or more entities within a storage object. We stubbed out vcache a bit more, which is not yet used in the system (only for tests) but will become a pivotal part of the database storage layer. Except for the small bits of refactoring, this commit is just moving and renaming things.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This moves the bsup loader code below the bsup package where it belongs and refactors the design so that column frames are exposed as one or more entities within a storage object. We stubbed out vcache a bit more, which is not yet used in the system (only for tests) but will become a pivotal part of the database storage layer.
Except for the small bits of refactoring, this commit is just moving and renaming things.