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
Fix unable to filter model correctly in case of filter options contain omitted field #3488
Conversation
Code coverage for golang is
|
/hold |
/hold cancel |
Code coverage for golang is
|
Code coverage for golang is
|
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.
pkg/datastore/filedb/filter.go
Outdated
var valNum, operandNum int64 | ||
var ( | ||
valNum, operandNum float64 | ||
valCasted bool = true |
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.
should omit type bool from declaration of var valCasted; it will be inferred from the right-hand side
pkg/datastore/filedb/filter.go
Outdated
var ( | ||
valNum, operandNum float64 | ||
valCasted bool = true | ||
operandCasted bool = true |
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.
should omit type bool from declaration of var operandCasted; it will be inferred from the right-hand side
Code coverage for golang is
|
Code coverage for golang is
|
Co-authored-by: knanao <50069775+knanao@users.noreply.github.com>
Code coverage for golang is
|
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.
Looks neat.
/lgtm
Got it. 👍 /approve |
What this PR does / why we need it:
The root cause of this issue is that we have to convert model objects to
map[string]interface{}
in order to make filter logic works. The previous converting logic does: marshal object first, then unmarshal raw byte tomap[string]interface{}
.Since our objects which are generated by protobuf are always contain
omitempty
as its json tag. While doing "first marshal", all fields with zero value will be omitted and unable to filter the object correctly if the filter options contain those omitted fields.Through this PR, I change to using protojson marshaler instead of encoding/json marshal, which has an option to enable us to keep zero value for fields with
omitempty
jsontag.Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: