zeroshade commented on code in PR #510:
URL: https://github.com/apache/arrow-go/pull/510#discussion_r2356428560
##########
arrow/array/timestamp.go:
##########
@@ -34,11 +34,18 @@ import (
type Timestamp struct {
array
values []arrow.Timestamp
+ layout string
}
// NewTimestampData creates a new Timestamp from Data.
func NewTimestampData(data arrow.ArrayData) *Timestamp {
- a := &Timestamp{}
+ return NewTimestampDataWithLayout(data, time.RFC3339Nano)
+}
+
+// NewTimestampDataWithLayout creates a new Timestamp from Data with a custom
string value layout.
+// This is useful for cases where consumers expect a non standard layout
+func NewTimestampDataWithLayout(data arrow.ArrayData, layout string)
*Timestamp {
Review Comment:
I'd rather slightly different naming to indicate we're talking about the
layout *when converting to a string*
Also, we should indicate in the comment that the layout refers to the layout
from `[time.Time.Format]`
##########
arrow/array/timestamp.go:
##########
@@ -130,10 +141,15 @@ type TimestampBuilder struct {
dtype *arrow.TimestampType
data *memory.Buffer
rawData []arrow.Timestamp
+ layout string
}
func NewTimestampBuilder(mem memory.Allocator, dtype *arrow.TimestampType)
*TimestampBuilder {
- tb := &TimestampBuilder{builder: builder{mem: mem}, dtype: dtype}
+ return NewTimestampBuilderWithLayout(mem, dtype, time.RFC3339Nano)
+}
+
+func NewTimestampBuilderWithLayout(mem memory.Allocator, dtype
*arrow.TimestampType, layout string) *TimestampBuilder {
Review Comment:
same as above, not a big fan of this naming and we should indicate in a
comment that the layout is referring to the `[time.Time.Format]` layout
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
