-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Allow docvalues-only search on geo_shape #94396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pr: 94396 | ||
summary: Allow docvalues-only search on `geo_shape` | ||
area: Geo | ||
type: enhancement | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,17 +197,23 @@ public String typeName() { | |
|
||
@Override | ||
public Query geoShapeQuery(SearchExecutionContext context, String fieldName, ShapeRelation relation, LatLonGeometry... geometries) { | ||
failIfNotIndexedNorDocValuesFallback(context); | ||
// CONTAINS queries are not supported by VECTOR strategy for indices created before version 7.5.0 (Lucene 8.3.0) | ||
if (relation == ShapeRelation.CONTAINS && context.indexVersionCreated().before(Version.V_7_5_0)) { | ||
throw new QueryShardException( | ||
context, | ||
ShapeRelation.CONTAINS + " query relation not supported for Field [" + fieldName + "]." | ||
); | ||
} | ||
Query query = LatLonShape.newGeometryQuery(fieldName, relation.getLuceneRelation(), geometries); | ||
if (hasDocValues()) { | ||
final Query queryDocValues = new LatLonShapeDocValuesQuery(fieldName, relation.getLuceneRelation(), geometries); | ||
query = new IndexOrDocValuesQuery(query, queryDocValues); | ||
Query query; | ||
if (isIndexed()) { | ||
query = LatLonShape.newGeometryQuery(fieldName, relation.getLuceneRelation(), geometries); | ||
if (hasDocValues()) { | ||
final Query queryDocValues = new LatLonShapeDocValuesQuery(fieldName, relation.getLuceneRelation(), geometries); | ||
query = new IndexOrDocValuesQuery(query, queryDocValues); | ||
} | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if both indexed and docValues are false? Or perhaps that combination is disallowed? The docs you added do not specify that at least one needs to be positive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking more carefully I see that null will be returned as the query, so perhaps this case is handled by the caller? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case is handle by |
||
query = new LatLonShapeDocValuesQuery(fieldName, relation.getLuceneRelation(), geometries); | ||
} | ||
return query; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
setup: | ||
- do: | ||
indices.create: | ||
index: shapes | ||
body: | ||
mappings: | ||
properties: | ||
default: | ||
type: geo_shape | ||
no_doc_values: | ||
type: geo_shape | ||
doc_values: false | ||
no_index: | ||
type: geo_shape | ||
index: false | ||
no_doc_values_no_index: | ||
type: geo_shape | ||
doc_values: false | ||
index: false | ||
- do: | ||
bulk: | ||
refresh: true | ||
body: | ||
- index: | ||
_index: shapes | ||
_id: 1 | ||
- '{"default": "POINT(4.912350 52.374081)", "no_doc_values": "POINT(4.912350 52.374081)", "no_index": "POINT(4.912350 52.374081)", "no_doc_values_no_index": "POINT(4.912350 52.374081)"}' | ||
- index: | ||
_index: shapes | ||
_id: 2 | ||
- '{"default": "POINT(2.327000 48.860000)", "no_doc_values": "POINT(2.327000 48.860000)", "no_index": "POINT(2.327000 48.860000)", "no_doc_values_no_index": "POINT(2.327000 48.860000)"}' | ||
- do: | ||
indices.refresh: {} | ||
|
||
--- | ||
"Test field mapping": | ||
- do: | ||
indices.get_mapping: | ||
index: shapes | ||
|
||
- match: {shapes.mappings.properties.default.type: geo_shape } | ||
- match: {shapes.mappings.properties.no_doc_values.type: geo_shape } | ||
- match: {shapes.mappings.properties.no_doc_values.doc_values: false } | ||
- match: {shapes.mappings.properties.no_index.type: geo_shape } | ||
- match: {shapes.mappings.properties.no_index.index: false } | ||
- match: {shapes.mappings.properties.no_doc_values_no_index.type: geo_shape } | ||
- match: {shapes.mappings.properties.no_doc_values_no_index.index: false } | ||
- match: {shapes.mappings.properties.no_doc_values_no_index.doc_values: false } | ||
|
||
--- | ||
"Test query default field": | ||
- do: | ||
search: | ||
index: shapes | ||
body: | ||
query: | ||
geo_bounding_box: | ||
default: | ||
top_left: | ||
lat: 55 | ||
lon: 4 | ||
bottom_right: | ||
lat: 50 | ||
lon: 5 | ||
|
||
- match: { hits.total.value: 1 } | ||
|
||
|
||
--- | ||
"Test query no_doc_values field": | ||
- do: | ||
search: | ||
index: shapes | ||
body: | ||
query: | ||
geo_bounding_box: | ||
no_doc_values: | ||
top_left: | ||
lat: 55 | ||
lon: 4 | ||
bottom_right: | ||
lat: 50 | ||
lon: 5 | ||
|
||
- match: { hits.total.value: 1 } | ||
|
||
|
||
--- | ||
"Test query no_index field": | ||
- do: | ||
search: | ||
index: shapes | ||
body: | ||
query: | ||
geo_bounding_box: | ||
no_index: | ||
top_left: | ||
lat: 55 | ||
lon: 4 | ||
bottom_right: | ||
lat: 50 | ||
lon: 5 | ||
|
||
- match: { hits.total.value: 1 } | ||
|
||
--- | ||
"Test query no_doc_values_no_index field": | ||
- do: | ||
catch: bad_request | ||
search: | ||
index: shapes | ||
body: | ||
query: | ||
geo_bounding_box: | ||
no_doc_values_no_index: | ||
top_left: | ||
lat: 55 | ||
lon: 4 | ||
bottom_right: | ||
lat: 50 | ||
lon: 5 | ||
|
||
- match: {error.type: search_phase_execution_exception} | ||
- match: {error.reason: "all shards failed"} | ||
- match: {error.phase: query} | ||
- match: {error.failed_shards.0.reason.type: query_shard_exception} | ||
- match: {error.failed_shards.0.reason.reason: "failed to create query: Cannot search on field [no_doc_values_no_index] since it is not indexed nor has doc values."} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. I see there is already error handling for the case I was worried about above. Perhaps a comment in the docs is a good idea? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something generic and I don't think we should explicitly document it on this field mapping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can both
index
anddoc_values
be set tofalse
?