Skip to content

Conversation

salvatore-campagna
Copy link
Contributor

@salvatore-campagna salvatore-campagna commented Sep 18, 2024

Here we introduce a new index-level setting, ignore_above, similar to what we have
for ignore_malformed. The setting will apply to all keyword, wildcard and flattened
fields. Each field mapping will still be allowed to override the index-level setting using a
mapping-level ignore_above value.

Resolves #112795

@salvatore-campagna salvatore-campagna requested a review from a team as a code owner September 18, 2024 14:26
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.0.0 labels Sep 18, 2024
@salvatore-campagna salvatore-campagna changed the title Feature/ignore above index setting Introduce an ignore_above index-level setting Sep 18, 2024
@salvatore-campagna salvatore-campagna added >non-issue :StorageEngine/Logs You know, for Logs v8.16.0 and removed needs:triage Requires assignment of a team area label v9.0.0 labels Sep 18, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

return (KeywordFieldMapper) in;
}

public static Builder buildForTest(final String name, boolean hasDocValues) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is needed and is unrelated to adding the new index setting? We can keep using the constructor with two parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this after changing the available constructors.

match_all: {}

- length: { hits.hits: 1 }
#TODO: synthetic source field reconstruction bug (TBD: add link to the issue here)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not reproduce the issue using only a keyword field...it looks like the issue happens when a keyword is used together with a flattened field.

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Left two comments, otherwise LGTM.

public abstract class Mapper implements ToXContentFragment, Iterable<Mapper> {

public static final NodeFeature SYNTHETIC_SOURCE_KEEP_FEATURE = new NodeFeature("mapper.synthetic_source_keep");
public static final NodeFeature IGNORE_ABOVE_INDEX_LEVEL_SETTING = new NodeFeature("mapper.ignore_above_index_level_setting");
Copy link
Member

Choose a reason for hiding this comment

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

maybe move this constant also to IndexSettings?

*/
protected abstract boolean modifySearch(ApiCallSection search);

private static Object getSetting(final Object map, final String... keys) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Contributor Author

@salvatore-campagna salvatore-campagna Sep 23, 2024

Choose a reason for hiding this comment

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

This change is necessary to skip testing with runtime fields when ignore_above is applied. Runtime fields don't support ignore_above, and the behavior is already in place at the mapping level (see code below). This ensures that tests don't incorrectly use runtime fields in scenarios where ignore_above is used (we skip them).
Below, in method runtimeifyMappingProperties, we do the following indeed, if ignore_above is used at field mapping level:

if (propertyMap.containsKey("ignore_above")) {
                    // Scripts don't support ignore_above so we skip those fields
                    continue;
                }

@salvatore-campagna salvatore-campagna force-pushed the feature/ignore-above-index-setting branch from b2b4015 to 086cc44 Compare September 23, 2024 10:59
@salvatore-campagna salvatore-campagna merged commit 208a1fe into elastic:main Sep 23, 2024
15 checks passed
@salvatore-campagna
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

salvatore-campagna added a commit to salvatore-campagna/elasticsearch that referenced this pull request Sep 23, 2024
Here we introduce a new index-level setting, `ignore_above`, similar to what we have
for `ignore_malformed`. The setting will apply to all `keyword`, `wildcard` and `flattened`
fields. Each field mapping will still be allowed to override the index-level setting using a
mapping-level `ignore_above` value.

(cherry picked from commit 208a1fe)
elasticsearchmachine pushed a commit that referenced this pull request Sep 23, 2024
Here we introduce a new index-level setting, `ignore_above`, similar to what we have
for `ignore_malformed`. The setting will apply to all `keyword`, `wildcard` and `flattened`
fields. Each field mapping will still be allowed to override the index-level setting using a
mapping-level `ignore_above` value.

(cherry picked from commit 208a1fe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce ignore_above index level setting

5 participants