Add support for using negative array indices in json path #11451
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
| @@ -122,6 +122,7 @@ void extractArray( | |||
| auto rv = folly::tryTo<int32_t>(key); | |||
| if (rv.hasValue()) { | |||
| auto idx = rv.value(); | |||
| idx = idx >= 0 ? idx : arrayLen + idx; | |||
There was a problem hiding this comment.
nit: arrayLen + idx wont overflow I suppose .
There was a problem hiding this comment.
It wont, arrayLen will always be non-negative and the addition is only done when idx is negative.
However, I did notice that arrayLen is size_t and idx is int32_t which can brittle when it comes to implicit conversions applied by the compiler. Therefore, I have explicitly changes both to int64_t.
bikramSingh91
force-pushed
the
jsonNegativeIndex
branch
from
626ebb6 to
e8491fd
Compare
|
@bikramSingh91 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This allows the use of negative array indices in json path to access elements from the end. Test Plan: Added unit tests
bikramSingh91
force-pushed
the
jsonNegativeIndex
branch
from
e8491fd to
e936f6e
Compare
|
@bikramSingh91 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
There is a failure in Linux Build tests due to an unrelated flaky test: Flaky AsyncDataCacheTest.ssdWriteOptions #11463 |
|
@bikramSingh91 merged this pull request in d1bf9da. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
This allows the use of negative array indices in json path to access
elements from the end.
Test Plan:
Added unit tests