Skip to content

feat(search_family): Add LOAD_FROM to the FT.AGGREGATE command #5703

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.

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

Conversation

BagritsevichStepan
Copy link
Contributor

@BagritsevichStepan BagritsevichStepan commented Aug 21, 2025

Join index_join with the FT.AGGREGATE command.

First PR is #5604

Most of the lines are from prev PR and also I added a lot of tests. Real number of lines is not so big

@romange
Copy link
Collaborator

romange commented Aug 21, 2025

it's huge. Is it possible to split it somehow?

@BagritsevichStepan
Copy link
Contributor Author

it's huge. Is it possible to split it somehow?

I left a comment in the PR description regarding the number of code lines. This is the second PR; once the first one is merged, the number of lines will be much lower.

Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
@BagritsevichStepan BagritsevichStepan force-pushed the search/ft-aggregate-load-from branch from 7e26a02 to 37ad2ee Compare August 24, 2025 07:25
@@ -27,7 +27,7 @@ using Value = ::dfly::search::SortableValue;

// DocValues sent through the pipeline
// TODO: Replace DocValues with compact linear search map instead of hash map
using DocValues = absl::flat_hash_map<std::string_view, Value>;
using DocValues = absl::flat_hash_map<std::string, Value>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is temporary and will be fixed in next PRs

dranikpg
dranikpg previously approved these changes Aug 24, 2025
Copy link
Contributor

@dranikpg dranikpg left a comment

Choose a reason for hiding this comment

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

Please make sure it doesn't misbehave with two load froms with the same index. Approach is ok. Left some small code comments

Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
@BagritsevichStepan
Copy link
Contributor Author

Please make sure it doesn't misbehave with two load froms with the same index. Approach is ok. Left some small code comments

Fixed and added this case to the tests

@BagritsevichStepan BagritsevichStepan enabled auto-merge (squash) August 24, 2025 15:00
@BagritsevichStepan BagritsevichStepan merged commit d1b5b12 into dragonflydb:main Aug 24, 2025
10 checks passed
@BagritsevichStepan BagritsevichStepan deleted the search/ft-aggregate-load-from branch August 24, 2025 15:08
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.

3 participants