fix: improve inferred json encoding for csv output #364
Conversation
Codecov Report
@@ Coverage Diff @@
## master #364 +/- ##
========================================
+ Coverage 42.7% 43.0% +0.2%
========================================
Files 25 25
Lines 1907 1917 +10
========================================
+ Hits 816 826 +10
Misses 964 964
Partials 127 127 |
|
|
||
| // Strings marked as json type are assumed to already be encoded | ||
| if fk != reflect.String && (t.types[i] == "json" || t.types[i] == "jsonb") { | ||
| encodeAsJSON = true |
There was a problem hiding this comment.
Should this be false if its already encoded?
There was a problem hiding this comment.
No, this is checking if it has a json datatype but is not a string
There was a problem hiding this comment.
Comment should be clearer
| } else if fk == reflect.Interface { | ||
| encodeAsJSON = true | ||
| } |
There was a problem hiding this comment.
Does this mean slices of strings/bytes/structs will be encoded as json?
There was a problem hiding this comment.
Slices of strings encode as json. For example MultisigTransaction.Approved is a []string with no column type tag. go-pg assigns that a json type by default, which matches our schema. Same with MinerInfo.ControlAddresses and MinerInfo.MultiAddresses.
Nil JSON values should be encoded as lower-case
nullin the CSV output, but other nil values should remain uppercaseSlices and other non-scalar types are automatically inferred to be stored as JSON by go-pg so we can use that to automatically encode. However
stringtypes that have thetype:jsonbannotation in our models refer to values that have already been encoded as JSON, so the CSV output needs to avoid encoding them again.