Skip to content

Commit 2c3bee7

Browse files
Kubik42Kubik42
authored
Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading (#134253)
* Added keyword multi field with ignore_above to match only text bwc tests * Rework ignore_above * Added unit tests * Undo changed to match only text bwc test * formatting * Removed indexMode from field type * Added another test case * Fixed failing bwc tests * Improved msg * Added additional tests * Added IgnoreAbove record, addressed index-level ignore above * Fixed test typos * Added IgnoreAboveTest * Enforce at least one value or defaultValue to always be non-null when IgnoreAbove is initialized * When initializing IgnoreAbove, dont use defaultValue from builder - this fixes failing BWC test * Fixed typo * Switched IgnoreAbove to constructor only, removed the ability to set default directly * Update docs/changelog/134253.yaml * Update 134253.yaml * Move IgnoreAbove into Mapper and make it final, move everything new out of IndexSettings and into IgnoreAbove * Fixed typo * Added helpful constructor to IgnoreAbove * Added helpful constructor to IgnoreAbove
1 parent 0a6576a commit 2c3bee7

File tree

21 files changed

+1177
-147
lines changed

21 files changed

+1177
-147
lines changed

‎docs/changelog/134253.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 134253
2+
summary: Fixed a bug where text fields in LogsDB indices did not use their keyword multi fields for block loading
3+
area: Mapping
4+
type: bug
5+
issues: []

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,7 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
253253
String parentField = searchExecutionContext.parentPath(name());
254254
var parent = searchExecutionContext.lookup().fieldType(parentField);
255255

256-
if (parent instanceof KeywordFieldMapper.KeywordFieldType keywordParent
257-
&& keywordParent.ignoreAbove() != Integer.MAX_VALUE) {
256+
if (parent instanceof KeywordFieldMapper.KeywordFieldType keywordParent && keywordParent.ignoreAbove().isSet()) {
258257
if (parent.isStored()) {
259258
return storedFieldFetcher(parentField, keywordParent.originalName());
260259
} else if (parent.hasDocValues()) {
@@ -278,7 +277,7 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
278277
if (kwd != null) {
279278
var fieldType = kwd.fieldType();
280279

281-
if (fieldType.ignoreAbove() != Integer.MAX_VALUE) {
280+
if (fieldType.ignoreAbove().isSet()) {
282281
if (fieldType.isStored()) {
283282
return storedFieldFetcher(fieldType.name(), fieldType.originalName());
284283
} else if (fieldType.hasDocValues()) {

‎plugins/analysis-icu/src/main/java/org/elasticsearch/plugin/analysis/icu/ICUCollationKeywordFieldMapper.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,12 +250,10 @@ public static class Builder extends FieldMapper.Builder {
250250
false
251251
).acceptsNull();
252252

253-
final Parameter<Integer> ignoreAbove = Parameter.intParam("ignore_above", true, m -> toType(m).ignoreAbove, Integer.MAX_VALUE)
254-
.addValidator(v -> {
255-
if (v < 0) {
256-
throw new IllegalArgumentException("[ignore_above] must be positive, got [" + v + "]");
257-
}
258-
});
253+
final Parameter<Integer> ignoreAbove = Parameter.ignoreAboveParam(
254+
m -> toType(m).ignoreAbove,
255+
IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE
256+
);
259257
final Parameter<String> nullValue = Parameter.stringParam("null_value", false, m -> toType(m).nullValue, null).acceptsNull();
260258

261259
public Builder(String name) {

‎server/src/main/java/org/elasticsearch/index/IndexSettings.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -805,20 +805,21 @@ public Iterator<Setting<?>> settings() {
805805

806806
public static final Setting<Integer> IGNORE_ABOVE_SETTING = Setting.intSetting(
807807
"index.mapping.ignore_above",
808-
IndexSettings::getIgnoreAboveDefaultValue,
808+
settings -> String.valueOf(getIgnoreAboveDefaultValue(settings)),
809809
0,
810810
Integer.MAX_VALUE,
811811
Property.IndexScope,
812812
Property.ServerlessPublic
813813
);
814814

815-
private static String getIgnoreAboveDefaultValue(final Settings settings) {
816-
if (IndexSettings.MODE.get(settings) == IndexMode.LOGSDB
817-
&& IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(IndexVersions.ENABLE_IGNORE_ABOVE_LOGSDB)) {
818-
return "8191";
819-
} else {
820-
return String.valueOf(Integer.MAX_VALUE);
815+
private static int getIgnoreAboveDefaultValue(final Settings settings) {
816+
if (settings == null) {
817+
return Mapper.IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE;
821818
}
819+
return Mapper.IgnoreAbove.getIgnoreAboveDefaultValue(
820+
IndexSettings.MODE.get(settings),
821+
IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings)
822+
);
822823
}
823824

824825
public static final Setting<SeqNoFieldMapper.SeqNoIndexOptions> SEQ_NO_INDEX_OPTIONS_SETTING = Setting.enumSetting(

‎server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,14 @@ public static Parameter<Boolean> normsParam(Function<FieldMapper, Boolean> initi
13371337
.setMergeValidator((prev, curr, c) -> prev == curr || (prev && curr == false));
13381338
}
13391339

1340+
public static Parameter<Integer> ignoreAboveParam(Function<FieldMapper, Integer> initializer, int defaultValue) {
1341+
return Parameter.intParam("ignore_above", true, initializer, defaultValue).addValidator(v -> {
1342+
if (v < 0) {
1343+
throw new IllegalArgumentException("[ignore_above] must be positive, got [" + v + "]");
1344+
}
1345+
});
1346+
}
1347+
13401348
/**
13411349
* Defines a script parameter
13421350
* @param initializer retrieves the equivalent parameter from an existing FieldMapper for use in merges

‎server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java‎

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
import static org.elasticsearch.index.IndexSettings.IGNORE_ABOVE_SETTING;
9393
import static org.elasticsearch.index.IndexSettings.USE_DOC_VALUES_SKIPPER;
9494
import static org.elasticsearch.index.mapper.FieldArrayContext.getOffsetsFieldName;
95+
import static org.elasticsearch.index.mapper.Mapper.IgnoreAbove.getIgnoreAboveDefaultValue;
9596

9697
/**
9798
* A field mapper for keywords. This mapper accepts strings and indexes them as-is.
@@ -232,15 +233,14 @@ public Builder(final String name, final MappingParserContext mappingParserContex
232233
String name,
233234
IndexAnalyzers indexAnalyzers,
234235
ScriptCompiler scriptCompiler,
235-
int ignoreAboveDefault,
236236
IndexVersion indexCreatedVersion,
237237
SourceKeepMode sourceKeepMode
238238
) {
239239
this(
240240
name,
241241
indexAnalyzers,
242242
scriptCompiler,
243-
ignoreAboveDefault,
243+
getIgnoreAboveDefaultValue(IndexMode.STANDARD, indexCreatedVersion),
244244
indexCreatedVersion,
245245
IndexMode.STANDARD,
246246
null,
@@ -289,12 +289,7 @@ private Builder(
289289
}
290290
}).precludesParameters(normalizer);
291291
this.ignoreAboveDefault = ignoreAboveDefault;
292-
this.ignoreAbove = Parameter.intParam("ignore_above", true, m -> toType(m).fieldType().ignoreAbove(), ignoreAboveDefault)
293-
.addValidator(v -> {
294-
if (v < 0) {
295-
throw new IllegalArgumentException("[ignore_above] must be positive, got [" + v + "]");
296-
}
297-
});
292+
this.ignoreAbove = Parameter.ignoreAboveParam(m -> toType(m).fieldType().ignoreAbove().get(), ignoreAboveDefault);
298293
this.indexSortConfig = indexSortConfig;
299294
this.indexMode = indexMode;
300295
this.enableDocValuesSkipper = enableDocValuesSkipper;
@@ -303,7 +298,7 @@ private Builder(
303298
}
304299

305300
public Builder(String name, IndexVersion indexCreatedVersion) {
306-
this(name, null, ScriptCompiler.NONE, Integer.MAX_VALUE, indexCreatedVersion, SourceKeepMode.NONE);
301+
this(name, null, ScriptCompiler.NONE, indexCreatedVersion, SourceKeepMode.NONE);
307302
}
308303

309304
public static Builder buildWithDocValuesSkipper(
@@ -316,7 +311,7 @@ public static Builder buildWithDocValuesSkipper(
316311
name,
317312
null,
318313
ScriptCompiler.NONE,
319-
Integer.MAX_VALUE,
314+
getIgnoreAboveDefaultValue(indexMode, indexCreatedVersion),
320315
indexCreatedVersion,
321316
indexMode,
322317
// Sort config is used to decide if DocValueSkippers can be used. Since skippers are forced, a sort config is not needed.
@@ -537,14 +532,15 @@ private static boolean indexSortConfigByHostName(final IndexSortConfig indexSort
537532

538533
public static final class KeywordFieldType extends StringFieldType {
539534

540-
private final int ignoreAbove;
535+
private static final IgnoreAbove IGNORE_ABOVE_DEFAULT = new IgnoreAbove(null, IndexMode.STANDARD);
536+
537+
private final IgnoreAbove ignoreAbove;
541538
private final String nullValue;
542539
private final NamedAnalyzer normalizer;
543540
private final boolean eagerGlobalOrdinals;
544541
private final FieldValues<String> scriptValues;
545542
private final boolean isDimension;
546543
private final boolean isSyntheticSource;
547-
private final IndexMode indexMode;
548544
private final IndexSortConfig indexSortConfig;
549545
private final boolean hasDocValuesSkipper;
550546
private final String originalName;
@@ -568,36 +564,34 @@ public KeywordFieldType(
568564
);
569565
this.eagerGlobalOrdinals = builder.eagerGlobalOrdinals.getValue();
570566
this.normalizer = normalizer;
571-
this.ignoreAbove = builder.ignoreAbove.getValue();
567+
this.ignoreAbove = new IgnoreAbove(builder.ignoreAbove.getValue(), builder.indexMode, builder.indexCreatedVersion);
572568
this.nullValue = builder.nullValue.getValue();
573569
this.scriptValues = builder.scriptValues();
574570
this.isDimension = builder.dimension.getValue();
575571
this.isSyntheticSource = isSyntheticSource;
576-
this.indexMode = builder.indexMode;
577572
this.indexSortConfig = builder.indexSortConfig;
578573
this.hasDocValuesSkipper = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false;
579574
this.originalName = isSyntheticSource ? name + "._original" : null;
580575
}
581576

577+
public KeywordFieldType(String name) {
578+
this(name, true, true, Collections.emptyMap());
579+
}
580+
582581
public KeywordFieldType(String name, boolean isIndexed, boolean hasDocValues, Map<String, String> meta) {
583582
super(name, isIndexed, false, hasDocValues, TextSearchInfo.SIMPLE_MATCH_ONLY, meta);
584583
this.normalizer = Lucene.KEYWORD_ANALYZER;
585-
this.ignoreAbove = Integer.MAX_VALUE;
584+
this.ignoreAbove = IGNORE_ABOVE_DEFAULT;
586585
this.nullValue = null;
587586
this.eagerGlobalOrdinals = false;
588587
this.scriptValues = null;
589588
this.isDimension = false;
590589
this.isSyntheticSource = false;
591-
this.indexMode = IndexMode.STANDARD;
592590
this.indexSortConfig = null;
593591
this.hasDocValuesSkipper = false;
594592
this.originalName = null;
595593
}
596594

597-
public KeywordFieldType(String name) {
598-
this(name, true, true, Collections.emptyMap());
599-
}
600-
601595
public KeywordFieldType(String name, FieldType fieldType) {
602596
super(
603597
name,
@@ -608,13 +602,12 @@ public KeywordFieldType(String name, FieldType fieldType) {
608602
Collections.emptyMap()
609603
);
610604
this.normalizer = Lucene.KEYWORD_ANALYZER;
611-
this.ignoreAbove = Integer.MAX_VALUE;
605+
this.ignoreAbove = IGNORE_ABOVE_DEFAULT;
612606
this.nullValue = null;
613607
this.eagerGlobalOrdinals = false;
614608
this.scriptValues = null;
615609
this.isDimension = false;
616610
this.isSyntheticSource = false;
617-
this.indexMode = IndexMode.STANDARD;
618611
this.indexSortConfig = null;
619612
this.hasDocValuesSkipper = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false;
620613
this.originalName = null;
@@ -623,13 +616,12 @@ public KeywordFieldType(String name, FieldType fieldType) {
623616
public KeywordFieldType(String name, NamedAnalyzer analyzer) {
624617
super(name, true, false, true, textSearchInfo(Defaults.FIELD_TYPE, null, analyzer, analyzer), Collections.emptyMap());
625618
this.normalizer = Lucene.KEYWORD_ANALYZER;
626-
this.ignoreAbove = Integer.MAX_VALUE;
619+
this.ignoreAbove = IGNORE_ABOVE_DEFAULT;
627620
this.nullValue = null;
628621
this.eagerGlobalOrdinals = false;
629622
this.scriptValues = null;
630623
this.isDimension = false;
631624
this.isSyntheticSource = false;
632-
this.indexMode = IndexMode.STANDARD;
633625
this.indexSortConfig = null;
634626
this.hasDocValuesSkipper = false;
635627
this.originalName = null;
@@ -938,10 +930,7 @@ protected String parseSourceValue(Object value) {
938930
}
939931

940932
private String applyIgnoreAboveAndNormalizer(String value) {
941-
if (value.length() > ignoreAbove) {
942-
return null;
943-
}
944-
933+
if (ignoreAbove.isIgnored(value)) return null;
945934
return normalizeValue(normalizer(), name(), value);
946935
}
947936

@@ -1060,7 +1049,7 @@ public CollapseType collapseType() {
10601049

10611050
/** Values that have more chars than the return value of this method will
10621051
* be skipped at parsing time. */
1063-
public int ignoreAbove() {
1052+
public IgnoreAbove ignoreAbove() {
10641053
return ignoreAbove;
10651054
}
10661055

@@ -1078,10 +1067,6 @@ public boolean hasNormalizer() {
10781067
return normalizer != Lucene.KEYWORD_ANALYZER;
10791068
}
10801069

1081-
public IndexMode getIndexMode() {
1082-
return indexMode;
1083-
}
1084-
10851070
public IndexSortConfig getIndexSortConfig() {
10861071
return indexSortConfig;
10871072
}
@@ -1216,7 +1201,7 @@ private boolean indexValue(DocumentParserContext context, XContentString value)
12161201
return false;
12171202
}
12181203

1219-
if (value.stringLength() > fieldType().ignoreAbove()) {
1204+
if (fieldType().ignoreAbove().isIgnored(value)) {
12201205
context.addIgnoredField(fullPath());
12211206
if (isSyntheticSource) {
12221207
// Save a copy of the field so synthetic source can load it
@@ -1385,7 +1370,7 @@ protected BytesRef preserve(BytesRef value) {
13851370
}
13861371
}
13871372

1388-
if (fieldType().ignoreAbove != Integer.MAX_VALUE) {
1373+
if (fieldType().ignoreAbove.isSet()) {
13891374
layers.add(new CompositeSyntheticFieldLoader.StoredFieldLayer(originalName) {
13901375
@Override
13911376
protected void writeValue(Object value, XContentBuilder b) throws IOException {

‎server/src/main/java/org/elasticsearch/index/mapper/Mapper.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.index.IndexVersions;
2020
import org.elasticsearch.xcontent.ToXContentFragment;
2121
import org.elasticsearch.xcontent.XContentBuilder;
22+
import org.elasticsearch.xcontent.XContentString;
2223

2324
import java.io.IOException;
2425
import java.util.Arrays;
@@ -131,6 +132,78 @@ default boolean supportsVersion(IndexVersion indexCreatedVersion) {
131132
}
132133
}
133134

135+
/**
136+
* This class models the ignore_above parameter in indices.
137+
*/
138+
public static final class IgnoreAbove {
139+
140+
public static final int IGNORE_ABOVE_DEFAULT_VALUE = Integer.MAX_VALUE;
141+
public static final int IGNORE_ABOVE_DEFAULT_VALUE_FOR_LOGSDB_INDICES = 8191;
142+
143+
private final Integer value;
144+
private final Integer defaultValue;
145+
146+
public IgnoreAbove(Integer value) {
147+
this(Objects.requireNonNull(value), IndexMode.STANDARD, IndexVersion.current());
148+
}
149+
150+
public IgnoreAbove(Integer value, IndexMode indexMode) {
151+
this(value, indexMode, IndexVersion.current());
152+
}
153+
154+
public IgnoreAbove(Integer value, IndexMode indexMode, IndexVersion indexCreatedVersion) {
155+
if (value != null && value < 0) {
156+
throw new IllegalArgumentException("[ignore_above] must be positive, got [" + value + "]");
157+
}
158+
159+
this.value = value;
160+
this.defaultValue = getIgnoreAboveDefaultValue(indexMode, indexCreatedVersion);
161+
}
162+
163+
public int get() {
164+
return value != null ? value : defaultValue;
165+
}
166+
167+
/**
168+
* Returns whether ignore_above is set; at field or index level.
169+
*/
170+
public boolean isSet() {
171+
// if ignore_above equals default, its not considered to be set, even if it was explicitly set to the default value
172+
return Integer.valueOf(get()).equals(defaultValue) == false;
173+
}
174+
175+
/**
176+
* Returns whether the given string will be ignored.
177+
*/
178+
public boolean isIgnored(final String s) {
179+
if (s == null) return false;
180+
return lengthExceedsIgnoreAbove(s.length());
181+
}
182+
183+
public boolean isIgnored(final XContentString s) {
184+
if (s == null) return false;
185+
return lengthExceedsIgnoreAbove(s.stringLength());
186+
}
187+
188+
private boolean lengthExceedsIgnoreAbove(int strLength) {
189+
return strLength > get();
190+
}
191+
192+
public static int getIgnoreAboveDefaultValue(final IndexMode indexMode, final IndexVersion indexCreatedVersion) {
193+
if (diffIgnoreAboveDefaultForLogs(indexMode, indexCreatedVersion)) {
194+
return IGNORE_ABOVE_DEFAULT_VALUE_FOR_LOGSDB_INDICES;
195+
} else {
196+
return IGNORE_ABOVE_DEFAULT_VALUE;
197+
}
198+
}
199+
200+
private static boolean diffIgnoreAboveDefaultForLogs(final IndexMode indexMode, final IndexVersion indexCreatedVersion) {
201+
return indexMode == IndexMode.LOGSDB
202+
&& (indexCreatedVersion != null && indexCreatedVersion.onOrAfter(IndexVersions.ENABLE_IGNORE_ABOVE_LOGSDB));
203+
}
204+
205+
}
206+
134207
private final String leafName;
135208

136209
@SuppressWarnings("this-escape")

‎server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,15 +1021,15 @@ public boolean isAggregatable() {
10211021
* A delegate by definition must have doc_values or be stored so most of the time it can be used for loading.
10221022
*/
10231023
public boolean canUseSyntheticSourceDelegateForLoading() {
1024-
return syntheticSourceDelegate != null && syntheticSourceDelegate.ignoreAbove() == Integer.MAX_VALUE;
1024+
return syntheticSourceDelegate != null && syntheticSourceDelegate.ignoreAbove().isSet() == false;
10251025
}
10261026

10271027
/**
10281028
* Returns true if the delegate sub-field can be used for querying only (ie. isIndexed must be true)
10291029
*/
10301030
public boolean canUseSyntheticSourceDelegateForQuerying() {
10311031
return syntheticSourceDelegate != null
1032-
&& syntheticSourceDelegate.ignoreAbove() == Integer.MAX_VALUE
1032+
&& syntheticSourceDelegate.ignoreAbove().isSet() == false
10331033
&& syntheticSourceDelegate.isIndexed();
10341034
}
10351035

@@ -1045,7 +1045,7 @@ public boolean canUseSyntheticSourceDelegateForQueryingEquality(String str) {
10451045
return false;
10461046
}
10471047
// Can't push equality if the field we're checking for is so big we'd ignore it.
1048-
return str.length() <= syntheticSourceDelegate.ignoreAbove();
1048+
return syntheticSourceDelegate.ignoreAbove().isIgnored(str) == false;
10491049
}
10501050

10511051
@Override

0 commit comments

Comments
 (0)