Permalink
Browse files

SERVER-7794 fix non-string non-geo keys and empty keys for 2dsphere

  • Loading branch information...
1 parent dd2ff17 commit 470d82e7bf89fb9b8bd8f369999a0d5dace122e1 Hari Khalsa committed Dec 1, 2012
Showing with 91 additions and 30 deletions.
  1. +40 −0 jstests/geo_s2edgecases.js
  2. +22 −0 jstests/geo_s2nonstring.js
  3. +29 −30 src/mongo/db/geo/s2index.cpp
@@ -0,0 +1,40 @@
+t = db.geo_s2edgecases
+t.drop()
+
+roundworldpoint = { "type" : "Point", "coordinates": [ 180, 0 ] }
+
+// Opposite the equator
+roundworld = { "type" : "Polygon",
+ "coordinates" : [ [ [179,1], [181,1], [181,-1], [179,-1], [179,1]]]}
+t.insert({geo : roundworld})
+
+roundworld2 = { "type" : "Polygon",
+ "coordinates" : [ [ [179,1], [179,-1], [181,-1], [181,1], [179,1]]]}
+t.insert({geo : roundworld2})
+
+// North pole
+santapoint = { "type" : "Point", "coordinates": [ 180, 90 ] }
+santa = { "type" : "Polygon",
+ "coordinates" : [ [ [179,89], [179,91], [181,91], [181,89], [179,89]]]}
+t.insert({geo : santa})
+santa2 = { "type" : "Polygon",
+ "coordinates" : [ [ [179,89], [181,89], [181,91], [179,91], [179,89]]]}
+t.insert({geo : santa2})
+
+// South pole
+penguinpoint = { "type" : "Point", "coordinates": [ 0, -90 ] }
+penguin1 = { "type" : "Polygon",
+ "coordinates" : [ [ [0,-89], [0,-91], [179,-91], [179,-89], [0,-89]]]}
+t.insert({geo : penguin1})
+penguin2 = { "type" : "Polygon",
+ "coordinates" : [ [ [0,-89], [179,-89], [179,-91], [0,-91], [0,-89]]]}
+t.insert({geo : penguin2})
+
+t.ensureIndex( { geo : "2dsphere", nonGeo: 1 } )
+
+res = t.find({ "geo" : { "$intersect" : { "$geometry" : roundworldpoint} } });
+assert.eq(res.count(), 2);
+res = t.find({ "geo" : { "$intersect" : { "$geometry" : santapoint} } });
+assert.eq(res.count(), 2);
+res = t.find({ "geo" : { "$intersect" : { "$geometry" : penguinpoint} } });
+assert.eq(res.count(), 2);
@@ -0,0 +1,22 @@
+// Added to make sure that S2 indexing's string AND non-string keys work.
+t = db.geo_s2nonstring
+t.drop()
+
+t.ensureIndex( { geo:'2dsphere', x:1 } );
+
+t.save( { geo:{ type:'Point', coordinates:[ 0, 0 ] }, x:'a' } );
+t.save( { geo:{ type:'Point', coordinates:[ 0, 0 ] }, x:5 } );
+
+t.drop()
+t.ensureIndex( { geo:'2dsphere', x:1 } );
+
+t.save( { geo:{ type:'Point', coordinates:[ 0, 0 ] }, x:'a' } );
+t.save( { geo:{ type:'Point', coordinates:[ 0, 0 ] } } );
+
+// Expect 1 match, where x is 'a'
+assert.eq( 1, t.count( { geo:{ $near:{ $geometry:{ type:'Point', coordinates:[ 0, 0 ] },
+ $maxDistance: 20 } }, x:'a' } ) );
+
+// Expect 1 match, where x matches null (missing matches null).
+assert.eq( 1, t.count( { geo:{ $near:{ $geometry:{ type:'Point', coordinates:[ 0, 0 ] },
+ $maxDistance: 20 } }, x:null } ) );
@@ -92,43 +92,35 @@ namespace mongo {
BSONElementSet fieldElements;
obj.getFieldsDotted(field.name, fieldElements);
- BSONObj keysForThisField;
+ BSONObjSet keysForThisField;
if (IndexedField::GEO == field.type) {
- keysForThisField = getGeoKeys(fieldElements);
+ getGeoKeys(fieldElements, &keysForThisField);
} else if (IndexedField::LITERAL == field.type) {
- keysForThisField = getLiteralKeys(fieldElements);
+ getLiteralKeys(fieldElements, &keysForThisField);
} else {
verify(0);
}
// We expect there to be _spec->_missingField() present in the keys if data is
// missing. So, this should be non-empty.
- verify(!keysForThisField.isEmpty());
+ verify(keysForThisField.size() > 0);
// We take the Cartesian product of all of the keys. This requires that we have
// some keys to take the Cartesian product with. If keysToAdd.empty(), we
// initialize it.
if (keysToAdd.empty()) {
- // This should only happen w/the first field.
- verify(0 == i);
- BSONObjIterator newIt(keysForThisField);
- while (newIt.more()) {
- BSONObjBuilder b;
- b.append("", newIt.next().String());
- keysToAdd.insert(b.obj());
- }
+ keysToAdd = keysForThisField;
continue;
}
BSONObjSet updatedKeysToAdd;
for (BSONObjSet::const_iterator it = keysToAdd.begin(); it != keysToAdd.end();
++it) {
-
- BSONObjIterator newIt(keysForThisField);
- while (newIt.more()) {
+ for (BSONObjSet::const_iterator newIt = keysForThisField.begin();
+ newIt!= keysForThisField.end(); ++newIt) {
BSONObjBuilder b;
b.appendElements(*it);
- b.append("", newIt.next().String());
+ b.append(newIt->firstElement());
updatedKeysToAdd.insert(b.obj());
}
}
@@ -253,9 +245,7 @@ namespace mongo {
const IndexDetails* getDetails() const { return _spec->getDetails(); }
private:
// Get the index keys for elements that are GeoJSON.
- BSONObj getGeoKeys(const BSONElementSet &elements) const {
- BSONArrayBuilder aBuilder;
-
+ void getGeoKeys(const BSONElementSet &elements, BSONObjSet *out) const {
S2RegionCoverer coverer;
_params.configureCoverer(&coverer);
@@ -281,29 +271,38 @@ namespace mongo {
}
for (vector<string>::const_iterator it = cells.begin(); it != cells.end(); ++it) {
- aBuilder.append(*it);
+ BSONObjBuilder b;
+ b.append("", *it);
+ out->insert(b.obj());
}
}
- if (0 == aBuilder.arrSize()) {
- // TODO(hk): We use "" for empty. I should verify this actually works.
- aBuilder.append("");
+ if (0 == out->size()) {
+ BSONObjBuilder b;
+ b.appendNull("");
+ out->insert(b.obj());
}
-
- return aBuilder.arr();
}
// elements is a non-geo field. Add the values literally, expanding arrays.
- BSONObj getLiteralKeys(const BSONElementSet &elements) const {
- BSONArrayBuilder builder;
+ void getLiteralKeys(const BSONElementSet &elements, BSONObjSet *out) const {
if (0 == elements.size()) {
- builder.append("");
+ BSONObjBuilder b;
+ b.appendNull("");
+ out->insert(b.obj());
+ } else if (1 == elements.size()) {
+ BSONObjBuilder b;
+ b.appendAs(*elements.begin(), "");
+ out->insert(b.obj());
} else {
+ BSONArrayBuilder aBuilder;
for (BSONElementSet::iterator i = elements.begin(); i != elements.end(); ++i) {
- builder.append(*i);
+ aBuilder.append(*i);
}
+ BSONObjBuilder b;
+ b.append("", aBuilder.arr());
+ out->insert(b.obj());
}
- return builder.arr();
}
vector<IndexedField> _fields;

0 comments on commit 470d82e

Please sign in to comment.