chanzuckerberg / redis-memo

A Redis-based version addressable caching system. Memoize pure functions, aggregated database queries, and 3rd party API calls.
https://rubygems.org/gems/redis-memo
MIT License
33 stars 3 forks source link

[feature] support more queries with range (>=, <= etc) #63

Closed Dingying0410 closed 3 years ago

Dingying0410 commented 3 years ago

Summary

This PR is to support the queries that have range in it. such as '>=', '<=', etc., and also combine the NotEqual query we had in #62 since they can all be merged into the NodeHasComparator condition in this PR.

Regarding the rspec, I created several shared examples so we can test multiple similar behaviors.

Details

Previously we decided to not to cache any queries with these comparison operators, however, queries with both comparison operators and other bound queries can be cached. E.g

Site.where(a: 1).where('b > ?' 1) can be cached based on a as dependency 
Site.where('b > ?' 1) should not be cached

Also, as according to here, there are two forms of queries with comparator:

  1. Site.where(a: 1).where('b > ?' 1) -- this will be analyzed as Arel::Nodes::Grouping when we parse the children of the wheres, thus, we look at node.expr of it. node.expr is usually a Arel::Nodes::SqlLiteral in this case
  2. Site.where(a: 1).where(b: 2..Float::INFINITY) -- this will be analyzed as Arel::Nodes::LessThan when we parse the children of the wheres, thus, we categorize Arel::Nodes::LessThan and also other node class as NodeHasComparators

My current approach is to return bind_params when we parse these statements

Test plan

TODO:

remove the duplicate Arel::Nodes::NotEqual block in #62 since they are brought in this PR

codecov[bot] commented 3 years ago

Codecov Report

Merging #63 (ae61aca) into main (ee26d55) will increase coverage by 0.05%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
+ Coverage   97.37%   97.42%   +0.05%     
==========================================
  Files          34       34              
  Lines        2209     2252      +43     
==========================================
+ Hits         2151     2194      +43     
  Misses         58       58              
Impacted Files Coverage Δ
lib/redis_memo/memoize_query/cached_select.rb 93.28% <100.00%> (-0.05%) :arrow_down:
spec/memoize_query_spec.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ee26d55...ae61aca. Read the comment docs.

donaldong commented 3 years ago

The build failure seems legit btw