Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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/137394.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 137394
summary: Fix dropped ignore above fields
area: Mapping
type: bug
issues:
- 137360
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
String parentFieldName = searchExecutionContext.parentPath(name());
var parent = searchExecutionContext.lookup().fieldType(parentFieldName);

if (parent instanceof KeywordFieldMapper.KeywordFieldType keywordParent && keywordParent.ignoreAbove().isSet()) {
if (parent instanceof KeywordFieldMapper.KeywordFieldType keywordParent
&& keywordParent.ignoreAbove().valuesPotentiallyIgnored()) {
final String parentFallbackFieldName = keywordParent.syntheticSourceFallbackFieldName();
if (parent.isStored()) {
return storedFieldFetcher(parentFieldName, parentFallbackFieldName);
Expand Down Expand Up @@ -325,7 +326,7 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
final SearchExecutionContext searchExecutionContext,
final KeywordFieldMapper.KeywordFieldType keywordDelegate
) {
if (keywordDelegate.ignoreAbove().isSet()) {
if (keywordDelegate.ignoreAbove().valuesPotentiallyIgnored()) {
// because we don't know whether the delegate field will be ignored during parsing, we must also check the current field
String fieldName = name();
String fallbackName = syntheticSourceFallbackFieldName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1390,7 +1390,7 @@ protected BytesRef preserve(BytesRef value) {

// if ignore_above is set, then there is a chance that this field will be ignored. In such cases, we save an
// extra copy of the field for supporting synthetic source. This layer will check that copy.
if (fieldType().ignoreAbove.isSet()) {
if (fieldType().ignoreAbove.valuesPotentiallyIgnored()) {
final String fieldName = fieldType().syntheticSourceFallbackFieldName();
layers.add(new CompositeSyntheticFieldLoader.StoredFieldLayer(fieldName) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ public boolean isSet() {
return Integer.valueOf(get()).equals(defaultValue) == false;
}

/**
* Returns whether values are potentially ignored, either by an explicitly configured ignore_above or by the default value.
*/
public boolean valuesPotentiallyIgnored() {
return get() != Integer.MAX_VALUE;
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] can you leave a comment explaining why Integer.MAX_VALUE is used here? Otherwise its use clashes with the default value for ignore_above for non-logsdb indices, which is confusing.

}

/**
* Returns whether the given string will be ignored.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1026,15 +1026,15 @@ public boolean isAggregatable() {
* 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() {
return syntheticSourceDelegate.isPresent() && syntheticSourceDelegate.get().ignoreAbove().isSet() == false;
return syntheticSourceDelegate.isPresent() && syntheticSourceDelegate.get().ignoreAbove().valuesPotentiallyIgnored() == false;
}

/**
* Returns true if the delegate sub-field can be used for querying only (ie. isSearchable must be true)
*/
public boolean canUseSyntheticSourceDelegateForQuerying() {
return syntheticSourceDelegate.isPresent()
&& syntheticSourceDelegate.get().ignoreAbove().isSet() == false
&& syntheticSourceDelegate.get().ignoreAbove().valuesPotentiallyIgnored() == false
&& syntheticSourceDelegate.get().isSearchable();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ protected SyntheticSourceSupport syntheticSourceSupport() {
() -> new FlattenedSortedSetDocValuesSyntheticFieldLoader(
fullPath(),
fullPath() + KEYED_FIELD_SUFFIX,
fieldType().ignoreAbove.isSet() ? fullPath() + KEYED_IGNORED_VALUES_FIELD_SUFFIX : null,
fieldType().ignoreAbove.valuesPotentiallyIgnored() ? fullPath() + KEYED_IGNORED_VALUES_FIELD_SUFFIX : null,
leafName()
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public void test_ignore_above_with_value_and_index_mode_and_index_version() {
// when/then
assertEquals(123, ignoreAbove.get());
assertTrue(ignoreAbove.isSet());
assertTrue(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_ignore_above_with_value_only() {
Expand All @@ -34,6 +35,7 @@ public void test_ignore_above_with_value_only() {
// when/then
assertEquals(123, ignoreAbove.get());
assertTrue(ignoreAbove.isSet());
assertTrue(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_ignore_above_with_null_value_should_throw() {
Expand All @@ -52,6 +54,7 @@ public void test_ignore_above_with_null_value() {
// when/then
assertEquals(Mapper.IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE, ignoreAbove.get());
assertFalse(ignoreAbove.isSet());
assertFalse(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_ignore_above_with_null_value_and_logsdb_index_mode() {
Expand All @@ -61,6 +64,7 @@ public void test_ignore_above_with_null_value_and_logsdb_index_mode() {
// when/then
assertEquals(Mapper.IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE_FOR_LOGSDB_INDICES, ignoreAbove.get());
assertFalse(ignoreAbove.isSet());
assertTrue(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_ignore_above_with_null_everything() {
Expand All @@ -70,6 +74,7 @@ public void test_ignore_above_with_null_everything() {
// when/then
assertEquals(Mapper.IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE, ignoreAbove.get());
assertFalse(ignoreAbove.isSet());
assertFalse(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_ignore_above_default_for_standard_indices() {
Expand All @@ -79,6 +84,7 @@ public void test_ignore_above_default_for_standard_indices() {
// when/then
assertEquals(Mapper.IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE, ignoreAbove.get());
assertFalse(ignoreAbove.isSet());
assertFalse(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_ignore_above_default_for_logsdb_indices() {
Expand All @@ -88,6 +94,7 @@ public void test_ignore_above_default_for_logsdb_indices() {
// when/then
assertEquals(Mapper.IgnoreAbove.IGNORE_ABOVE_DEFAULT_VALUE_FOR_LOGSDB_INDICES, ignoreAbove.get());
assertFalse(ignoreAbove.isSet());
assertTrue(ignoreAbove.valuesPotentiallyIgnored());
}

public void test_string_isIgnored() {
Expand Down
Loading