Skip to content

Commit

Permalink
Refactor GeoGridTiler classes (#72201)
Browse files Browse the repository at this point in the history 
This PR introduces the classes  AbstractGeoHashGridTiler and AbstractGeoTileGridTiler in order to 
simplify the current class hierarchy.
  • Loading branch information
@iverase
iverase committed Apr 27, 2021
1 parent 6078ac9 commit 423c47a
Show file tree
Hide file tree
Showing 12 changed files with 328 additions and 337 deletions.
16 changes: 8 additions & 8 deletions x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.elasticsearch.xpack.spatial.action.SpatialInfoTransportAction;
import org.elasticsearch.xpack.spatial.action.SpatialStatsTransportAction;
import org.elasticsearch.xpack.spatial.action.SpatialUsageTransportAction;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.UnBoundedGeoTileGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.UnboundedGeoHashGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.metrics.GeoShapeCentroidAggregator;
import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper;
import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper;
Expand All @@ -45,11 +47,9 @@
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.BoundedGeoHashGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.BoundedGeoTileGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoHashGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoShapeCellIdSource;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoShapeHashGridAggregator;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoShapeTileGridAggregator;
import org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid.GeoTileGridTiler;
import org.elasticsearch.xpack.spatial.search.aggregations.metrics.GeoShapeBoundsAggregator;
import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSource;
import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
Expand Down Expand Up @@ -150,11 +150,11 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde
if (getLicenseState().checkFeature(XPackLicenseState.Feature.SPATIAL_GEO_GRID)) {
final GeoGridTiler tiler;
if (geoBoundingBox.isUnbounded()) {
tiler = new GeoHashGridTiler();
tiler = new UnboundedGeoHashGridTiler(precision);
} else {
tiler = new BoundedGeoHashGridTiler(geoBoundingBox);
tiler = new BoundedGeoHashGridTiler(precision, geoBoundingBox);
}
GeoShapeCellIdSource cellIdSource = new GeoShapeCellIdSource((GeoShapeValuesSource) valuesSource, precision, tiler);
GeoShapeCellIdSource cellIdSource = new GeoShapeCellIdSource((GeoShapeValuesSource) valuesSource, tiler);
GeoShapeHashGridAggregator agg = new GeoShapeHashGridAggregator(name, factories, cellIdSource, requiredSize, shardSize,
aggregationContext, parent, collectsFromSingleBucket, metadata);
// this would ideally be something set in an immutable way on the ValuesSource
Expand All @@ -172,11 +172,11 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde
if (getLicenseState().checkFeature(XPackLicenseState.Feature.SPATIAL_GEO_GRID)) {
final GeoGridTiler tiler;
if (geoBoundingBox.isUnbounded()) {
tiler = new GeoTileGridTiler();
tiler = new UnBoundedGeoTileGridTiler(precision);
} else {
tiler = new BoundedGeoTileGridTiler(geoBoundingBox);
tiler = new BoundedGeoTileGridTiler(precision, geoBoundingBox);
}
GeoShapeCellIdSource cellIdSource = new GeoShapeCellIdSource((GeoShapeValuesSource) valuesSource, precision, tiler);
GeoShapeCellIdSource cellIdSource = new GeoShapeCellIdSource((GeoShapeValuesSource) valuesSource, tiler);
GeoShapeTileGridAggregator agg = new GeoShapeTileGridAggregator(name, factories, cellIdSource, requiredSize, shardSize,
context, parent, collectsFromSingleBucket, metadata);
// this would ideally be something set in an immutable way on the ValuesSource
Expand Down
85 changes: 46 additions & 39 deletions ...ions/bucket/geogrid/GeoHashGridTiler.java → ...ket/geogrid/AbstractGeoHashGridTiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,29 @@

package org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid;

import org.elasticsearch.geometry.Rectangle;
import org.elasticsearch.geometry.utils.Geohash;
import org.elasticsearch.xpack.spatial.index.fielddata.GeoRelation;
import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues;

public class GeoHashGridTiler implements GeoGridTiler {
/**
* Implements most of the logic for the GeoHash aggregation.
*/
abstract class AbstractGeoHashGridTiler extends GeoGridTiler {

AbstractGeoHashGridTiler(int precision) {
super(precision);
}

/** check if the provided hash is in the solution space of this tiler */
protected abstract boolean validHash(String hash);

@Override
public long encode(double x, double y, int precision) {
public long encode(double x, double y) {
return Geohash.longEncode(x, y, precision);
}

@Override
public int setValues(GeoShapeCellValues values, GeoShapeValues.GeoShapeValue geoValue, int precision) {
if (precision == 1) {
values.resizeCell(1);
values.add(0, Geohash.longEncode(0, 0, 0));
}
public int setValues(GeoShapeCellValues values, GeoShapeValues.GeoShapeValue geoValue) {

GeoShapeValues.BoundingBox bounds = geoValue.boundingBox();
assert bounds.minX() <= bounds.maxX();
Expand All @@ -35,29 +40,12 @@ public int setValues(GeoShapeCellValues values, GeoShapeValues.GeoShapeValue geo

// optimization for setting just one value for when the shape represents a point
if (bounds.minX() == bounds.maxX() && bounds.minY() == bounds.maxY()) {
return setValue(values, geoValue, bounds, precision);
return setValue(values, geoValue, bounds);
}
return setValuesByRasterization("", values, 0, precision, geoValue);
}

/**
* Sets a singular doc-value for the {@link GeoShapeValues.GeoShapeValue}. To be overriden by {@link BoundedGeoHashGridTiler}
* to account for {@link org.elasticsearch.common.geo.GeoBoundingBox} conditions
*/
protected int setValue(GeoShapeCellValues docValues, GeoShapeValues.GeoShapeValue geoValue, GeoShapeValues.BoundingBox bounds,
int precision) {
String hash = Geohash.stringEncode(bounds.minX(), bounds.minY(), precision);
docValues.resizeCell(1);
docValues.add(0, Geohash.longEncode(hash));
return 1;
return setValuesByRasterization("", values, 0, geoValue);
}

protected GeoRelation relateTile(GeoShapeValues.GeoShapeValue geoValue, String hash) {
Rectangle rectangle = Geohash.toBoundingBox(hash);
return geoValue.relate(rectangle);
}

protected int setValuesByBruteForceScan(GeoShapeCellValues values, GeoShapeValues.GeoShapeValue geoValue, int precision,
protected int setValuesByBruteForceScan(GeoShapeCellValues values, GeoShapeValues.GeoShapeValue geoValue,
GeoShapeValues.BoundingBox bounds) {
// TODO: This way to discover cells inside of a bounding box seems not to work as expected. I can
// see that eventually we will be visiting twice the same cell which should not happen.
Expand All @@ -75,33 +63,50 @@ protected int setValuesByBruteForceScan(GeoShapeCellValues values, GeoShapeValue
GeoRelation relation = relateTile(geoValue, hash);
if (relation != GeoRelation.QUERY_DISJOINT) {
values.resizeCell(idx + 1);
values.add(idx++, encode(i, j, precision));
values.add(idx++, encode(i, j));
}
}
}
return idx;
}

protected int setValuesByRasterization(String hash, GeoShapeCellValues values, int valuesIndex, int targetPrecision,
/**
* Sets a singular doc-value for the {@link GeoShapeValues.GeoShapeValue}.
*/
protected int setValue(GeoShapeCellValues docValues, GeoShapeValues.GeoShapeValue geoValue, GeoShapeValues.BoundingBox bounds) {
String hash = Geohash.stringEncode(bounds.minX(), bounds.minY(), precision);
if (relateTile(geoValue, hash) != GeoRelation.QUERY_DISJOINT) {
docValues.resizeCell(1);
docValues.add(0, Geohash.longEncode(hash));
return 1;
}
return 0;
}

protected GeoRelation relateTile(GeoShapeValues.GeoShapeValue geoValue, String hash) {
return validHash(hash) ? geoValue.relate(Geohash.toBoundingBox(hash)) : GeoRelation.QUERY_DISJOINT;
}

protected int setValuesByRasterization(String hash, GeoShapeCellValues values, int valuesIndex,
GeoShapeValues.GeoShapeValue geoValue) {
String[] hashes = Geohash.getSubGeohashes(hash);
for (int i = 0; i < hashes.length; i++) {
GeoRelation relation = relateTile(geoValue, hashes[i]);
if (relation == GeoRelation.QUERY_CROSSES) {
if (hashes[i].length() == targetPrecision) {
if (hashes[i].length() == precision) {
values.resizeCell(valuesIndex + 1);
values.add(valuesIndex++, Geohash.longEncode(hashes[i]));
} else {
valuesIndex =
setValuesByRasterization(hashes[i], values, valuesIndex, targetPrecision, geoValue);
setValuesByRasterization(hashes[i], values, valuesIndex, geoValue);
}
} else if (relation == GeoRelation.QUERY_INSIDE) {
if (hashes[i].length() == targetPrecision) {
if (hashes[i].length() == precision) {
values.resizeCell(valuesIndex + 1);
values.add(valuesIndex++, Geohash.longEncode(hashes[i]));
} else {
values.resizeCell(valuesIndex + (int) Math.pow(32, targetPrecision - hash.length()) + 1);
valuesIndex = setValuesForFullyContainedTile(hashes[i],values, valuesIndex, targetPrecision);
values.resizeCell(valuesIndex + (int) Math.pow(32, precision - hash.length()) + 1);
valuesIndex = setValuesForFullyContainedTile(hashes[i],values, valuesIndex, precision);
}
}
}
Expand All @@ -112,10 +117,12 @@ protected int setValuesForFullyContainedTile(String hash, GeoShapeCellValues val
int valuesIndex, int targetPrecision) {
String[] hashes = Geohash.getSubGeohashes(hash);
for (int i = 0; i < hashes.length; i++) {
if (hashes[i].length() == targetPrecision) {
values.add(valuesIndex++, Geohash.longEncode(hashes[i]));
} else {
valuesIndex = setValuesForFullyContainedTile(hashes[i], values, valuesIndex, targetPrecision);
if (validHash(hashes[i])) {
if (hashes[i].length() == targetPrecision) {
values.add(valuesIndex++, Geohash.longEncode(hashes[i]));
} else {
valuesIndex = setValuesForFullyContainedTile(hashes[i], values, valuesIndex, targetPrecision);
}
}
}
return valuesIndex;
Expand Down

0 comments on commit 423c47a

Please sign in to comment.