From b16fb085660aac7b7635614e31751f53e22afce6 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 6 May 2025 17:39:56 -0500 Subject: [PATCH 01/10] Dont store geo_point in ignored source --- .../index/mapper/DocumentParser.java | 2 +- .../GeoPointFieldBlockLoaderTests.java | 29 +++++++------------ 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 06dec6a090352..f877502fe8754 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -460,7 +460,7 @@ static void parseObjectOrField(DocumentParserContext context, Mapper mapper) thr if (context.canAddIgnoredField() && (fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK || sourceKeepMode == Mapper.SourceKeepMode.ALL - || (sourceKeepMode == Mapper.SourceKeepMode.ARRAYS && context.inArrayScope()) + || (sourceKeepMode == Mapper.SourceKeepMode.ARRAYS && context.inArrayScope() && parsesArrayValue(mapper) == false) || (context.isWithinCopyTo() == false && context.isCopyToDestinationField(mapper.fullPath())))) { context = context.addIgnoredFieldFromContext( IgnoredSourceFieldMapper.NameValue.fromContext(context, fieldMapper.fullPath(), null) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java index 530be9fbbc738..619cac375df51 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java @@ -30,10 +30,7 @@ public GeoPointFieldBlockLoaderTests(BlockLoaderTestCase.Params params) { @Override @SuppressWarnings("unchecked") - protected Object expected(Map fieldMapping, Object value, TestContext testContext) { - var extractedFieldValues = (ExtractedFieldValues) value; - var values = extractedFieldValues.values(); - + protected Object expected(Map fieldMapping, Object values, TestContext testContext) { var nullValue = switch (fieldMapping.get("null_value")) { case String s -> convert(s, null, false); case null -> null; @@ -75,9 +72,6 @@ protected Object expected(Map fieldMapping, Object value, TestCo if (syntheticSourceKeep.equals("all")) { return exactValuesFromSource(values, nullValue, false); } - if (syntheticSourceKeep.equals("arrays") && extractedFieldValues.documentHasObjectArrays()) { - return exactValuesFromSource(values, nullValue, false); - } // synthetic source and doc_values are present if (hasDocValues(fieldMapping, true)) { @@ -112,42 +106,39 @@ private Object exactValuesFromSource(Object value, GeoPoint nullValue, boolean n return maybeFoldList(resultList); } - private record ExtractedFieldValues(Object values, boolean documentHasObjectArrays) {} - @Override protected Object getFieldValue(Map document, String fieldName) { var extracted = new ArrayList<>(); - var documentHasObjectArrays = processLevel(document, fieldName, extracted, false); + processLevel(document, fieldName, extracted); if (extracted.size() == 1) { - return new ExtractedFieldValues(extracted.get(0), documentHasObjectArrays); + return extracted.get(0) ; } - return new ExtractedFieldValues(extracted, documentHasObjectArrays); + return extracted; } @SuppressWarnings("unchecked") - private boolean processLevel(Map level, String field, ArrayList extracted, boolean documentHasObjectArrays) { + private void processLevel(Map level, String field, ArrayList extracted) { if (field.contains(".") == false) { var value = level.get(field); processLeafLevel(value, extracted); - return documentHasObjectArrays; + return; } var nameInLevel = field.split("\\.")[0]; var entry = level.get(nameInLevel); if (entry instanceof Map m) { - return processLevel((Map) m, field.substring(field.indexOf('.') + 1), extracted, documentHasObjectArrays); + processLevel((Map) m, field.substring(field.indexOf('.') + 1), extracted); + return; } if (entry instanceof List l) { for (var object : l) { - processLevel((Map) object, field.substring(field.indexOf('.') + 1), extracted, true); + processLevel((Map) object, field.substring(field.indexOf('.') + 1), extracted); } - return true; + return; } - assert false : "unexpected document structure"; - return false; } private void processLeafLevel(Object value, ArrayList extracted) { From 43455d15be949d8b0c31ecac2642c5c5ba405680 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 6 May 2025 18:17:12 -0500 Subject: [PATCH 02/10] Simplify tests --- .../GeoPointFieldBlockLoaderTests.java | 53 ------------------- 1 file changed, 53 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java index 619cac375df51..fdb1d89175fef 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java @@ -17,7 +17,6 @@ import org.elasticsearch.index.mapper.MappedFieldType; import java.nio.ByteOrder; -import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -106,58 +105,6 @@ private Object exactValuesFromSource(Object value, GeoPoint nullValue, boolean n return maybeFoldList(resultList); } - @Override - protected Object getFieldValue(Map document, String fieldName) { - var extracted = new ArrayList<>(); - processLevel(document, fieldName, extracted); - - if (extracted.size() == 1) { - return extracted.get(0) ; - } - - return extracted; - } - - @SuppressWarnings("unchecked") - private void processLevel(Map level, String field, ArrayList extracted) { - if (field.contains(".") == false) { - var value = level.get(field); - processLeafLevel(value, extracted); - return; - } - - var nameInLevel = field.split("\\.")[0]; - var entry = level.get(nameInLevel); - if (entry instanceof Map m) { - processLevel((Map) m, field.substring(field.indexOf('.') + 1), extracted); - return; - } - if (entry instanceof List l) { - for (var object : l) { - processLevel((Map) object, field.substring(field.indexOf('.') + 1), extracted); - } - return; - } - assert false : "unexpected document structure"; - } - - private void processLeafLevel(Object value, ArrayList extracted) { - if (value instanceof List l) { - if (l.size() > 0 && l.get(0) instanceof Double) { - // this must be a single point in array form - // we'll put it into a different form here to make our lives a bit easier while implementing `expected` - extracted.add(Map.of("type", "point", "coordinates", l)); - } else { - // this is actually an array of points but there could still be points in array form inside - for (var arrayValue : l) { - processLeafLevel(arrayValue, extracted); - } - } - } else { - extracted.add(value); - } - } - @SuppressWarnings("unchecked") private GeoPoint convert(Object value, GeoPoint nullValue, boolean needsMultifieldAdjustment) { if (value == null) { From 462a3cf38fe9859ac0ee0c13d05a27dfd0add167 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 6 May 2025 21:14:31 -0500 Subject: [PATCH 03/10] Update docs/changelog/127795.yaml --- docs/changelog/127795.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/127795.yaml diff --git a/docs/changelog/127795.yaml b/docs/changelog/127795.yaml new file mode 100644 index 0000000000000..b9f78083d29bb --- /dev/null +++ b/docs/changelog/127795.yaml @@ -0,0 +1,5 @@ +pr: 127795 +summary: Do not respect synthetic_source_keep=arrays if type parses arrays +area: Mapping +type: enhancement +issues: [] From 187fd448239483d1fc5fd2549ffc60413f9fc330 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 7 May 2025 08:58:54 -0500 Subject: [PATCH 04/10] Update docs/changelog/127796.yaml --- docs/changelog/127796.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/127796.yaml diff --git a/docs/changelog/127796.yaml b/docs/changelog/127796.yaml new file mode 100644 index 0000000000000..d6f34314406ef --- /dev/null +++ b/docs/changelog/127796.yaml @@ -0,0 +1,5 @@ +pr: 127796 +summary: Do not respect synthetic_source_keep=arrays if type parses arrays +area: Mapping +type: enhancement +issues: [] From 933f9e50958dd4e711da873de63822413122b0bb Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 7 May 2025 09:01:44 -0500 Subject: [PATCH 05/10] Remove changelog from old PR --- docs/changelog/127795.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/127795.yaml diff --git a/docs/changelog/127795.yaml b/docs/changelog/127795.yaml deleted file mode 100644 index b9f78083d29bb..0000000000000 --- a/docs/changelog/127795.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 127795 -summary: Do not respect synthetic_source_keep=arrays if type parses arrays -area: Mapping -type: enhancement -issues: [] From c0cf863752e1490588643181faa044cf4a52a51e Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 7 May 2025 10:10:06 -0500 Subject: [PATCH 06/10] Update docs/changelog/127796.yaml --- docs/changelog/127796.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog/127796.yaml b/docs/changelog/127796.yaml index d6f34314406ef..c87e777f83d40 100644 --- a/docs/changelog/127796.yaml +++ b/docs/changelog/127796.yaml @@ -2,4 +2,5 @@ pr: 127796 summary: Do not respect synthetic_source_keep=arrays if type parses arrays area: Mapping type: enhancement -issues: [] +issues: + - 126155 From d924d2c3be0de8e8ac75fd8a40336400b62237b5 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 7 May 2025 16:26:53 -0500 Subject: [PATCH 07/10] update docs --- .../reference/elasticsearch/mapping-reference/geo-point.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/reference/elasticsearch/mapping-reference/geo-point.md b/docs/reference/elasticsearch/mapping-reference/geo-point.md index faeada7cf15a4..be2f56cb27b7f 100644 --- a/docs/reference/elasticsearch/mapping-reference/geo-point.md +++ b/docs/reference/elasticsearch/mapping-reference/geo-point.md @@ -218,7 +218,9 @@ of official GA features. Synthetic source may sort `geo_point` fields (first by latitude and then -longitude) and reduces them to their stored precision. For example: +longitude) and reduces them to their stored precision. Additionally, unlike most +types, arrays of `geo_point` fields will not store their source even when +`synthetic_source_keep` is set to `arrays`. For example: $$$synthetic-source-geo-point-example$$$ @@ -229,7 +231,8 @@ PUT idx "index": { "mapping": { "source": { - "mode": "synthetic" + "mode": "synthetic", + "synthetic_source_keep": "arrays" } } } From 2cf5c5c65cfd01e20cf544ef82e5df98fe06334a Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 9 May 2025 10:10:30 -0500 Subject: [PATCH 08/10] Docs should use type specific synthetic_source_keep --- .../elasticsearch/mapping-reference/geo-point.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/reference/elasticsearch/mapping-reference/geo-point.md b/docs/reference/elasticsearch/mapping-reference/geo-point.md index be2f56cb27b7f..0b52c40e3314a 100644 --- a/docs/reference/elasticsearch/mapping-reference/geo-point.md +++ b/docs/reference/elasticsearch/mapping-reference/geo-point.md @@ -231,15 +231,17 @@ PUT idx "index": { "mapping": { "source": { - "mode": "synthetic", - "synthetic_source_keep": "arrays" + "mode": "synthetic" } } } }, "mappings": { "properties": { - "point": { "type": "geo_point" } + "point": { + "type": "geo_point", + "synthetic_source_keep": "arrays" + } } } } From 1672338cb299a587f5112828f8ca67c215806667 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 9 May 2025 10:44:13 -0500 Subject: [PATCH 09/10] Update docs/reference/elasticsearch/mapping-reference/geo-point.md Co-authored-by: Oleksandr Kolomiiets --- docs/reference/elasticsearch/mapping-reference/geo-point.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/elasticsearch/mapping-reference/geo-point.md b/docs/reference/elasticsearch/mapping-reference/geo-point.md index 0b52c40e3314a..e5b34c68d79e1 100644 --- a/docs/reference/elasticsearch/mapping-reference/geo-point.md +++ b/docs/reference/elasticsearch/mapping-reference/geo-point.md @@ -219,7 +219,7 @@ of official GA features. Synthetic source may sort `geo_point` fields (first by latitude and then longitude) and reduces them to their stored precision. Additionally, unlike most -types, arrays of `geo_point` fields will not store their source even when +types, arrays of `geo_point` fields will not preserve order `synthetic_source_keep` is set to `arrays`. For example: $$$synthetic-source-geo-point-example$$$ From 9ffebd2104c185b0dc6e9f2dafea0a7a9f2770ce Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 9 May 2025 10:45:04 -0500 Subject: [PATCH 10/10] Swap point order to highlight ordering not preserved --- docs/reference/elasticsearch/mapping-reference/geo-point.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reference/elasticsearch/mapping-reference/geo-point.md b/docs/reference/elasticsearch/mapping-reference/geo-point.md index e5b34c68d79e1..c26b8ecfa5eb7 100644 --- a/docs/reference/elasticsearch/mapping-reference/geo-point.md +++ b/docs/reference/elasticsearch/mapping-reference/geo-point.md @@ -219,7 +219,7 @@ of official GA features. Synthetic source may sort `geo_point` fields (first by latitude and then longitude) and reduces them to their stored precision. Additionally, unlike most -types, arrays of `geo_point` fields will not preserve order +types, arrays of `geo_point` fields will not preserve order if `synthetic_source_keep` is set to `arrays`. For example: $$$synthetic-source-geo-point-example$$$ @@ -248,8 +248,8 @@ PUT idx PUT idx/_doc/1 { "point": [ - {"lat":-90, "lon":-80}, - {"lat":10, "lon":30} + {"lat":10, "lon":30}, + {"lat":-90, "lon":-80} ] } ```