You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@usergrid.apache.org by gr...@apache.org on 2015/08/05 23:55:24 UTC
incubator-usergrid git commit: Added fixes to do distance ordered by
queries.
Repository: incubator-usergrid
Updated Branches:
refs/heads/USERGRID-460-two-dot-o [created] 4887b0235
Added fixes to do distance ordered by queries.
Project: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/commit/4887b023
Tree: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/tree/4887b023
Diff: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/diff/4887b023
Branch: refs/heads/USERGRID-460-two-dot-o
Commit: 4887b02354d699b0abf5bbd5278a197b46907d3e
Parents: 2aeb7c6
Author: GERey <gr...@apigee.com>
Authored: Wed Aug 5 14:55:20 2015 -0700
Committer: GERey <gr...@apigee.com>
Committed: Wed Aug 5 14:55:20 2015 -0700
----------------------------------------------------------------------
.../cassandra/QueryProcessorImpl.java | 8 +-
.../org/apache/usergrid/persistence/GeoIT.java | 114 +++++++++++--------
.../index/impl/EsEntityIndexImpl.java | 44 ++++---
.../persistence/index/impl/EsQueryVistor.java | 66 ++++++-----
.../usergrid/persistence/index/query/Query.java | 16 +++
.../index/query/tree/QueryVisitor.java | 10 +-
.../applications/queries/GeoPagingTest.java | 43 +++++++
7 files changed, 209 insertions(+), 92 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessorImpl.java
----------------------------------------------------------------------
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessorImpl.java b/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessorImpl.java
index 94569d5..dc1728a 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessorImpl.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessorImpl.java
@@ -71,6 +71,7 @@ import org.apache.usergrid.persistence.query.ir.result.ScanColumn;
import org.apache.usergrid.persistence.schema.CollectionInfo;
import org.elasticsearch.index.query.FilterBuilder;
import org.elasticsearch.index.query.QueryBuilder;
+import org.elasticsearch.search.sort.GeoDistanceSortBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -469,7 +470,7 @@ public class QueryProcessorImpl implements QueryProcessor {
// change the property name to coordinates
nodes.push( new WithinNode( op.getProperty().getIndexedName(), op.getDistance().getFloatValue(),
- op.getLatitude().getFloatValue(), op.getLongitude().getFloatValue(), ++contextCount ) );
+ op.getLatitude().getFloatValue(), op.getLongitude().getFloatValue(), ++contextCount ) );
}
@@ -628,6 +629,11 @@ public class QueryProcessorImpl implements QueryProcessor {
public FilterBuilder getFilterBuilder() {
throw new UnsupportedOperationException("Not supported by this vistor implementation.");
}
+
+ @Override
+ public GeoDistanceSortBuilder getGeoDistanceSortBuilder() {
+ throw new UnsupportedOperationException("Not supported by this vistor implementation.");
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java
----------------------------------------------------------------------
diff --git a/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java b/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java
index a1ac4ff..68a4ae0 100644
--- a/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java
+++ b/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java
@@ -419,53 +419,6 @@ public class GeoIT extends AbstractCoreIT {
assertEquals(numEntities, count);
}
-
- @Test
- public void testSamePointPaging() throws Exception {
-
- EntityManager em = app.getEntityManager();
- assertNotNull(em);
-
- // save objects in a diagonal line from -90 -180 to 90 180
-
- int numEntities = 500;
-
- for (int i = 0; i < numEntities; i++) {
- Map<String, Object> data = new HashMap<String, Object>(2);
- data.put("name", String.valueOf(i));
- setPos(data, 0, 0);
-
- em.create("store", data);
- }
-
- em.refreshIndex();
-
- Query query = new Query();
- // earth's circumference is 40,075 kilometers. Up it to 50,000kilometers
- // just to be save
- query.addFilter("location within 50000000 of 0, 0");
- query.setLimit(100);
-
- int count = 0;
- Results results;
-
- do {
- results = em.searchCollection(em.getApplicationRef(), "stores", query);
-
- for (Entity entity : results.getEntities()) {
- assertEquals(String.valueOf(count), entity.getName());
- count++;
- }
-
- // set for the next "page"
- query.setCursor(results.getCursor());
- }
- while (results.getCursor() != null);
-
- // check we got back all 500 entities
- assertEquals(numEntities, count);
- }
-
@Test
public void testDistanceByLimit() throws Exception {
@@ -580,6 +533,73 @@ public class GeoIT extends AbstractCoreIT {
assertEquals(startDelta - (size - max), count);
}
+ /**
+ * Verify that elasticsearch does a secondary ordering on paging such that we get consistent results
+ * back from a cursor despite having a geoquery with all the positions in the same location.
+ * @throws Exception
+ */
+ @Test
+ public void testSamePointConsistantPaging() throws Exception {
+
+ EntityManager em = app.getEntityManager();
+ assertNotNull(em);
+
+ // save objects in a diagonal line from -90 -180 to 90 180
+
+ int numEntities = 500;
+
+ for (int i = 0; i < numEntities; i++) {
+ Map<String, Object> data = new HashMap<String, Object>(2);
+ data.put("name", String.valueOf(i));
+ setPos(data, 0, 0);
+
+ em.create("store", data);
+ }
+
+ em.refreshIndex();
+
+ Query query = new Query();
+ // earth's circumference is 40,075 kilometers. Up it to 50,000kilometers
+ // just to be save
+ query.addFilter("location within 50000000 of 0, 0");
+ query.setLimit(100);
+ List<String> names = new ArrayList<String>();
+
+ int count = 0;
+ Results results;
+ //get arraylist of entities from a search
+ do {
+ results = em.searchCollection(em.getApplicationRef(), "stores", query);
+
+ for (Entity entity : results.getEntities()) {
+ names.add( count,entity.getName() );
+ count++;
+ }
+
+ // set for the next "page"
+ query.setCursor(results.getCursor());
+ }
+ while (results.getCursor() != null);
+ //verify that entities come out in the same order when doing the same query against the same data.
+ //aka make sure the elasticsearch does a secondary search.
+ count = 0;
+ do {
+ results = em.searchCollection(em.getApplicationRef(), "stores", query);
+
+ for (Entity entity : results.getEntities()) {
+ assertEquals( names.get( count ),entity.getName() );
+ count++;
+ }
+
+ // set for the next "page"
+ query.setCursor(results.getCursor());
+ }
+ while (results.getCursor() != null);
+
+ // check we got back all 500 entities
+ assertEquals(numEntities, count);
+ }
+
@Test
public void testDenseSearch() throws Exception {
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java
----------------------------------------------------------------------
diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java
index 9add426..84672a1 100644
--- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java
+++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java
@@ -36,6 +36,7 @@ import org.apache.usergrid.persistence.index.exceptions.IndexException;
import org.apache.usergrid.persistence.index.query.CandidateResult;
import org.apache.usergrid.persistence.index.query.CandidateResults;
import org.apache.usergrid.persistence.index.query.Query;
+import org.apache.usergrid.persistence.index.query.tree.QueryVisitor;
import org.apache.usergrid.persistence.map.MapManager;
import org.apache.usergrid.persistence.map.MapManagerFactory;
import org.apache.usergrid.persistence.map.MapScope;
@@ -58,8 +59,10 @@ import org.elasticsearch.action.search.SearchRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchScrollRequestBuilder;
import org.elasticsearch.client.AdminClient;
+import org.elasticsearch.common.geo.GeoDistance;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.*;
@@ -293,7 +296,7 @@ public class EsEntityIndexImpl implements AliasedEntityIndex {
@Override
public String[] getIndexes(final AliasType aliasType) {
- return aliasCache.getIndexes(alias, aliasType);
+ return aliasCache.getIndexes( alias, aliasType );
}
@@ -387,6 +390,7 @@ public class EsEntityIndexImpl implements AliasedEntityIndex {
final String[] entityTypes = searchTypes.getTypeNames();
QueryBuilder qb = query.createQueryBuilder(context);
+ QueryVisitor queryVisitor = query.getQueryVisitor();
SearchResponse searchResponse;
@@ -423,24 +427,32 @@ public class EsEntityIndexImpl implements AliasedEntityIndex {
// that you can order by: string, number and boolean and we ask ElasticSearch
// to ignore any fields that are not present.
- final String stringFieldName = STRING_PREFIX + sp.getPropertyName();
- final FieldSortBuilder stringSort = SortBuilders.fieldSort( stringFieldName )
- .order( order ).ignoreUnmapped( true );
- srb.addSort( stringSort );
+ if(fb instanceof GeoDistanceFilterBuilder){
+ srb.addSort( queryVisitor.getGeoDistanceSortBuilder().order( SortOrder.ASC )
+ .unit( DistanceUnit.KILOMETERS )
+ .geoDistance( GeoDistance.SLOPPY_ARC ) );
- logger.debug( " Sort: {} order by {}", stringFieldName, order.toString() );
+ logger.info( " Geo Sort: {} order by {}", sp.getPropertyName(), order.toString() );
+ }
+ else {
- final String numberFieldName = NUMBER_PREFIX + sp.getPropertyName();
- final FieldSortBuilder numberSort = SortBuilders.fieldSort( numberFieldName )
- .order( order ).ignoreUnmapped( true );
- srb.addSort( numberSort );
- logger.debug( " Sort: {} order by {}", numberFieldName, order.toString() );
+ final String stringFieldName = STRING_PREFIX + sp.getPropertyName();
+ final FieldSortBuilder stringSort = SortBuilders.fieldSort( stringFieldName ).order( order ).ignoreUnmapped( true );
+ srb.addSort( stringSort );
- final String booleanFieldName = BOOLEAN_PREFIX + sp.getPropertyName();
- final FieldSortBuilder booleanSort = SortBuilders.fieldSort( booleanFieldName )
- .order( order ).ignoreUnmapped( true );
- srb.addSort( booleanSort );
- logger.debug( " Sort: {} order by {}", booleanFieldName, order.toString() );
+ logger.debug( " Sort: {} order by {}", stringFieldName, order.toString() );
+
+ final String numberFieldName = NUMBER_PREFIX + sp.getPropertyName();
+ final FieldSortBuilder numberSort = SortBuilders.fieldSort( numberFieldName ).order( order ).ignoreUnmapped( true );
+ srb.addSort( numberSort );
+ logger.debug( " Sort: {} order by {}", numberFieldName, order.toString() );
+
+ final String booleanFieldName = BOOLEAN_PREFIX + sp.getPropertyName();
+ final FieldSortBuilder booleanSort = SortBuilders.fieldSort( booleanFieldName ).order( order )
+ .ignoreUnmapped( true );
+ srb.addSort( booleanSort );
+ logger.debug( " Sort: {} order by {}", booleanFieldName, order.toString() );
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsQueryVistor.java
----------------------------------------------------------------------
diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsQueryVistor.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsQueryVistor.java
index f012bab..53613b5 100644
--- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsQueryVistor.java
+++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsQueryVistor.java
@@ -30,6 +30,8 @@ import org.elasticsearch.index.query.FilterBuilder;
import org.elasticsearch.index.query.FilterBuilders;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
+import org.elasticsearch.search.sort.GeoDistanceSortBuilder;
+import org.elasticsearch.search.sort.SortBuilders;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -58,7 +60,7 @@ import static org.apache.usergrid.persistence.index.impl.IndexingUtils.STRING_PR
/**
- * Visits tree of parsed Query operands and populates
+ * Visits tree of parsed Query operands and populates
* ElasticSearch QueryBuilder that represents the query.
*/
public class EsQueryVistor implements QueryVisitor {
@@ -66,8 +68,12 @@ public class EsQueryVistor implements QueryVisitor {
Stack<QueryBuilder> stack = new Stack<QueryBuilder>();
List<FilterBuilder> filterBuilders = new ArrayList<FilterBuilder>();
+ Boolean geoFieldPresent = false;
+
+ GeoDistanceSortBuilder geoSortBuilder = null;
+
+
-
@Override
public void visit( AndOperand op ) throws IndexException {
@@ -157,14 +163,14 @@ public class EsQueryVistor implements QueryVisitor {
Object value = op.getLiteral().getValue();
BoolQueryBuilder qb = QueryBuilders.boolQuery(); // let's do a boolean OR
- qb.minimumNumberShouldMatch(1);
+ qb.minimumNumberShouldMatch(1);
// field is an entity/array that needs no name prefix
qb = qb.should( QueryBuilders.matchQuery( name, value ) );
// OR field is a string and needs the prefix on the name
qb = qb.should( QueryBuilders.matchQuery( addPrefix( value.toString(), name, true), value));
-
+
stack.push( qb );
}
@@ -186,7 +192,10 @@ public class EsQueryVistor implements QueryVisitor {
FilterBuilder fb = FilterBuilders.geoDistanceFilter( name )
.lat( lat ).lon( lon ).distance( distance, DistanceUnit.METERS );
filterBuilders.add( fb );
- }
+
+
+ geoSortBuilder = SortBuilders.geoDistanceSort( GEO_PREFIX + "location" ).point( lat,lon );
+ }
@Override
@@ -222,19 +231,19 @@ public class EsQueryVistor implements QueryVisitor {
qb.minimumNumberShouldMatch(1);
// field is an entity/array that does not need a prefix on its name
- // TODO is this right now that we've updated our doc structure?
+ // TODO is this right now that we've updated our doc structure?
// Should this be "must" instead of should?
qb = qb.should( QueryBuilders.wildcardQuery( name, svalue ) );
-
+
// or field is just a string that does need a prefix
if ( svalue.indexOf("*") != -1 ) {
qb = qb.should( QueryBuilders.wildcardQuery( addPrefix( value, name ), svalue ) );
} else {
qb = qb.should( QueryBuilders.termQuery( addPrefix( value, name ), value ));
- }
+ }
stack.push( qb );
return;
- }
+ }
// assume all other types need prefix
stack.push( QueryBuilders.termQuery( addPrefix( value, name ), value ));
@@ -276,7 +285,7 @@ public class EsQueryVistor implements QueryVisitor {
if ( parts.length > 1 ) {
name = parts[ parts.length - 1 ];
}
-
+
if ( value instanceof String && analyzed ) {
name = addAnalyzedStringPrefix( name );
@@ -293,12 +302,12 @@ public class EsQueryVistor implements QueryVisitor {
name = addStringPrefix( name );
}
- // re-create nested property name
+ // re-create nested property name
if ( parts.length > 1 ) {
parts[parts.length - 1] = name;
Joiner joiner = Joiner.on(".").skipNulls();
return joiner.join(parts);
- }
+ }
return name;
}
@@ -307,34 +316,34 @@ public class EsQueryVistor implements QueryVisitor {
private String addAnalyzedStringPrefix( String name ) {
if ( name.startsWith( ANALYZED_STRING_PREFIX ) ) {
return name;
- }
+ }
return ANALYZED_STRING_PREFIX + name;
- }
-
+ }
+
private String addStringPrefix( String name ) {
if ( name.startsWith( STRING_PREFIX ) ) {
return name;
- }
+ }
return STRING_PREFIX + name;
- }
-
+ }
+
private String addNumberPrefix( String name ) {
if ( name.startsWith( NUMBER_PREFIX ) ) {
return name;
- }
+ }
return NUMBER_PREFIX + name;
- }
-
+ }
+
private String addBooleanPrefix( String name ) {
if ( name.startsWith( BOOLEAN_PREFIX ) ) {
return name;
- }
+ }
return BOOLEAN_PREFIX + name;
- }
-
+ }
+
@Override
public QueryBuilder getQueryBuilder() {
@@ -354,14 +363,19 @@ public class EsQueryVistor implements QueryVisitor {
for ( FilterBuilder fb : filterBuilders ) {
if ( andFilter == null ) {
andFilter = FilterBuilders.andFilter( fb );
- } else {
+ } else {
andFilter = FilterBuilders.andFilter( andFilter, fb );
}
- }
+ }
} else if ( !filterBuilders.isEmpty() ) {
return filterBuilders.get(0);
}
return null;
}
+
+ @Override
+ public GeoDistanceSortBuilder getGeoDistanceSortBuilder(){
+ return geoSortBuilder;
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java
----------------------------------------------------------------------
diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java
index 9a7a867..c0361fc 100644
--- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java
+++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java
@@ -213,6 +213,22 @@ public class Query {
return filterBuilder;
}
+ public QueryVisitor getQueryVisitor() {
+
+ if ( getRootOperand() != null ) {
+ QueryVisitor v = new EsQueryVistor();
+ try {
+ getRootOperand().visit( v );
+
+ } catch ( IndexException ex ) {
+ throw new RuntimeException( "Error building ElasticSearch query", ex );
+ }
+ return v;
+ }
+
+ return null;
+ }
+
/**
* Create a query instance from the QL. If the string is null, return an empty query
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/tree/QueryVisitor.java
----------------------------------------------------------------------
diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/tree/QueryVisitor.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/tree/QueryVisitor.java
index d295c92..10203ad 100644
--- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/tree/QueryVisitor.java
+++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/tree/QueryVisitor.java
@@ -24,6 +24,7 @@ import org.apache.usergrid.persistence.index.exceptions.NoIndexException;
import org.apache.usergrid.persistence.index.exceptions.IndexException;
import org.elasticsearch.index.query.FilterBuilder;
import org.elasticsearch.index.query.QueryBuilder;
+import org.elasticsearch.search.sort.GeoDistanceSortBuilder;
/**
@@ -93,10 +94,15 @@ public interface QueryVisitor {
*/
public void visit( GreaterThanEqual op ) throws NoIndexException;
- /**
+ /**
* Returns resulting query builder.
*/
public QueryBuilder getQueryBuilder();
- public FilterBuilder getFilterBuilder();
+ public FilterBuilder getFilterBuilder();
+
+ /**
+ * Returns resulting geo distance sort builder.
+ */
+ public GeoDistanceSortBuilder getGeoDistanceSortBuilder();
}
http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/4887b023/stack/rest/src/test/java/org/apache/usergrid/rest/applications/queries/GeoPagingTest.java
----------------------------------------------------------------------
diff --git a/stack/rest/src/test/java/org/apache/usergrid/rest/applications/queries/GeoPagingTest.java b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/queries/GeoPagingTest.java
index f9fe5b3..7bab8e2 100644
--- a/stack/rest/src/test/java/org/apache/usergrid/rest/applications/queries/GeoPagingTest.java
+++ b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/queries/GeoPagingTest.java
@@ -289,4 +289,47 @@ public class GeoPagingTest extends AbstractRestIT {
assertEquals("Expected 0 results", 0, collection.getResponse().getEntityCount());
}
}
+
+
+ /**
+ * Test that geo-query returns co-located entities in expected order.
+ */
+ @Test
+ public void groupQueriesWithDistanceOrderedResults() throws IOException {
+
+ int maxRangeLimit = 9;
+ Entity[] cats = new Entity[maxRangeLimit+1];
+
+ // 1. Create several entities
+ for (int i = maxRangeLimit; i >= 0; i--) {
+ Entity cat = new Entity();
+ cat.put("name", "cat" + i);
+ cat.put( "location",
+ new MapUtils.HashMapBuilder<String, Double>().map( "latitude", 37.0 + i )
+ .map( "longitude", ( -75.0 + i) ) );
+ cats[i] = cat;
+ this.app().collection("cats").post(cat);
+ }
+ this.refreshIndex();
+
+ QueryParameters params = new QueryParameters();
+ String query = String.format(
+ "location within 15000000 of 37,-75");
+ params.setQuery(query);
+ Collection collection = this.app().collection("cats").get(params);
+ assertNotNull( collection );
+ List entities = collection.getResponse().getEntities();
+ assertNotNull( entities );
+
+ for (int consistent = 0; consistent < maxRangeLimit; consistent++) {
+ //got entities back, just need to page through them and make sure that i got them in location order.
+ Entity entity = (Entity) entities.get( consistent );
+ assertNotNull( entity );
+ LinkedHashMap location = ( LinkedHashMap ) entity.get( "location" );
+ assertEquals( 37.0+ consistent,location.get( "latitude" ) );
+ assertEquals( -75.0+ consistent , location.get( "longitude" ) );
+
+ }
+ }
+
}