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.

Signed-off-by: Navneet Verma <[email protected]>
  • Loading branch information
navneet1v committed Nov 7, 2023
1 parent 05c2bef commit e1cb307
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 @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

### Fixed
- Fix class_cast_exception when passing int to _version and other metadata fields in ingest simulate API ([#10101](https://github.com/opensearch-project/OpenSearch/pull/10101))
- 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 e1cb307

Please sign in to comment.