Skip to content

Exclude unmapped fields during max clause limit checking for querying #49523

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.

Sign up for GitHub

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 12 commits into from
Jan 20, 2020
Merged

Exclude unmapped fields during max clause limit checking for querying #49523

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
81b1256
Exclude unmapped fileds during max clause limit checking for querying
zacharymorn Nov 21, 2019
79df51c
Address comments
zacharymorn Nov 26, 2019
4268745
Remove unneeded changes in elasticsearch.yml
zacharymorn Nov 28, 2019
b4279d1
Add integration test for igoring unmapping fields
zacharymorn Jan 10, 2020
60efd2c
Remove unit tests and unneeded changes to integration test
zacharymorn Jan 14, 2020
abbb18d
Merge branch 'master' into issue-49002
elasticmachine Jan 15, 2020
0c72d0b
Merge branch 'master' into issue-49002
zacharymorn Jan 16, 2020
fe53f2d
Update test to use CreateIndexRequestBuilder.setMapping after merge f…
zacharymorn Jan 16, 2020
6804d83
Merge branch 'master' into issue-49002
zacharymorn Jan 16, 2020
96436a3
Merge branch 'issue-49002' of https://github.com/zacharymorn/elastics…
zacharymorn Jan 16, 2020
a6ef57d
ix test failure as type has been removed in CreateIndexRequest
zacharymorn Jan 16, 2020
4ef62f5
Merge branch 'master' into issue-49002
elasticmachine Jan 17, 2020
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
7 changes: 3 additions & 4 deletions server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ public static Map<String, Float> resolveMappingFields(QueryShardContext context,
boolean allField = Regex.isMatchAllPattern(fieldEntry.getKey());
boolean multiField = Regex.isSimpleMatchPattern(fieldEntry.getKey());
float weight = fieldEntry.getValue() == null ? 1.0f : fieldEntry.getValue();
Map<String, Float> fieldMap = resolveMappingField(context, fieldEntry.getKey(), weight,
!multiField, !allField, fieldSuffix);
Map<String, Float> fieldMap = resolveMappingField(context, fieldEntry.getKey(), weight, !multiField, !allField, fieldSuffix);

for (Map.Entry<String, Float> field : fieldMap.entrySet()) {
float boost = field.getValue();
if (resolvedFields.containsKey(field.getKey())) {
Expand All @@ -97,6 +97,7 @@ public static Map<String, Float> resolveMappingFields(QueryShardContext context,
resolvedFields.put(field.getKey(), boost);
}
}

checkForTooManyFields(resolvedFields, context);
return resolvedFields;
}
Expand Down Expand Up @@ -141,8 +142,6 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,

MappedFieldType fieldType = context.getMapperService().fullName(fieldName);
if (fieldType == null) {
// Note that we don't ignore unmapped fields.
fields.put(fieldName, weight);
continue;
}

Expand Down
29 changes: 29 additions & 0 deletions server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,35 @@ public void testLimitOnExpandedFields() throws Exception {
+ (CLUSTER_MAX_CLAUSE_COUNT + 1)));
}

// The only expectation for this test is to not throw exception
public void testLimitOnExpandedFieldsButIgnoreUnmappedFields() throws Exception {
XContentBuilder builder = jsonBuilder();
builder.startObject();
builder.startObject("type1");
builder.startObject("properties");
for (int i = 0; i < CLUSTER_MAX_CLAUSE_COUNT; i++) {
builder.startObject("field" + i).field("type", "text").endObject();
}
builder.endObject(); // properties
builder.endObject(); // type1
builder.endObject();

assertAcked(prepareCreate("ignoreunmappedfields").addMapping("type1", builder));
Copy link
Contributor

@jimczi jimczi Jan 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zacharymorn I merged master in your pr and now there's a test compilation error. Can you take a look ?

Copy link
Contributor Author

@zacharymorn zacharymorn Jan 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged master in and looks like there’s change in CreateIndexRequestBuilder.addMapping . Just updated the test and pushed up the commits. The CI is still failing though it was broken for an seemingly unrelated integration test org.elasticsearch.snapshots.DedicatedClusterSnapshotRestoreIT.testMasterShutdownDuringSnapshot. Is this something I should be looking into as well ?


client().prepareIndex("ignoreunmappedfields").setId("1").setSource("field1", "foo bar baz").get();
refresh();

QueryStringQueryBuilder qb = queryStringQuery("bar");
if (randomBoolean()) {
qb.field("*")
.field("unmappedField1")
.field("unmappedField2")
.field("unmappedField3")
.field("unmappedField4");
}
client().prepareSearch("ignoreunmappedfields").setQuery(qb).get();
}

public void testFieldAlias() throws Exception {
List<IndexRequestBuilder> indexRequests = new ArrayList<>();
indexRequests.add(client().prepareIndex("test").setId("1").setSource("f3", "text", "f2", "one"));
Expand Down