Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -295,7 +295,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 @@ -323,7 +324,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 @@ -1407,7 +1407,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
11 changes: 10 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ default boolean supportsVersion(IndexVersion indexCreatedVersion) {
* This class models the ignore_above parameter in indices.
*/
public static final class IgnoreAbove {

// We use Integer.MAX_VALUE to represent a no-op, accepting all values.
public static final int IGNORE_ABOVE_DEFAULT_VALUE = Integer.MAX_VALUE;
public static final int IGNORE_ABOVE_DEFAULT_VALUE_FOR_LOGSDB_INDICES = 8191;

Expand Down Expand Up @@ -172,6 +172,15 @@ 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() {
// We use Integer.MAX_VALUE to represent accepting all values. If the value is anything else, then either we have an
// explicitly configured ignore_above, or we have a non no-op default.
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