Forbid negative weight in Function Score Query#33390
Merged
jimczi merged 5 commits intoelastic:masterfrom Sep 12, 2018
Merged
Conversation
Collaborator
|
Pinging @elastic/es-search-aggs |
jimczi
requested changes
Sep 7, 2018
| } | ||
|
|
||
| private Float checkWeight(Float weight) { | ||
| if (weight != null && weight < 0) { |
Contributor
There was a problem hiding this comment.
Can you do Float.compare(weight, 0) < 0)
jimczi
requested changes
Sep 7, 2018
|
|
||
| private Float checkWeight(Float weight) { | ||
| if (weight != null && weight < 0) { | ||
| DEPRECATION_LOGGER.deprecated("[weight] cannot be negative for a filtering function [{}]", weight); |
Contributor
There was a problem hiding this comment.
Since this is master can you throw an IllegalArgumentException and we'll handle the deprecation when we backport to 6x.
Contributor
Author
There was a problem hiding this comment.
Hm... I thought that it is preferred to have one PR against master handing the deprecation (that is backported to 6.x) and a second (followup) PR for the removal (and IAE) that is only on master.
My bad;) I will change the PR shortly.
Contributor
There was a problem hiding this comment.
For this one I think we can have two prs, one for the removal and one for the deprecation that targets 6x only. There are some [tests]6269744#diff-9af0a85c71293ca3d30f83fe80aedd28 in master that will fail in 6x because they produce negative weights so the backport is not trivial.
weight is negative in a Function Score Queryweight in Function Score Query
jimczi
approved these changes
Sep 11, 2018
Contributor
|
test this please |
…o neg_weight_depr
Contributor
Author
Contributor
|
test this please |
Contributor
|
Thanks @lipsill ! As discussed earlier we need a new pr for the backport in 6x where some tests need to be adapted. |
Contributor
Author
jasontedor
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
Sep 12, 2018
* master: (43 commits) [HLRC][ML] Add ML put datafeed API to HLRC (elastic#33603) Update AWS SDK to 1.11.406 in repository-s3 (elastic#30723) Expose CCR stats to monitoring (elastic#33617) [Docs] Update match-query.asciidoc (elastic#33610) TEST: Adjust rollback condition when shard is empty [CCR] Improve shard follow task's retryable error handling (elastic#33371) Forbid negative `weight` in Function Score Query (elastic#33390) Clarify context suggestions filtering and boosting (elastic#33601) Disable CCR REST endpoints if CCR disabled (elastic#33619) Lower version on full cluster restart settings test Upgrade remote cluster settings (elastic#33537) NETWORKING: http.publish_host Should Contain CNAME (elastic#32806) Add test coverage for global checkpoint listeners Reset replica engine to global checkpoint on promotion (elastic#33473) HLRC: ML Delete Forecast API (elastic#33526) Remove debug logging in full cluster restart tests (elastic#33612) Expose CCR to the transport client (elastic#33608) Mute testIndexDeletionWhenNodeRejoins SQL: Make Literal a NamedExpression (elastic#33583) [DOCS] Adds missing built-in user information (elastic#33585) ...
jasontedor
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
Sep 12, 2018
* master: (128 commits) [HLRC][ML] Add ML put datafeed API to HLRC (elastic#33603) Update AWS SDK to 1.11.406 in repository-s3 (elastic#30723) Expose CCR stats to monitoring (elastic#33617) [Docs] Update match-query.asciidoc (elastic#33610) TEST: Adjust rollback condition when shard is empty [CCR] Improve shard follow task's retryable error handling (elastic#33371) Forbid negative `weight` in Function Score Query (elastic#33390) Clarify context suggestions filtering and boosting (elastic#33601) Disable CCR REST endpoints if CCR disabled (elastic#33619) Lower version on full cluster restart settings test Upgrade remote cluster settings (elastic#33537) NETWORKING: http.publish_host Should Contain CNAME (elastic#32806) Add test coverage for global checkpoint listeners Reset replica engine to global checkpoint on promotion (elastic#33473) HLRC: ML Delete Forecast API (elastic#33526) Remove debug logging in full cluster restart tests (elastic#33612) Expose CCR to the transport client (elastic#33608) Mute testIndexDeletionWhenNodeRejoins SQL: Make Literal a NamedExpression (elastic#33583) [DOCS] Adds missing built-in user information (elastic#33585) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #31927