-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Add @UpdateForV10 to AbstractObjectParser methods #130738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add @UpdateForV10 to AbstractObjectParser methods #130738
Conversation
Adds an `@UpdateForV10` annotation to the `AbstractObjectParser .declareInt` method and assigns it to the Core Infra team. This method needs to throw an `IllegalArgumentException` for non-integer values being passed, such as floats, doubles, and stringified floats and doubles.
2bfb7f6
to
5bacefc
Compare
@@ -245,6 +246,7 @@ public void declareLongOrNull(BiConsumer<Value, Long> consumer, long nullValue, | |||
); | |||
} | |||
|
|||
@UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA) // https://github.com/elastic/elasticsearch/issues/130797 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to created issue - #130797
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the same also applies to declareLong
and the ...OrNull
variants thereof?
Adds an `@UpdateForV10` annotation to the following methods in `AbstractObjectParser`: `declareInt`, `declareIntOrNull`, `declareLong`, `declareLongOrNull`, `declareIntArray` and `declareLongArray`. The required change is explained in https://github .com/elastic/issues/130797. The current parsing methods do not throw exceptions when floats or doubles are passed in place of longs or ints. The current behaviour is to truncate the float / double but this can lead to incorrect behaviour, and is also a discrepancy versus the stricter validation found when floats are passed as query parameters.
@DaveCTurner Good catch! Have expanded the PR accordingly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Adds an
@UpdateForV10
annotation to the following methods from theAbstractObjectParser
class:declareInt
declareIntOrNull
declareLong
declareLongOrNull
declareIntArray
declareLongArray
and assigns it to the Core Infra team. The required change is explained in more detail in #130797. These current parsing methods do not throw exceptions when non-integer, or non-long, values, such as floats, doubles and stringified floats and doubles, are passed. The current behaviour is to truncate the floats / doubles into ints / longs. This is problematic for two reasons:
The proposed solution will be to throw
IllegalArgumentException
's whenever non-integer and non-long values are passed.