Skip to content

Commit

Permalink
[#6187, #6222] Fixing the GeoShapes GeoHash and GeoTile Aggregations …
Browse files Browse the repository at this point in the history
…Integration tests to make them more robust.

Signed-off-by: Navneet Verma <navneev@amazon.com>
  • Loading branch information
navneet1v committed Feb 8, 2023
1 parent 1f4cdd2 commit facb25a
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected void prepareGeoShapeIndexForAggregations(final Random random) throws E
}

i++;
final Set<String> values = generateBucketsForGeometry(geometry, geometryDocValue, isShapeIntersectingBB);
final Set<String> values = generateBucketsForGeometry(geometry, geometryDocValue);
geoshapes.add(indexGeoShape(GEO_SHAPE_INDEX_NAME, geometry));
for (final String hash : values) {
expectedDocsCountForGeoShapes.put(hash, expectedDocsCountForGeoShapes.getOrDefault(hash, 0) + 1);
Expand All @@ -112,16 +112,11 @@ protected void prepareGeoShapeIndexForAggregations(final Random random) throws E
* Returns a set of buckets for the shape at different precision level. Override this method for different bucket
* aggregations.
*
* @param geometry {@link Geometry}
* @param geoShapeDocValue {@link GeoShapeDocValue}
* @param intersectingWithBB boolean
* @param geometry {@link Geometry}
* @param geoShapeDocValue {@link GeoShapeDocValue}
* @return A {@link Set} of {@link String} which represents the buckets.
*/
protected abstract Set<String> generateBucketsForGeometry(
final Geometry geometry,
final GeoShapeDocValue geoShapeDocValue,
final boolean intersectingWithBB
);
protected abstract Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue);

/**
* Prepares a GeoPoint index for testing the GeoPoint bucket aggregations. Different bucket aggregations can use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,15 @@ public void testShardSizeIsZero() {
}

@Override
protected Set<String> generateBucketsForGeometry(
final Geometry geometry,
final GeoShapeDocValue geometryDocValue,
boolean intersectingWithBB
) {
protected Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geometryDocValue) {
final GeoPoint topLeft = new GeoPoint();
final GeoPoint bottomRight = new GeoPoint();
assert geometry != null;
GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight);
final Set<String> geoHashes = new HashSet<>();
final boolean isIntersectingWithBoundingRectangle = geometryDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg);
for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && intersectingWithBB == false) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && isIntersectingWithBoundingRectangle == false) {
continue;
}
final GeoPoint topRight = new GeoPoint(topLeft.getLat(), bottomRight.getLon());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,20 @@ public void testMultivaluedGeoPointsAggregation() throws Exception {
* Returns a set of buckets for the shape at different precision level. Override this method for different bucket
* aggregations.
*
* @param geometry {@link Geometry}
* @param geoShapeDocValue {@link GeoShapeDocValue}
* @param intersectingWithBB
* @param geometry {@link Geometry}
* @param geoShapeDocValue {@link GeoShapeDocValue}
* @return A {@link Set} of {@link String} which represents the buckets.
*/
@Override
protected Set<String> generateBucketsForGeometry(Geometry geometry, GeoShapeDocValue geoShapeDocValue, boolean intersectingWithBB) {
protected Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue) {
final GeoPoint topLeft = new GeoPoint();
final GeoPoint bottomRight = new GeoPoint();
assert geometry != null;
GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight);
final Set<String> geoTiles = new HashSet<>();
final boolean isIntersectingWithBoundingRectangle = geoShapeDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg);
for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && intersectingWithBB == false) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && isIntersectingWithBoundingRectangle == false) {
continue;
}
geoTiles.addAll(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.opensearch.common.geo.GeoBoundingBox;
import org.opensearch.common.geo.GeoShapeDocValue;
import org.opensearch.geometry.Rectangle;
import org.opensearch.index.fielddata.AbstractSortingNumericDocValues;
import org.opensearch.index.fielddata.GeoShapeValue;

Expand Down Expand Up @@ -57,8 +58,7 @@ public boolean advanceExact(int docId) throws IOException {
* @opensearch.internal
*/
static class BoundedCellValues extends GeoShapeCellValues {

private final GeoBoundingBox geoBoundingBox;
private final Rectangle geoBoundingBox;

public BoundedCellValues(
final GeoShapeValue geoShapeValue,
Expand All @@ -67,7 +67,7 @@ public BoundedCellValues(
final GeoBoundingBox boundingBox
) {
super(geoShapeValue, precision, encoder);
this.geoBoundingBox = boundingBox;
this.geoBoundingBox = new Rectangle(boundingBox.left(), boundingBox.right(), boundingBox.top(), boundingBox.bottom());
}

/**
Expand All @@ -78,30 +78,21 @@ public BoundedCellValues(
*/
@Override
void relateShape(final GeoShapeDocValue geoShapeDocValue) {
if (intersect(geoShapeDocValue.getBoundingRectangle())) {
if (geoShapeDocValue.isIntersectingRectangle(geoBoundingBox)) {
// now we know the shape is in the bounding rectangle, we need add them in longValues
// generate all grid that this shape intersects
final List<Long> encodedValues = encoder.encode(geoShapeDocValue, precision);
resize(encodedValues.size());
for (int i = 0; i < encodedValues.size(); i++) {
values[i] = encodedValues.get(i);
}
} else {
// As the shape is not intersecting with GeoBounding box, we need to reset the GeoShapeCellValues
// calling this function resets the CellValues for the current shape.
resize(0);
}
}

/**
* Validate that shape is intersecting the bounding box provided as input.
*
* @param rectangle {@link GeoShapeDocValue.BoundingRectangle}
* @return true or false
*/
private boolean intersect(final GeoShapeDocValue.BoundingRectangle rectangle) {
return geoBoundingBox.pointInBounds(rectangle.getMaxLongitude(), rectangle.getMaxLatitude())
|| geoBoundingBox.pointInBounds(rectangle.getMaxLongitude(), rectangle.getMinLatitude())
|| geoBoundingBox.pointInBounds(rectangle.getMinLongitude(), rectangle.getMaxLatitude())
|| geoBoundingBox.pointInBounds(rectangle.getMinLongitude(), rectangle.getMinLatitude());
}

}

/**
Expand Down

0 comments on commit facb25a

Please sign in to comment.