Skip to content

Commit

Permalink
Adding version condition while adding geoshape doc values to the inde…
Browse files Browse the repository at this point in the history
…x, to ensure backward compatibility. (#11190)

Signed-off-by: Navneet Verma <navneev@amazon.com>
  • Loading branch information
navneet1v committed Nov 14, 2023
1 parent 2eb43d3 commit 58af58d
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

### Fixed
- [BUG] Disable sort optimization for HALF_FLOAT ([#10999](https://github.com/opensearch-project/OpenSearch/pull/10999))
- Adding version condition while adding geoshape doc values to the index, to ensure backward compatibility.([#11095](https://github.com/opensearch-project/OpenSearch/pull/11095))

### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected boolean forbidPrivateIndexSettings() {
*/
protected void prepareGeoShapeIndexForAggregations(final Random random) throws Exception {
expectedDocsCountForGeoShapes = new HashMap<>();
final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build();
final List<IndexRequestBuilder> geoshapes = new ArrayList<>();
assertAcked(prepareCreate(GEO_SHAPE_INDEX_NAME).setSettings(settings).setMapping(GEO_SHAPE_FIELD_NAME, "type" + "=geo_shape"));
boolean isShapeIntersectingBB = false;
Expand Down Expand Up @@ -136,7 +136,7 @@ protected void prepareSingleValueGeoPointIndex(final Random random) throws Excep
expectedDocCountsForSingleGeoPoint = new HashMap<>();
createIndex("idx_unmapped");
final Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, version)
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put("index.number_of_shards", 4)
.put("index.number_of_replicas", 0)
.build();
Expand All @@ -160,7 +160,7 @@ protected void prepareSingleValueGeoPointIndex(final Random random) throws Excep

protected void prepareMultiValuedGeoPointIndex(final Random random) throws Exception {
multiValuedExpectedDocCountsGeoPoint = new HashMap<>();
final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build();
final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build();
final List<IndexRequestBuilder> cities = new ArrayList<>();
assertAcked(
prepareCreate("multi_valued_idx").setSettings(settings)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"Insert Document with geoshape field":
- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "geo_shape_index_old", "_id":191}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [101.0, 0.0] ]}}'
- '{"index": {"_index": "geo_shape_index_old", "_id":219}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [106.0, 0.0] ]}}'

- do:
search:
rest_total_hits_as_int: true
index: geo_shape_index_old
- match: { hits.total: 2 }
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
"Create index with Geoshape field":
- do:
indices.create:
index: geo_shape_index_old
body:
settings:
index:
number_of_replicas: 2
mappings:
"properties":
"location":
"type": "geo_shape"

- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "geo_shape_index_old", "_id":191}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [101.0, 0.0] ]}}'
- '{"index": {"_index": "geo_shape_index_old", "_id":219}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [106.0, 0.0] ]}}'

- do:
search:
rest_total_hits_as_int: true
index: geo_shape_index_old
- match: { hits.total: 2 }
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
---
"Validate we are able to index documents after upgrade":
- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "geo_shape_index_old", "_id":191}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [101.0, 0.0] ]}}'
- '{"index": {"_index": "geo_shape_index_old", "_id":219}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [106.0, 0.0] ]}}'

- do:
search:
rest_total_hits_as_int: true
index: geo_shape_index_old
- match: { hits.total: 2 }


---
"Create index with Geoshape field in new cluster":
- do:
indices.create:
index: geo_shape_index_new
body:
settings:
index:
number_of_replicas: 2
mappings:
"properties":
"location":
"type": "geo_shape"

- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "geo_shape_index_new", "_id":191}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [101.0, 0.0] ]}}'
- '{"index": {"_index": "geo_shape_index_new", "_id":219}}'
- '{"name": "NEMO Science Museum","location": {"type": "envelope","coordinates": [ [100.0, 1.0], [106.0, 0.0] ]}}'

- do:
search:
rest_total_hits_as_int: true
index: geo_shape_index_new
- match: { hits.total: 2 }

- do:
search:
rest_total_hits_as_int: true
index: geo_shape_index_new
body:
aggregations:
myaggregation:
geo_bounds:
field: "location"
- match: { hits.total: 2 }
- match: { aggregations.myaggregation.bounds.top_left.lat: 0.9999999823048711 }
- match: { aggregations.myaggregation.bounds.top_left.lon: 99.99999999068677 }
- match: { aggregations.myaggregation.bounds.bottom_right.lat: 0.0 }
- match: { aggregations.myaggregation.bounds.bottom_right.lon: 105.99999996833503 }
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,15 @@

package org.opensearch.index.mapper;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.document.LatLonShape;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.search.Query;
import org.opensearch.Version;
import org.opensearch.common.Explicit;
import org.opensearch.common.geo.GeometryParser;
import org.opensearch.common.geo.ShapeRelation;
Expand Down Expand Up @@ -77,6 +80,7 @@
* @opensearch.internal
*/
public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
private static final Logger logger = LogManager.getLogger(GeoShapeFieldMapper.class);
public static final String CONTENT_TYPE = "geo_shape";
public static final FieldType FIELD_TYPE = new FieldType();
static {
Expand Down Expand Up @@ -205,9 +209,24 @@ protected void addDocValuesFields(
final List<IndexableField> indexableFields,
final ParseContext context
) {
Field[] fieldsArray = new Field[indexableFields.size()];
fieldsArray = indexableFields.toArray(fieldsArray);
context.doc().add(LatLonShape.createDocValueField(name, fieldsArray));
/*
* We are adding the doc values for GeoShape only if the index is created with 2.9 and above version of
* OpenSearch. If we don't do that after the upgrade of OpenSearch customers are not able to index documents
* with GeoShape fields. Github issue: https://github.com/opensearch-project/OpenSearch/issues/10958,
* https://github.com/opensearch-project/OpenSearch/issues/10795
*/
if (context.indexSettings().getIndexVersionCreated().onOrAfter(Version.V_2_9_0)) {
Field[] fieldsArray = new Field[indexableFields.size()];
fieldsArray = indexableFields.toArray(fieldsArray);
context.doc().add(LatLonShape.createDocValueField(name, fieldsArray));
} else {
logger.warn(
"The index was created with Version : {}, for geoshape doc values to work index must be "
+ "created with OpenSearch Version : {} or above",
context.indexSettings().getIndexVersionCreated(),
Version.V_2_9_0
);
}
}

@Override
Expand Down

0 comments on commit 58af58d

Please sign in to comment.