Skip to content

Commit 637baa9

Browse files
authored
[Query Rules] Fix bug where combining the same metadata with text/numeric values leads to error (#102891) (#102923)
* Fix issue where query rule criteria with matching metadata but different types returns error * Update docs/changelog/102891.yaml
1 parent 8adf56c commit 637baa9

File tree

5 files changed

+65
-4
lines changed

5 files changed

+65
-4
lines changed

docs/changelog/102891.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 102891
2+
summary: "[Query Rules] Fix bug where combining the same metadata with text/numeric\
3+
\ values leads to error"
4+
area: Application
5+
type: bug
6+
issues:
7+
- 102827

x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/260_rule_query_search.yml

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,4 +194,46 @@ setup:
194194
- match: { hits.hits.0._id: 'doc2' }
195195
- match: { hits.hits.1._id: 'doc3' }
196196

197+
---
198+
"Perform a rule query over a ruleset with combined numeric and text rule matching":
199+
200+
- do:
201+
query_ruleset.put:
202+
ruleset_id: combined-ruleset
203+
body:
204+
rules:
205+
- rule_id: rule1
206+
type: pinned
207+
criteria:
208+
- type: fuzzy
209+
metadata: foo
210+
values: [ bar ]
211+
actions:
212+
ids:
213+
- 'doc1'
214+
- rule_id: rule2
215+
type: pinned
216+
criteria:
217+
- type: lte
218+
metadata: foo
219+
values: [ 100 ]
220+
actions:
221+
ids:
222+
- 'doc2'
223+
- do:
224+
search:
225+
body:
226+
query:
227+
rule_query:
228+
organic:
229+
query_string:
230+
default_field: text
231+
query: blah blah blah
232+
match_criteria:
233+
foo: baz
234+
ruleset_id: combined-ruleset
235+
236+
- match: { hits.total.value: 1 }
237+
- match: { hits.hits.0._id: 'doc1' }
238+
197239

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ public AppliedQueryRules applyRule(AppliedQueryRules appliedRules, Map<String, O
298298
final String criteriaMetadata = criterion.criteriaMetadata();
299299

300300
if (criteriaType == ALWAYS || (criteriaMetadata != null && criteriaMetadata.equals(match))) {
301-
boolean singleCriterionMatches = criterion.isMatch(matchValue, criteriaType);
301+
boolean singleCriterionMatches = criterion.isMatch(matchValue, criteriaType, false);
302302
isRuleMatch = (isRuleMatch == null) ? singleCriterionMatches : isRuleMatch && singleCriterionMatches;
303303
}
304304
}

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRuleCriteria.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,12 +191,19 @@ public String toString() {
191191
}
192192

193193
public boolean isMatch(Object matchValue, QueryRuleCriteriaType matchType) {
194+
return isMatch(matchValue, matchType, true);
195+
}
196+
197+
public boolean isMatch(Object matchValue, QueryRuleCriteriaType matchType, boolean throwOnInvalidInput) {
194198
if (matchType == ALWAYS) {
195199
return true;
196200
}
197201
final String matchString = matchValue.toString();
198202
for (Object criteriaValue : criteriaValues) {
199-
matchType.validateInput(matchValue);
203+
boolean isValid = matchType.validateInput(matchValue, throwOnInvalidInput);
204+
if (isValid == false) {
205+
return false;
206+
}
200207
boolean matchFound = matchType.isMatch(matchString, criteriaValue);
201208
if (matchFound) {
202209
return true;

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRuleCriteriaType.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,16 @@ public boolean isMatch(Object input, Object criteriaValue) {
8686
}
8787
};
8888

89-
public void validateInput(Object input) {
89+
public boolean validateInput(Object input, boolean throwOnInvalidInput) {
9090
boolean isValid = isValidForInput(input);
91-
if (isValid == false) {
91+
if (isValid == false && throwOnInvalidInput) {
9292
throw new IllegalArgumentException("Input [" + input + "] is not valid for CriteriaType [" + this + "]");
9393
}
94+
return isValid;
95+
}
96+
97+
public boolean validateInput(Object input) {
98+
return validateInput(input, true);
9499
}
95100

96101
public abstract boolean isMatch(Object input, Object criteriaValue);

0 commit comments

Comments
 (0)