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

Allow filtering Redis list items on read #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ruksi
Copy link
Member

@ruksi ruksi commented Jan 30, 2025

I was pondering one of our use cases about limited reading of big lists, would be nice to be able to choose which lines to fetch

let me know if this is not a good place for this, could also be in the... project that actually needs it instead of in Minique 🤔

but this would implement it for that possible use case!

@ruksi ruksi requested review from akx and hylje January 30, 2025 12:13
@ruksi ruksi self-assigned this Jan 30, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.70%. Comparing base (e4443b3) to head (bdc9d20).

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage   91.11%   91.70%   +0.59%     
==========================================
  Files          25       25              
  Lines         855      880      +25     
  Branches      115       82      -33     
==========================================
+ Hits          779      807      +28     
+ Misses         45       44       -1     
+ Partials       31       29       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

assert list(last_four) == [b"2", b"3", b"4", b"5"]

last_everything = read_list(redis, random_queue_name, last_n=999_999)
assert list(last_everything) == [b"0", b"1", b"2", b"3", b"4", b"5"]
Copy link

Choose a reason for hiding this comment

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

these are all spanning just one chunk, does it behave correctly if it requests multiple chunks? I have a hunch that it's not properly getting the last_n if it has to span multiple chunks, as while it iterates each chunk in reverse order, chunks iterate in forward order, so the last N of the first chunk are not the last N of the entire list

Copy link
Member Author

Choose a reason for hiding this comment

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

I can test!

Copy link
Member Author

Choose a reason for hiding this comment

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

image

yes

Copy link
Member Author

@ruksi ruksi Jan 30, 2025

Choose a reason for hiding this comment

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

lrange            = [3, 4]  [1, 2] (chunks read from the end of the list)
reversed(chunk)   = [4, 3]  [2, 1]
result            = [4, 3, 2, 1]   (assuming no filtering, append as long as last_n)
reversed(results) = [1, 2, 3, 4]

Copy link

Choose a reason for hiding this comment

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

oh yeah, the start index calculation means it's starting from the end so the chunks are iterated back to front

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