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.

Sign up for GitHub

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

Jump to bottom

Removes need to maintain a Redis Set for Primary Keys #118

Merged
merged 14 commits into from
Nov 2, 2022
Merged

Removes need to maintain a Redis Set for Primary Keys #118

merged 14 commits into from
Nov 2, 2022

Conversation

bsbodden
Copy link
Contributor

Spring Data Redis uses a Redis Set to keep primary keys for a mapped entity and perform operations related to primary keys. The PR removes the PK set and rewrites all dependent operations using Search Queries or Aggregations. A side-effect of this refactoring is that now ALL mapped entities with @RedisHash or @Document will have an associated index.

slorello89
slorello89 requested changes Oct 31, 2022
View reviewed changes
Copy link
Member

@slorello89 slorello89 left a comment

Choose a reason for hiding this comment

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

Some suggestions as to how to make these operations more performant and straight forward - it's entirely possible I'm missing some context on why the hash operations are being enumerated the way they are?

redis-om-spring/src/main/java/com/redis/om/spring/RedisEnhancedKeyValueAdapter.java Outdated Show resolved Hide resolved
redis-om-spring/src/main/java/com/redis/om/spring/RedisEnhancedKeyValueAdapter.java Outdated Show resolved Hide resolved
redis-om-spring/src/main/java/com/redis/om/spring/RedisEnhancedKeyValueAdapter.java Outdated Show resolved Hide resolved
redis-om-spring/src/main/java/com/redis/om/spring/RedisEnhancedKeyValueAdapter.java Outdated Show resolved Hide resolved
redis-om-spring/src/main/java/com/redis/om/spring/RedisEnhancedKeyValueAdapter.java Show resolved Hide resolved
slorello89
slorello89 approved these changes Nov 2, 2022
View reviewed changes
Copy link
Member

@slorello89 slorello89 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@bsbodden bsbodden merged commit a144039 into redis:main Nov 2, 2022
@bsbodden bsbodden deleted the bsb/sans-id-sets branch November 22, 2022 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slorello89 slorello89 slorello89 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bsbodden @slorello89