Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Handle BigDecimal Ranges #190

Open
wants to merge 1 commit into from

3 participants

@dark-panda

Comparing a BigDecimal to Float::INFINITY throws a FloatDomainError: Infinity exception in ruby MRI 1.9+ and a TypeError on 1.8. This causes the comparisons in Arel::Predications#in and #not_in to fail when using BigDecimal values, which can often happen when using PostgreSQL and ActiveRecord, as AR casts numeric values in PostgreSQL to BigDecimal.

@tamird

@dark-panda can you rebase this please?

@dark-panda

Done.

@tamird

looks good to me.

@vipulnsward vipulnsward commented on the diff
test/visitors/test_to_sql.rb
@@ -386,6 +386,23 @@ def dispatch
compile(node).must_be_like %{1=1}
end
+ it 'can handle BigDecimal ranges bounded by infinity' do
+ node = @attr.in BigDecimal.new(1)..BigDecimal.new('Infinity')
+ compile(node).must_be_like %{
+ "users"."id" >= 1
+ }

We are not taking care of

node = @attr.in BigDecimal.new(1)...BigDecimal.new('Infinity')
          compile(node).must_be_like %{
            "users"."id" > 1
          }

The generated SQL would still be the same though wouldn't it? In your example you've changed >= to just >, but shouldn't the generated SQL still be >=? If so, I'll just add the test and re-push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 24, 2014
  1. @dark-panda
This page is out of date. Refresh to see the latest.
Showing with 45 additions and 6 deletions.
  1. +22 −6 lib/arel/predications.rb
  2. +23 −0 test/visitors/test_to_sql.rb
View
28 lib/arel/predications.rb
@@ -29,15 +29,15 @@ def in other
when Arel::SelectManager
Arel::Nodes::In.new(self, other.ast)
when Range
- if other.begin == -Float::INFINITY
- if other.end == Float::INFINITY
+ if negative_infinite?(other.begin)
+ if positive_infinite?(other.end)
Nodes::NotIn.new self, []
elsif other.exclude_end?
Nodes::LessThan.new(self, Nodes.build_quoted(other.end, self))
else
Nodes::LessThanOrEqual.new(self, Nodes.build_quoted(other.end, self))
end
- elsif other.end == Float::INFINITY
+ elsif positive_infinite?(other.end)
Nodes::GreaterThanOrEqual.new(self, Nodes.build_quoted(other.begin, self))
elsif other.exclude_end?
left = Nodes::GreaterThanOrEqual.new(self, Nodes.build_quoted(other.begin, self))
@@ -66,15 +66,15 @@ def not_in other
when Arel::SelectManager
Arel::Nodes::NotIn.new(self, other.ast)
when Range
- if other.begin == -Float::INFINITY # The range begins with negative infinity
- if other.end == Float::INFINITY
+ if negative_infinite?(other.begin) # The range begins with negative infinity
+ if positive_infinite?(other.end)
Nodes::In.new self, [] # The range is infinite, so return an empty range
elsif other.exclude_end?
Nodes::GreaterThanOrEqual.new(self, Nodes.build_quoted(other.end, self))
else
Nodes::GreaterThan.new(self, Nodes.build_quoted(other.end, self))
end
- elsif other.end == Float::INFINITY
+ elsif positive_infinite?(other.end)
Nodes::LessThan.new(self, Nodes.build_quoted(other.begin, self))
else
left = Nodes::LessThan.new(self, Nodes.build_quoted(other.begin, self))
@@ -185,5 +185,21 @@ def grouping_all method_id, others, *extras
nodes = others.map {|expr| send(method_id, expr, *extras)}
Nodes::Grouping.new Nodes::And.new(nodes)
end
+
+ def positive_infinite? value
+ if value.respond_to? :infinite?
+ value.infinite? == 1
+ else
+ value == Float::INFINITY
+ end
+ end
+
+ def negative_infinite? value
+ if value.respond_to? :infinite?
+ value.infinite? == -1
+ else
+ value == -Float::INFINITY
+ end
+ end
end
end
View
23 test/visitors/test_to_sql.rb
@@ -405,6 +405,29 @@ def dispatch
compile(node).must_be_like %{1=1}
end
+ it 'can handle BigDecimal ranges bounded by infinity' do
+ node = @attr.in BigDecimal.new(1)..BigDecimal.new('Infinity')
+ compile(node).must_be_like %{
+ "users"."id" >= 1
+ }

We are not taking care of

node = @attr.in BigDecimal.new(1)...BigDecimal.new('Infinity')
          compile(node).must_be_like %{
            "users"."id" > 1
          }

The generated SQL would still be the same though wouldn't it? In your example you've changed >= to just >, but shouldn't the generated SQL still be >=? If so, I'll just add the test and re-push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ node = @attr.in BigDecimal.new(1)...BigDecimal.new('Infinity')
+ compile(node).must_be_like %{
+ "users"."id" >= 1
+ }
+ node = @attr.in BigDecimal.new('-Infinity')..BigDecimal.new(3)
+ compile(node).must_be_like %{
+ "users"."id" <= 3
+ }
+ node = @attr.in BigDecimal.new('-Infinity')...BigDecimal.new(3)
+ compile(node).must_be_like %{
+ "users"."id" < 3
+ }
+ node = @attr.in BigDecimal.new('-Infinity')..BigDecimal.new('Infinity')
+ compile(node).must_be_like %{1=1}
+ node = @attr.in BigDecimal.new('-Infinity')...BigDecimal.new('Infinity')
+ compile(node).must_be_like %{1=1}
+ end
+
it 'can handle subqueries' do
table = Table.new(:users)
subquery = table.project(:id).where(table[:name].eq('Aaron'))
Something went wrong with that request. Please try again.