Skip to content

Adding check for isIndexed in text fields when generating field exists queries to avoid ISE when field is stored but not indexed or with doc_values #130531

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/130531.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 130531
summary: Adding check for `isIndexed` in text fields when generating field exists
queries to avoid ISE when field is stored but not indexed or with `doc_values`
area: Analysis
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ setup:
type: keyword
text:
type: text
text_stored_not_indexed:
type: text
store: true
index: false

- do:
headers:
Expand All @@ -70,6 +74,7 @@ setup:
inner1: "foo"
inner2: "bar"
text: "foo bar"
text_stored_not_indexed: "foo bar"

- do:
headers:
Expand All @@ -94,6 +99,7 @@ setup:
object:
inner1: "foo"
text: "foo bar"
text_stored_not_indexed: "foo bar"

- do:
headers:
Expand All @@ -119,6 +125,7 @@ setup:
object:
inner2: "bar"
text: "foo bar"
text_stored_not_indexed: "foo bar"

- do:
index:
Expand Down Expand Up @@ -184,6 +191,12 @@ setup:
doc_values: false
text:
type: text
keyword_stored_norms_not_indexed:
type: keyword
doc_values: false
index: false
store: true
norms: true

- do:
headers:
Expand All @@ -209,6 +222,7 @@ setup:
inner1: "foo"
inner2: "bar"
text: "foo bar"
keyword_stored_norms_not_indexed: "foo bar"

- do:
headers:
Expand All @@ -233,6 +247,7 @@ setup:
object:
inner1: "foo"
text: "foo bar"
keyword_stored_norms_not_indexed: "foo bar"

- do:
headers:
Expand All @@ -258,6 +273,7 @@ setup:
object:
inner2: "bar"
text: "foo bar"
keyword_stored_norms_not_indexed: "foo bar"

- do:
index:
Expand Down Expand Up @@ -1268,3 +1284,48 @@ setup:
field: text

- match: {hits.total: 1}

---
"Test exists query on text field with no dv, that is stored but not indexed":
- requires:
capabilities:
- method: POST
path: /_search
capabilities: [ field_exists_query_for_text_fields_no_index_or_dv ]
test_runner_features: capabilities
reason: "Before the fix, this query would throw an ISE because the field is not indexed and has no doc values."

- do:
search:
rest_total_hits_as_int: true
index: test
body:
query:
exists:
field: text_stored_not_indexed

# this should not throw, but rather return 0 hits, as the field is not indexed nor it has doc values
- match: {hits.total: 0}


---
"Test exists query on keyword field with no dv, that is stored, with norms, but not indexed":
- requires:
capabilities:
- method: POST
path: /_search
capabilities: [ field_exists_query_for_text_fields_no_index_or_dv ]
test_runner_features: capabilities
reason: "Before the fix, this query would throw an ISE because the field is not indexed and has no doc values."

- do:
search:
rest_total_hits_as_int: true
index: test-no-dv
body:
query:
exists:
field: keyword_stored_norms_not_indexed

# this should not throw, but rather return 0 hits, as the field is not indexed nor it has doc values
- match: {hits.total: 0}
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ public Query regexpQuery(
}

public Query existsQuery(SearchExecutionContext context) {
if (hasDocValues() || getTextSearchInfo().hasNorms()) {
if (hasDocValues() || (isIndexed() && getTextSearchInfo().hasNorms())) {
return new FieldExistsQuery(name());
} else {
return new TermQuery(new Term(FieldNamesFieldMapper.NAME, name()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ private SearchCapabilities() {}
private static final String SIGNIFICANT_TERMS_ON_NESTED_FIELDS = "significant_terms_on_nested_fields";
private static final String EXCLUDE_VECTORS_PARAM = "exclude_vectors_param";
private static final String DENSE_VECTOR_UPDATABLE_BBQ = "dense_vector_updatable_bbq";
private static final String FIELD_EXISTS_QUERY_FOR_TEXT_FIELDS_NO_INDEX_OR_DV = "field_exists_query_for_text_fields_no_index_or_dv";

public static final Set<String> CAPABILITIES;
static {
Expand All @@ -75,6 +76,7 @@ private SearchCapabilities() {}
capabilities.add(SIGNIFICANT_TERMS_ON_NESTED_FIELDS);
capabilities.add(EXCLUDE_VECTORS_PARAM);
capabilities.add(DENSE_VECTOR_UPDATABLE_BBQ);
capabilities.add(FIELD_EXISTS_QUERY_FOR_TEXT_FIELDS_NO_INDEX_OR_DV);
CAPABILITIES = Set.copyOf(capabilities);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ public void testExistsQuery() {
FieldType fieldType = new FieldType();
fieldType.setOmitNorms(false);
KeywordFieldType ft = new KeywordFieldType("field", fieldType);
assertEquals(new FieldExistsQuery("field"), ft.existsQuery(MOCK_CONTEXT));
// updated in #130531 so that a field that is neither indexed nor has doc values will generate a TermQuery
// to avoid ISE from FieldExistsQuery
assertEquals(new TermQuery(new Term(FieldNamesFieldMapper.NAME, "field")), ft.existsQuery(MOCK_CONTEXT));
}
{
KeywordFieldType ft = new KeywordFieldType("field", true, false, Collections.emptyMap());
Expand Down
Loading