opt: fix duplicate empty JSON value spans for <@ expressions #63355
Conversation
|
This change is |
There was a problem hiding this comment.
Also, I'm not sure if isObjectValue is the best name for the boolean being passed in the functions, it's more like isNonEmptyObjectOrArrayValue (!isEnd), not sure if there's a more fitting yet simple name?
Reviewable status:
complete! 0 of 0 LGTMs obtained
There was a problem hiding this comment.
Good find!
How about keeping the name isObjectValue, but making it actually mean that (i.e., you would pass true from the function calls inside jsonObject.encodeContainedInvertedIndexSpans())? Then you'd need to check if !isObjectValue || len(j) == 0. I think that should work, right?
Reviewed 3 of 3 files at r1.
Reviewable status:
angelazxu
force-pushed
the
fix_empty_spans1
branch
from
fd9659e to
70e58a0
Compare
There was a problem hiding this comment.
yup that works!! thanks :D
Reviewable status:
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status:
pkg/util/json/json.go, line 108 at r2 (raw file):
// // If isRoot is true, this function is being called at the root level of the // JSON hierarchy.
add a description of the isObjectValue param here
Previously, there were some duplicate spans produced for empty JSON values,
such as {"a": []} and {"a": {}}. This was discovered in the spans produced
from contained by and fetch/contained by expressions.
This commit fixes this issue, only allowing one empty object or array span to
be created per level of nesting, in encodedContainedInvertedIndexSpans.
Informs: cockroachdb#61430
Release note: None
angelazxu
force-pushed
the
fix_empty_spans1
branch
from
70e58a0 to
a72d1bb
Compare
|
bors r+ |
|
Build succeeded: |
|
this might benefit from a backport to the stable release branch(es). |
|
I think fixes a bug that was introduced after the branch cut for 21.1, so I don't think it needs a backport. |
Previously, there were some duplicate spans produced for empty JSON values,
such as {"a": []} and {"a": {}}. This was discovered in the spans produced
from contained by and fetch/contained by expressions.
This commit fixes this issue, only allowing one empty object or array span to
be created per level of nesting, in encodedContainedInvertedIndexSpans.
Informs: #61430
Release note: None