fractaledmind / activeset

A toolkit for working with enumerable sets.

MIT License

2 stars 1 forks source link

Ensure that ActiveRecord sorting treats NULL values in the same way a… #43

Closed fractaledmind closed 5 years ago

fractaledmind commented 5 years ago

…s Enumerable sorting, where NULL values are sorted as if larger than any non-null value

codecov[bot] commented 5 years ago

Codecov Report

Merging #43 into master will increase coverage by <.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
+ Coverage   99.76%   99.76%   +<.01%     
==========================================
  Files          17       17              
  Lines         419      420       +1     
==========================================
+ Hits          418      419       +1     
  Misses          1        1

Impacted Files	Coverage Δ
lib/active_set/sorting/active_record_strategy.rb	`100% <100%> (ø)`	:arrow_up:

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 53a042d...f5286d8. Read the comment docs.

fractaledmind commented 5 years ago

@riyengar8: Be sure to check this PR out. This standardizes how sorting with NULL values is handled in ActiveRecord, across all DB engines. As laid out in this S/O answer and your original Enumerable PR, different SQL engines handle NULL sorting differently. This is a pain for a few reasons, but most notably in testing. I want ActiveSet to behave in an ordeal fashion, which means that sorting should have the same output regardless of how that instruction is processed.

riyengar8 commented 5 years ago

Perhaps at some point in the future, the "heavy nulls" behavior here should be a configuration option (the user may desire the opposite). It can be tied with the same behavior in Enumerable sets for consistency (such that the same configuration option effects both types of sets in the same way). Having such an option also affords a convenient piece of documentation for the behavior. If this seems like a good idea to you I can open a pull request.

fractaledmind commented 5 years ago

That seems like a great idea