Skip to content
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

avoid deep recursions by introducing multiple RSQNodes under a single… #1275

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

swilly22
Copy link
Contributor

@swilly22 swilly22 commented Aug 5, 2020

This PR modifies the way we construct RediSearch queries for IN filters,
Consider: MATCH (n) WHERE n.v IN [0,1,2,3,4,5,6,7,8,9] RETURN n
This used to be translated in to a nested OR filter:
n.v = 0 OR n.v = 1 OR n.v = 2 OR n.v = 3 OR ...
and similarly a nested UNION RediSearch query, causing deep recursions as can be seen in the following image
image (3)

This PR takes advantage of multi child UNION RediSearch query node(s) which reduces the depth of the query tree.

@MeirShpilraien
Copy link
Collaborator

@swilly22 the search part looks good to me, I am less familiar with the rest of the changes...

Copy link
Contributor

@jeffreylovitz jeffreylovitz left a comment

Choose a reason for hiding this comment

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

The utilize_indices logic all looks sound to me, and @MeirShpilraien has signed off on the RediSearch components, so this seems good!

@swilly22 swilly22 merged commit 4efb34b into master Aug 6, 2020
@swilly22 swilly22 deleted the compact_IN_index_query branch August 6, 2020 05:16
pnxguide pushed a commit to CMU-SPEED/RedisGraph that referenced this pull request Mar 22, 2023
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.

3 participants