ESQL: Fix AttributeSet#add() returning the opposite expected value#117367
ESQL: Fix AttributeSet#add() returning the opposite expected value#117367ivancea merged 3 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
There was a problem hiding this comment.
I agree with this change.
I manually went over the declarations of AttributeSet to check for usages of the add method's return type. (Local var declarations, field declarations, method parameter declarations)
I only found one: AttributeSet.add's return type is used in CombineProjections. (There may be more if I overlooked them, as with every manual check.)
This means there is probably a bug there which we should investigate before merging this.
|
@alex-spies In |
alex-spies
left a comment
There was a problem hiding this comment.
Argh, sorry - I was looking at your branch where you started using the return value. Of course, this change should be fine then - phew!
|
Thanks for yanking it into it's own PR! |
costin
left a comment
There was a problem hiding this comment.
Oopps. LGTM
P.S. Thanks for extracting it into its own PR!
|
Also updated the QL counterpart. I didn't find any usage of the add(), at all |
💚 Backport successful
|
…lastic#117367) Set/Collection#add() is supposed to return `true` if the collection changed (If it actually added something). In this case, it must return if the old value is null. Extracted from elastic#114317 (Where it's being used)
…lastic#117367) Set/Collection#add() is supposed to return `true` if the collection changed (If it actually added something). In this case, it must return if the old value is null. Extracted from elastic#114317 (Where it's being used)
Set/Collection#add() is supposed to return
trueif the collection changed (If it actually added something).In this case, it must return if the old value is null.
Extracted from #114317 (Where it's being used)