Correct the condition to use a multi field block loader in text field block loader (#126718)

This commit is contained in:
Oleksandr Kolomiiets 2025-04-22 09:01:05 -07:00 committed by GitHub
parent 962e953a66
commit 093ebd356b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 17 additions and 22 deletions

View file

@ -967,12 +967,11 @@ public final class TextFieldMapper extends FieldMapper {
} }
/** /**
* Returns true if the delegate sub-field can be used for loading and querying (ie. either isIndexed or isStored is true) * Returns true if the delegate sub-field can be used for loading.
* A delegate by definition must have doc_values or be stored so most of the time it can be used for loading.
*/ */
public boolean canUseSyntheticSourceDelegateForLoading() { public boolean canUseSyntheticSourceDelegateForLoading() {
return syntheticSourceDelegate != null return syntheticSourceDelegate != null && syntheticSourceDelegate.ignoreAbove() == Integer.MAX_VALUE;
&& syntheticSourceDelegate.ignoreAbove() == Integer.MAX_VALUE
&& (syntheticSourceDelegate.isIndexed() || syntheticSourceDelegate.isStored());
} }
/** /**

View file

@ -35,20 +35,20 @@ public class TextFieldBlockLoaderTests extends BlockLoaderTestCase {
var fields = (Map<String, Object>) fieldMapping.get("fields"); var fields = (Map<String, Object>) fieldMapping.get("fields");
if (fields != null) { if (fields != null) {
var keywordMultiFieldMapping = (Map<String, Object>) fields.get("kwd"); var keywordMultiFieldMapping = (Map<String, Object>) fields.get("kwd");
Object normalizer = fields.get("normalizer");
boolean docValues = hasDocValues(keywordMultiFieldMapping, true); boolean docValues = hasDocValues(keywordMultiFieldMapping, true);
boolean index = keywordMultiFieldMapping.getOrDefault("index", true).equals(true);
boolean store = keywordMultiFieldMapping.getOrDefault("store", false).equals(true); boolean store = keywordMultiFieldMapping.getOrDefault("store", false).equals(true);
Object ignoreAbove = keywordMultiFieldMapping.get("ignore_above"); Object ignoreAbove = keywordMultiFieldMapping.get("ignore_above");
// See TextFieldMapper.SyntheticSourceHelper#usingSyntheticSourceDelegate // See TextFieldMapper.SyntheticSourceHelper#getKeywordFieldMapperForSyntheticSource
// and TextFieldMapper#canUseSyntheticSourceDelegateForLoading(). // and TextFieldMapper#canUseSyntheticSourceDelegateForLoading().
boolean usingSyntheticSourceDelegate = docValues || store; boolean usingSyntheticSourceDelegate = normalizer == null && (docValues || store);
boolean canUseSyntheticSourceDelegateForLoading = usingSyntheticSourceDelegate && ignoreAbove == null && (index || store); boolean canUseSyntheticSourceDelegateForLoading = usingSyntheticSourceDelegate && ignoreAbove == null;
if (canUseSyntheticSourceDelegateForLoading) { if (canUseSyntheticSourceDelegateForLoading) {
return KeywordFieldBlockLoaderTests.expectedValue(keywordMultiFieldMapping, value, params, testContext); return KeywordFieldBlockLoaderTests.expectedValue(keywordMultiFieldMapping, value, params, testContext);
} }
// Even if multi-field is not eligible for loading it can still be used to produce synthetic source // Even if multi field is not eligible for loading it can still be used to produce synthetic source
// and then we load from the synthetic source. // and then we load from the synthetic source.
// Synthetic source is actually different from keyword field block loader results // Synthetic source is actually different from keyword field block loader results
// because synthetic source includes values exceeding ignore_above and block loader doesn't. // because synthetic source includes values exceeding ignore_above and block loader doesn't.
@ -66,9 +66,7 @@ public class TextFieldBlockLoaderTests extends BlockLoaderTestCase {
boolean textFieldIndexed = (boolean) fieldMapping.getOrDefault("index", true); boolean textFieldIndexed = (boolean) fieldMapping.getOrDefault("index", true);
if (value == null) { if (value == null) {
if (textFieldIndexed == false if (textFieldIndexed == false && nullValue != null && nullValue.length() <= (int) ignoreAbove) {
&& nullValue != null
&& (ignoreAbove == null || nullValue.length() <= (int) ignoreAbove)) {
return new BytesRef(nullValue); return new BytesRef(nullValue);
} }
@ -89,7 +87,7 @@ public class TextFieldBlockLoaderTests extends BlockLoaderTestCase {
var indexed = values.stream() var indexed = values.stream()
.map(s -> s == null ? nullValue : s) .map(s -> s == null ? nullValue : s)
.filter(Objects::nonNull) .filter(Objects::nonNull)
.filter(s -> ignoreAbove == null || s.length() <= (int) ignoreAbove) .filter(s -> s.length() <= (int) ignoreAbove)
.map(BytesRef::new) .map(BytesRef::new)
.collect(Collectors.toList()); .collect(Collectors.toList());
@ -100,14 +98,12 @@ public class TextFieldBlockLoaderTests extends BlockLoaderTestCase {
} }
// ignored values always come last // ignored values always come last
List<BytesRef> ignored = ignoreAbove == null List<BytesRef> ignored = values.stream()
? List.of() .map(s -> s == null ? nullValue : s)
: values.stream() .filter(Objects::nonNull)
.map(s -> s == null ? nullValue : s) .filter(s -> s.length() > (int) ignoreAbove)
.filter(Objects::nonNull) .map(BytesRef::new)
.filter(s -> s.length() > (int) ignoreAbove) .toList();
.map(BytesRef::new)
.toList();
indexed.addAll(ignored); indexed.addAll(ignored);
@ -115,7 +111,7 @@ public class TextFieldBlockLoaderTests extends BlockLoaderTestCase {
} }
} }
// Loading from _ignored_source or stored _source // Loading from stored field, _ignored_source or stored _source
return valuesInSourceOrder(value); return valuesInSourceOrder(value);
} }