feat: Add support for specifying Timestamp ValueStr output layout#510
Conversation
|
|
||
| func (t TimestampMicros) MarshalJSON() ([]byte, error) { | ||
| ts := time.Unix(0, int64(t)*int64(time.Microsecond)).UTC().Format(time.RFC3339Nano) | ||
| // arrow record marshaller trims trailing zero digits from timestamp so we do the same |
There was a problem hiding this comment.
This comment seems no longer relevant
| return NewTimestampDataWithFormat(data, time.RFC3339Nano) | ||
| } | ||
|
|
||
| func NewTimestampDataWithFormat(data arrow.ArrayData, format string) *Timestamp { |
There was a problem hiding this comment.
| func NewTimestampDataWithFormat(data arrow.ArrayData, format string) *Timestamp { | |
| func NewTimestampDataWithLayout(data arrow.ArrayData, layout string) *Timestamp { |
reflecting the terminology used in stdlib/time (s/format/layout/g throughout)
Also probably want godoc for this.
Timestamp valueStr output formatTimestamp valueStr output layout
Timestamp valueStr output layoutTimestamp ValueStr output layout
zeroshade
left a comment
There was a problem hiding this comment.
Looks good to me, just one nitpick: can we add a JSON test which verifies the JSON marshalling output respects the layout
Added a test in 36fd689, and discovered I had a redundant method |
|
Thank you @zeroshade and @efd6 for the swift review process, appreciate it |
|
Hi @zeroshade any chance of doing a release with this feature soon? |
#2367) Instead of #2306 that also updates the Arrow version to v18.5.0 with apache/arrow-go#510. This PR has lint fixes and test fixes using apache/arrow-go#510 ---
Rationale for this change
See discussion in #450
What changes are included in this PR?
Allow setting the formatting of timestamps when using
ValueStr(or JSON marshalling)Are these changes tested?
Yes - re-added the original test that was modified in #450
Are there any user-facing changes?
Yes - users will now be able to revert back to the old format before #450, to avoid unintentional breakage