Skip to content

Conversation

gmarouli
Copy link
Contributor

@gmarouli gmarouli commented Sep 25, 2025

This PR is an alternative of #134996:

This fixes a bug that would be triggered when we would downsample an index that had a metric field under a passthrough field. For example:

"metrics": {
   "type": "passthrough",
   "priority": 10,
   "properties": {
      "cpu_usage": {
         "type": "double",
         "time_series_metric": "gauge"
      }
    }
}

In this PR, we extend the merging possibilities of a passthrough field to allow merging with object mappers that have subobjects false and they are not a root object. They also fix merging an object mapper with a passthrough field and ensure that the result will either be a passthrough field or an error if the objects are incompatible.

@gmarouli gmarouli force-pushed the fix-downsampling-passthrough-fields-v2 branch from 4e4c829 to c7656ea Compare September 25, 2025 11:01
@gmarouli gmarouli added >bug :StorageEngine/Mapping The storage related side of mappings labels Sep 29, 2025
@gmarouli gmarouli marked this pull request as ready for review September 29, 2025 09:59
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @gmarouli, I've created a changelog YAML for you.

});
safeAwait(listener);

assertDownsampleIndexFieldsAndDimensions(sourceIndex, targetIndex, downsampleConfig);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider deduplicating the shared parts.

@kkrik-es kkrik-es requested a review from romseygeek September 29, 2025 13:20
@kkrik-es
Copy link
Contributor

Maybe backport to 9.1 too?

@gmarouli gmarouli added auto-backport Automatically create backport pull requests when merged v9.1.5 labels Sep 30, 2025
@gmarouli gmarouli added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Sep 30, 2025
@elasticsearchmachine elasticsearchmachine merged commit c71588b into elastic:main Sep 30, 2025
34 checks passed
@gmarouli gmarouli deleted the fix-downsampling-passthrough-fields-v2 branch September 30, 2025 07:56
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.1

gmarouli added a commit to gmarouli/elasticsearch that referenced this pull request Sep 30, 2025
…pers under certain conditions. (elastic#135431)

This PR is an alternative of
elastic#134996:

This fixes a bug that would be triggered when we would downsample an
index that had a metric field under a passthrough field. For example:

```
"metrics": {
   "type": "passthrough",
   "priority": 10,
   "properties": {
      "cpu_usage": {
         "type": "double",
         "time_series_metric": "gauge"
      }
    }
}
```

In this PR, we extend the merging possibilities of a passthrough field
to allow merging with object mappers that have subobjects false and they
are not a root object. They also fix merging an object mapper with a
passthrough field and ensure that the result will either be a
passthrough field or an error if the objects are incompatible.
@gmarouli
Copy link
Contributor Author

gmarouli commented Sep 30, 2025

Maybe backport to 9.1 too?

We opened a backport PR but it looks like we need extra changes to allow an object to merge with a passthrough object. Considering this, we advice to not backport this to 9.1. This mean a user would need to upgrade a minor.

@felixbarny & @AlexanderWert any objections?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :StorageEngine/Mapping The storage related side of mappings Team:StorageEngine v9.1.5 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants