[BUG] jsonNode array index access for EXTRACTJSONFIELD #610
Conversation
|
It looks like @robert2d hasn't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here. Once you've signed reply with Appreciation of efforts, clabot |
|
[clabot:check] |
|
@confluentinc It looks like @robert2d just signed our Contributor License Agreement. 👍 Always at your service, clabot |
|
related to: #523 |
| ("{\"log\":{\"@timestamp\":\"2017-05-30T16:44:22.175Z\",\"@version\":\"1\"," | ||
| + "\"caasVersion\":\"0.0.2\",\"cloud\":\"aws\",\"clusterId\":\"cp99\",\"clusterName\":\"kafka\",\"cpComponentId\":\"kafka\",\"host\":\"kafka-1-wwl0p\",\"k8sId\":\"k8s13\",\"k8sName\":\"perf\",\"level\":\"ERROR\",\"logger\":\"kafka.server.ReplicaFetcherThread\",\"message\":\"Found invalid messages during fetch for partition [foo512,172] offset 0 error Record is corrupt (stored crc = 1321230880, computed crc = 1139143803)\",\"networkId\":\"vpc-d8c7a9bf\",\"region\":\"us-west-2\",\"serverId\":\"1\",\"skuId\":\"sku5\",\"source\":\"kafka\",\"tenantId\":\"t47\",\"tenantName\":\"perf-test\",\"thread\":\"ReplicaFetcherThread-0-2\",\"zone\":\"us-west-2a\"},\"stream\":\"stdout\",\"time\":2017}")); | ||
| ("{\"log\":{\"@timestamp\":\"2017-05-30T16:44:22.175Z\",\"@version\":\"1\"," | ||
| + "\"caasVersion\":\"0.0.2\",\"cloud\":\"aws\",\"logs\":[{\"entry\":\"first\"}],\"clusterId\":\"cp99\",\"clusterName\":\"kafka\",\"cpComponentId\":\"kafka\",\"host\":\"kafka-1-wwl0p\",\"k8sId\":\"k8s13\",\"k8sName\":\"perf\",\"level\":\"ERROR\",\"logger\":\"kafka.server.ReplicaFetcherThread\",\"message\":\"Found invalid messages during fetch for partition [foo512,172] offset 0 error Record is corrupt (stored crc = 1321230880, computed crc = 1139143803)\",\"networkId\":\"vpc-d8c7a9bf\",\"region\":\"us-west-2\",\"serverId\":\"1\",\"skuId\":\"sku5\",\"source\":\"kafka\",\"tenantId\":\"t47\",\"tenantName\":\"perf-test\",\"thread\":\"ReplicaFetcherThread-0-2\",\"zone\":\"us-west-2a\"},\"stream\":\"stdout\",\"time\":2017}")); | ||
|
|
There was a problem hiding this comment.
Can you have both previous tests and this one? The UDF should work for both cases.
There was a problem hiding this comment.
No problem I thought this might come up. Reason I modified the existing is that it essentially performs both tests when accessing the nested nodes anyway. But I'll add back in the original spec unmodified and duplicate it into a new one for array access now 😄
robert2d
force-pushed
the
bug/extract-json-field-with-index
branch
2 times, most recently
from
329a0e2 to
6f9ea5b
Compare
|
@hjafarpour Updated to include original test back in. Also rebased upstream/master to ensure no changes between original work and updates have any effect. |
|
|
||
| assertThat(results, equalTo(expectedResults)); | ||
|
|
||
| ksqlEngine.terminateQuery(queryMetadata.getId(), true); |
There was a problem hiding this comment.
This should probably be done in a `try{...}finally{...}' otherwise it won't run when the test fails
| Assert.assertTrue(tokenList.get(0).equalsIgnoreCase("log")); | ||
| Assert.assertTrue(tokenList.get(1).equalsIgnoreCase("cloud")); | ||
| Assert.assertTrue(tokenList.get(2).equalsIgnoreCase("region")); | ||
| Assert.assertTrue(tokenList.size() == 4); |
There was a problem hiding this comment.
I know these were previously assertTrue but it would be good to change this to use assertThat(..) instead
|
@robert2d are you able to update this PR with the feedback? |
|
Hi I have addressed the issues you mentioned @dguy and will submit a seperate pull request for it in the interest of code quality 😄 |
Allows access to retrieval of JSON node when in array or complex nested objects.
Previously the above query returned null because jsonNode would only access an index of an array node when using an
intwhich was not being catered for in the current release.Two tests have been updated to check this change both "tokenises" correctly using an array accessor and that it integrates into the jsonFormatTest when used in an actual query.