Fix creating filtered alias using now in a date_nanos range query failed #54785
Conversation
There was a problem hiding this comment.
Hi @gaobinlong, thanks for submitting this fix. The change itself makes sense to me, but could you also add a test that checks that the validation with this filter string works now on date_nanos?
Ideally this could maybe be in MetadataIndexAliasesServiceTests, but I quickly looked at it and this might require quite a bit of mocking on the test setup side. Alternatively you could add this particular check in the rest tests (yml tests) in some existing tests under rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_alias
cbuescher
added
:Data Management/Indices APIs
|
Pinging @elastic/es-core-features (:Core/Features/Indices APIs) |
|
@cbuescher, I have added a yaml test for this change, can you take a look on that? |
There was a problem hiding this comment.
LGTM, thanks for adding the test. I left a note where I think we will need to adapt the skip-versions in the yaml test before backporting this to 7.x but I can also do that while doing the backport. Let me know if you want to push an update or if I should take over the PR for the test-related backporting details.
| --- | ||
| "Can create filtered alias with a date_nanos range query": | ||
| - skip: | ||
| version: " - 6.8.99" |
There was a problem hiding this comment.
I think this needs changing to " - 7.99.99" initially since the bugfix will land on the master branch first. We also run this tests in "mixed" cluster scenarios to simulate behaviours during a rolling upgrade, so this might hit a 7.latest/8.0 cluster and would then could fail if hitting a 7.x node without the fix first.
The way we usually handle this is disabeling the tests except for the version this is first checked in, then backport and adapt the versions accordingly. I can handle that if you want, probably faster than going through extra PRs...
|
@elasticmachine ok to test |
|
@gaobinlong as mentioned, the failing tests are due to running the new yml test in a mixed cluster. I can update the skip versions if you like and do the backport, just let me know what you prefer. |
|
@cbuescher, you can help to change the skip versions and do the backport as you are clear about that, thanks! |
|
@elasticmachine update branch |
…led (elastic#54785) Modify the value of nowInMillis in queryShardContext to current timestamp, because the value will be used lately when validating the filtered alias which uses now in a date_nanos range query. Closes elastic#54315
|
@gaobinlong thanks for working on this fix, I also backported it to the 7.8 branch. |
Closes #54315.
Modify the value of nowInMillis in queryShardContext to current timestamp, because the value will be used lately when validating the filtered alias which uses
nowin a date_nanos range query.