Skip to content
This repository

Bug when aggregating on field that doesn't always exist #2334

Merged
merged 2 commits into from over 1 year ago

3 participants

James McKinney Don't Add Me To Your Organization a.k.a The Travis Bot Durran Jordan
James McKinney

When a field only sometimes exists, the aggregates behave oddly. In both the specs, 50 should be the min. But in one case it's nil and in another it's 100 (!).

James McKinney

Testing that the field is not null in the reduce function gets the specs to pass.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 30dbb21 into 68f4555).

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 418ae6e into 68f4555).

James McKinney

Hmm, the JRuby failures seem unrelated.

Durran Jordan
Owner

Sweet - thanks! (Yes the failures are unrelated, pulling in)

Durran Jordan durran merged commit 7fe4b58 into from September 01, 2012
Durran Jordan durran closed this September 01, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
8  lib/mongoid/contextual/aggregable/mongo.rb
@@ -167,10 +167,12 @@ def reducer
167 167
           function(key, values) {
168 168
             var agg = { count: 0, max: null, min: null, sum: 0 };
169 169
             values.forEach(function(val) {
170  
-              if (agg.max == null || val.max > agg.max) agg.max = val.max;
171  
-              if (agg.min == null || val.max < agg.min) agg.min = val.max;
  170
+              if (val.max !== null) {
  171
+                if (agg.max == null || val.max > agg.max) agg.max = val.max;
  172
+                if (agg.min == null || val.max < agg.min) agg.min = val.max;
  173
+                agg.sum += val.sum;
  174
+              }
172 175
               agg.count += 1;
173  
-              agg.sum += val.sum;
174 176
             });
175 177
             return agg;
176 178
           }}
50  spec/mongoid/contextual/aggregable/mongo_spec.rb
@@ -110,6 +110,56 @@
110 110
         end
111 111
       end
112 112
 
  113
+      context "when the field sometimes exists" do
  114
+        let!(:oasis) do
  115
+          Band.create(name: "Oasis", likes: 50)
  116
+        end
  117
+
  118
+        let!(:radiohead) do
  119
+          Band.create(name: "Radiohead")
  120
+        end
  121
+
  122
+        context "and the field doesn't exist on the last document" do
  123
+          let(:criteria) do
  124
+            Band.all
  125
+          end
  126
+
  127
+          let(:context) do
  128
+            Mongoid::Contextual::Mongo.new(criteria)
  129
+          end
  130
+
  131
+          let(:aggregates) do
  132
+            context.aggregates(:likes)
  133
+          end
  134
+
  135
+          it "returns a min" do
  136
+            aggregates["min"].should eq(50)
  137
+          end
  138
+        end
  139
+
  140
+        context "and the field doesn't exist on the before-last document" do
  141
+          let!(:u2) do
  142
+            Band.create(name: "U2", likes: 100)
  143
+          end
  144
+
  145
+          let(:criteria) do
  146
+            Band.all
  147
+          end
  148
+
  149
+          let(:context) do
  150
+            Mongoid::Contextual::Mongo.new(criteria)
  151
+          end
  152
+
  153
+          let(:aggregates) do
  154
+            context.aggregates(:likes)
  155
+          end
  156
+
  157
+          it "returns a min" do
  158
+            aggregates["min"].should eq(50)
  159
+          end
  160
+        end
  161
+      end
  162
+
113 163
       context "when there are no matching documents" do
114 164
 
115 165
         let(:criteria) do
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.