zeroshade commented on code in PR #39019:
URL: https://github.com/apache/arrow/pull/39019#discussion_r1427382070
##########
go/arrow/cdata/cdata.go:
##########
@@ -654,6 +680,40 @@ func (imp *cimporter) importStringLike(offsetByteWidth
int64) (err error) {
return
}
+func (imp *cimporter) importBinaryViewLike() (err error) {
+ if err = imp.checkNoChildren(); err != nil {
+ return
+ }
+
+ buffers := make([]*memory.Buffer, len(imp.cbuffers)-1)
+ // XXX couldn't figure out how to extract
file_reader.go::releaseBuffers as a utility
+ defer func() {
+ for _, buf := range buffers {
+ if buf != nil {
+ buf.Release()
+ }
+ }
+ }()
Review Comment:
we could export it (just make the first letter capitalized) but if we do
that, it might make more sense for it to exist inside of `memory` as a utility
rather than from the `ipc` package
##########
go/arrow/type_traits.go:
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package arrow
+
+import (
+ "reflect"
+ "unsafe"
+
+ "github.com/apache/arrow/go/v15/arrow/decimal128"
+ "github.com/apache/arrow/go/v15/arrow/decimal256"
+ "github.com/apache/arrow/go/v15/arrow/float16"
+ "golang.org/x/exp/constraints"
+)
+
+// IntType is a type constraint for raw values represented as signed
+// integer types by We aren't just using constraints.Signed
+// because we don't want to include the raw `int` type here whose size
+// changes based on the architecture (int32 on 32-bit architectures and
+// int64 on 64-bit architectures).
+//
+// This will also cover types like MonthInterval or the time types
+// as their underlying types are int32 and int64 which will get covered
+// by using the ~
+type IntType interface {
+ ~int8 | ~int16 | ~int32 | ~int64
+}
+
+// UintType is a type constraint for raw values represented as unsigned
+// integer types by We aren't just using constraints.Unsigned
+// because we don't want to include the raw `uint` type here whose size
+// changes based on the architecture (uint32 on 32-bit architectures and
+// uint64 on 64-bit architectures). We also don't want to include uintptr
+type UintType interface {
+ ~uint8 | ~uint16 | ~uint32 | ~uint64
+}
+
+// FloatType is a type constraint for raw values for representing
+// floating point values in This consists of constraints.Float and
+// float16.Num
+type FloatType interface {
+ float16.Num | constraints.Float
+}
+
+// NumericType is a type constraint for just signed/unsigned integers
+// and float32/float64.
+type NumericType interface {
+ IntType | UintType | constraints.Float
+}
+
+// FixedWidthType is a type constraint for raw values in Arrow that
+// can be represented as FixedWidth byte slices. Specifically this is for
+// using Go generics to easily re-type a byte slice to a properly-typed
+// slice. Booleans are excluded here since they are represented by Arrow
+// as a bitmap and thus the buffer can't be just reinterpreted as a []bool
+type FixedWidthType interface {
+ IntType | UintType |
+ FloatType | decimal128.Num | decimal256.Num |
+ DayTimeInterval | MonthDayNanoInterval
+}
+
+type TemporalType interface {
+ Date32 | Date64 | Time32 | Time64 |
+ Timestamp | Duration | DayTimeInterval |
+ MonthInterval | MonthDayNanoInterval
+}
+
+func sliceAs[Out, T interface{}](b []T) []Out {
Review Comment:
just to be cleaner, use `any` instead of `interface{}`. I prefer it
aesthetically
##########
go/arrow/internal/arrjson/arrjson.go:
##########
@@ -1090,7 +1091,7 @@ func arrayFromJSON(mem memory.Allocator, dt
arrow.DataType, arr Array) arrow.Arr
case arrow.BinaryViewDataType:
valids := validsToBitmap(validsFromJSON(arr.Valids), mem)
nulls := arr.Count - bitutil.CountSetBits(valids.Bytes(), 0,
arr.Count)
- headers := stringHeadersFromJSON(mem, !dt.IsUtf8(), arr.Data)
+ headers := stringHeadersFromJSON(mem, !dt.IsUtf8(), arr.Views)
Review Comment:
was this a change in the integration testing spec?
##########
go/arrow/type_traits.go:
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package arrow
+
+import (
+ "reflect"
+ "unsafe"
+
+ "github.com/apache/arrow/go/v15/arrow/decimal128"
+ "github.com/apache/arrow/go/v15/arrow/decimal256"
+ "github.com/apache/arrow/go/v15/arrow/float16"
+ "golang.org/x/exp/constraints"
+)
+
+// IntType is a type constraint for raw values represented as signed
+// integer types by We aren't just using constraints.Signed
+// because we don't want to include the raw `int` type here whose size
+// changes based on the architecture (int32 on 32-bit architectures and
+// int64 on 64-bit architectures).
+//
+// This will also cover types like MonthInterval or the time types
+// as their underlying types are int32 and int64 which will get covered
+// by using the ~
+type IntType interface {
+ ~int8 | ~int16 | ~int32 | ~int64
+}
+
+// UintType is a type constraint for raw values represented as unsigned
+// integer types by We aren't just using constraints.Unsigned
+// because we don't want to include the raw `uint` type here whose size
+// changes based on the architecture (uint32 on 32-bit architectures and
+// uint64 on 64-bit architectures). We also don't want to include uintptr
+type UintType interface {
+ ~uint8 | ~uint16 | ~uint32 | ~uint64
+}
+
+// FloatType is a type constraint for raw values for representing
+// floating point values in This consists of constraints.Float and
+// float16.Num
+type FloatType interface {
+ float16.Num | constraints.Float
+}
+
+// NumericType is a type constraint for just signed/unsigned integers
+// and float32/float64.
+type NumericType interface {
+ IntType | UintType | constraints.Float
+}
+
+// FixedWidthType is a type constraint for raw values in Arrow that
+// can be represented as FixedWidth byte slices. Specifically this is for
+// using Go generics to easily re-type a byte slice to a properly-typed
+// slice. Booleans are excluded here since they are represented by Arrow
+// as a bitmap and thus the buffer can't be just reinterpreted as a []bool
+type FixedWidthType interface {
+ IntType | UintType |
+ FloatType | decimal128.Num | decimal256.Num |
+ DayTimeInterval | MonthDayNanoInterval
+}
+
+type TemporalType interface {
+ Date32 | Date64 | Time32 | Time64 |
+ Timestamp | Duration | DayTimeInterval |
+ MonthInterval | MonthDayNanoInterval
+}
+
+func sliceAs[Out, T interface{}](b []T) []Out {
+ len_bytes := len(b) * int(unsafe.Sizeof(b[0]))
+ cap_bytes := cap(b) * int(unsafe.Sizeof(b[0]))
+
+ var z Out
+ len_out := len_bytes / int(unsafe.Sizeof(z))
+ cap_out := cap_bytes / int(unsafe.Sizeof(z))
Review Comment:
go typically prefers pascalCase rather than snake_case
##########
go/arrow/internal/utils.go:
##########
@@ -45,3 +45,17 @@ func HasValidityBitmap(id arrow.Type, version
flatbuf.MetadataVersion) bool {
}
return true
}
+
+// HasBufferSizesBuffer returns whether a given type has an extra buffer
+// in the C ABI to store the sizes of other buffers. Currently this is only
+// StringView and BinaryView.
+func HasBufferSizesBuffer(id arrow.Type) bool {
+ switch id {
+ case arrow.STRING_VIEW:
+ return true
+ case arrow.BINARY_VIEW:
+ return true
Review Comment:
```go
switch id {
case arrow.STRING_VIEW, arrow.BINARY_VIEW:
return true
```
##########
go/arrow/type_traits.go:
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package arrow
+
+import (
+ "reflect"
+ "unsafe"
+
+ "github.com/apache/arrow/go/v15/arrow/decimal128"
+ "github.com/apache/arrow/go/v15/arrow/decimal256"
+ "github.com/apache/arrow/go/v15/arrow/float16"
+ "golang.org/x/exp/constraints"
+)
+
+// IntType is a type constraint for raw values represented as signed
+// integer types by We aren't just using constraints.Signed
+// because we don't want to include the raw `int` type here whose size
+// changes based on the architecture (int32 on 32-bit architectures and
+// int64 on 64-bit architectures).
+//
+// This will also cover types like MonthInterval or the time types
+// as their underlying types are int32 and int64 which will get covered
+// by using the ~
+type IntType interface {
+ ~int8 | ~int16 | ~int32 | ~int64
+}
+
+// UintType is a type constraint for raw values represented as unsigned
+// integer types by We aren't just using constraints.Unsigned
+// because we don't want to include the raw `uint` type here whose size
+// changes based on the architecture (uint32 on 32-bit architectures and
+// uint64 on 64-bit architectures). We also don't want to include uintptr
+type UintType interface {
+ ~uint8 | ~uint16 | ~uint32 | ~uint64
+}
+
+// FloatType is a type constraint for raw values for representing
+// floating point values in This consists of constraints.Float and
+// float16.Num
+type FloatType interface {
+ float16.Num | constraints.Float
+}
+
+// NumericType is a type constraint for just signed/unsigned integers
+// and float32/float64.
+type NumericType interface {
+ IntType | UintType | constraints.Float
+}
+
+// FixedWidthType is a type constraint for raw values in Arrow that
+// can be represented as FixedWidth byte slices. Specifically this is for
+// using Go generics to easily re-type a byte slice to a properly-typed
+// slice. Booleans are excluded here since they are represented by Arrow
+// as a bitmap and thus the buffer can't be just reinterpreted as a []bool
+type FixedWidthType interface {
+ IntType | UintType |
+ FloatType | decimal128.Num | decimal256.Num |
+ DayTimeInterval | MonthDayNanoInterval
+}
+
+type TemporalType interface {
+ Date32 | Date64 | Time32 | Time64 |
+ Timestamp | Duration | DayTimeInterval |
+ MonthInterval | MonthDayNanoInterval
+}
+
+func sliceAs[Out, T interface{}](b []T) []Out {
+ len_bytes := len(b) * int(unsafe.Sizeof(b[0]))
+ cap_bytes := cap(b) * int(unsafe.Sizeof(b[0]))
+
+ var z Out
+ len_out := len_bytes / int(unsafe.Sizeof(z))
+ cap_out := cap_bytes / int(unsafe.Sizeof(z))
+
+ h := (*reflect.SliceHeader)(unsafe.Pointer(&b))
+ return unsafe.Slice((*Out)(unsafe.Pointer(h.Data)), cap_out)[:len_out]
+}
+
+func GetValues[T FixedWidthType](data ArrayData, i int) []T {
+ if data.Buffers()[i] == nil || data.Buffers()[i].Len() == 0 {
+ return nil
+ }
+ return
sliceAs[T](data.Buffers()[i].Bytes())[data.Offset():data.Offset()+data.Len()]
+}
+
+func GetOffsets[T int32 | int64](data ArrayData, i int) []T {
Review Comment:
same here and the ones below it
##########
go/arrow/type_traits.go:
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package arrow
+
+import (
+ "reflect"
+ "unsafe"
+
+ "github.com/apache/arrow/go/v15/arrow/decimal128"
+ "github.com/apache/arrow/go/v15/arrow/decimal256"
+ "github.com/apache/arrow/go/v15/arrow/float16"
+ "golang.org/x/exp/constraints"
+)
+
+// IntType is a type constraint for raw values represented as signed
+// integer types by We aren't just using constraints.Signed
+// because we don't want to include the raw `int` type here whose size
+// changes based on the architecture (int32 on 32-bit architectures and
+// int64 on 64-bit architectures).
+//
+// This will also cover types like MonthInterval or the time types
+// as their underlying types are int32 and int64 which will get covered
+// by using the ~
+type IntType interface {
+ ~int8 | ~int16 | ~int32 | ~int64
+}
+
+// UintType is a type constraint for raw values represented as unsigned
+// integer types by We aren't just using constraints.Unsigned
+// because we don't want to include the raw `uint` type here whose size
+// changes based on the architecture (uint32 on 32-bit architectures and
+// uint64 on 64-bit architectures). We also don't want to include uintptr
+type UintType interface {
+ ~uint8 | ~uint16 | ~uint32 | ~uint64
+}
+
+// FloatType is a type constraint for raw values for representing
+// floating point values in This consists of constraints.Float and
+// float16.Num
+type FloatType interface {
+ float16.Num | constraints.Float
+}
+
+// NumericType is a type constraint for just signed/unsigned integers
+// and float32/float64.
+type NumericType interface {
+ IntType | UintType | constraints.Float
+}
+
+// FixedWidthType is a type constraint for raw values in Arrow that
+// can be represented as FixedWidth byte slices. Specifically this is for
+// using Go generics to easily re-type a byte slice to a properly-typed
+// slice. Booleans are excluded here since they are represented by Arrow
+// as a bitmap and thus the buffer can't be just reinterpreted as a []bool
+type FixedWidthType interface {
+ IntType | UintType |
+ FloatType | decimal128.Num | decimal256.Num |
+ DayTimeInterval | MonthDayNanoInterval
+}
+
+type TemporalType interface {
+ Date32 | Date64 | Time32 | Time64 |
+ Timestamp | Duration | DayTimeInterval |
+ MonthInterval | MonthDayNanoInterval
+}
+
+func sliceAs[Out, T interface{}](b []T) []Out {
+ len_bytes := len(b) * int(unsafe.Sizeof(b[0]))
+ cap_bytes := cap(b) * int(unsafe.Sizeof(b[0]))
+
+ var z Out
+ len_out := len_bytes / int(unsafe.Sizeof(z))
+ cap_out := cap_bytes / int(unsafe.Sizeof(z))
+
+ h := (*reflect.SliceHeader)(unsafe.Pointer(&b))
+ return unsafe.Slice((*Out)(unsafe.Pointer(h.Data)), cap_out)[:len_out]
Review Comment:
fix the indentation, and let's make this more portable by just doing
`unsafe.Pointer(&b[0])` instead of going to the `reflect.SliceHeader`
##########
go/arrow/type_traits.go:
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package arrow
+
+import (
+ "reflect"
+ "unsafe"
+
+ "github.com/apache/arrow/go/v15/arrow/decimal128"
+ "github.com/apache/arrow/go/v15/arrow/decimal256"
+ "github.com/apache/arrow/go/v15/arrow/float16"
+ "golang.org/x/exp/constraints"
+)
+
+// IntType is a type constraint for raw values represented as signed
+// integer types by We aren't just using constraints.Signed
+// because we don't want to include the raw `int` type here whose size
+// changes based on the architecture (int32 on 32-bit architectures and
+// int64 on 64-bit architectures).
+//
+// This will also cover types like MonthInterval or the time types
+// as their underlying types are int32 and int64 which will get covered
+// by using the ~
+type IntType interface {
+ ~int8 | ~int16 | ~int32 | ~int64
+}
+
+// UintType is a type constraint for raw values represented as unsigned
+// integer types by We aren't just using constraints.Unsigned
+// because we don't want to include the raw `uint` type here whose size
+// changes based on the architecture (uint32 on 32-bit architectures and
+// uint64 on 64-bit architectures). We also don't want to include uintptr
+type UintType interface {
+ ~uint8 | ~uint16 | ~uint32 | ~uint64
+}
+
+// FloatType is a type constraint for raw values for representing
+// floating point values in This consists of constraints.Float and
+// float16.Num
+type FloatType interface {
+ float16.Num | constraints.Float
+}
+
+// NumericType is a type constraint for just signed/unsigned integers
+// and float32/float64.
+type NumericType interface {
+ IntType | UintType | constraints.Float
+}
+
+// FixedWidthType is a type constraint for raw values in Arrow that
+// can be represented as FixedWidth byte slices. Specifically this is for
+// using Go generics to easily re-type a byte slice to a properly-typed
+// slice. Booleans are excluded here since they are represented by Arrow
+// as a bitmap and thus the buffer can't be just reinterpreted as a []bool
+type FixedWidthType interface {
+ IntType | UintType |
+ FloatType | decimal128.Num | decimal256.Num |
+ DayTimeInterval | MonthDayNanoInterval
+}
+
+type TemporalType interface {
+ Date32 | Date64 | Time32 | Time64 |
+ Timestamp | Duration | DayTimeInterval |
+ MonthInterval | MonthDayNanoInterval
+}
+
+func sliceAs[Out, T interface{}](b []T) []Out {
+ len_bytes := len(b) * int(unsafe.Sizeof(b[0]))
+ cap_bytes := cap(b) * int(unsafe.Sizeof(b[0]))
+
+ var z Out
+ len_out := len_bytes / int(unsafe.Sizeof(z))
+ cap_out := cap_bytes / int(unsafe.Sizeof(z))
+
+ h := (*reflect.SliceHeader)(unsafe.Pointer(&b))
+ return unsafe.Slice((*Out)(unsafe.Pointer(h.Data)), cap_out)[:len_out]
+}
+
+func GetValues[T FixedWidthType](data ArrayData, i int) []T {
Review Comment:
since we're exposing this now, we should add a proper comment docstring to it
##########
go/arrow/type_traits.go:
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package arrow
+
+import (
+ "reflect"
+ "unsafe"
+
+ "github.com/apache/arrow/go/v15/arrow/decimal128"
+ "github.com/apache/arrow/go/v15/arrow/decimal256"
+ "github.com/apache/arrow/go/v15/arrow/float16"
+ "golang.org/x/exp/constraints"
+)
+
+// IntType is a type constraint for raw values represented as signed
+// integer types by We aren't just using constraints.Signed
+// because we don't want to include the raw `int` type here whose size
+// changes based on the architecture (int32 on 32-bit architectures and
+// int64 on 64-bit architectures).
+//
+// This will also cover types like MonthInterval or the time types
+// as their underlying types are int32 and int64 which will get covered
+// by using the ~
+type IntType interface {
+ ~int8 | ~int16 | ~int32 | ~int64
+}
+
+// UintType is a type constraint for raw values represented as unsigned
+// integer types by We aren't just using constraints.Unsigned
+// because we don't want to include the raw `uint` type here whose size
+// changes based on the architecture (uint32 on 32-bit architectures and
+// uint64 on 64-bit architectures). We also don't want to include uintptr
+type UintType interface {
+ ~uint8 | ~uint16 | ~uint32 | ~uint64
+}
+
+// FloatType is a type constraint for raw values for representing
+// floating point values in This consists of constraints.Float and
+// float16.Num
+type FloatType interface {
+ float16.Num | constraints.Float
+}
+
+// NumericType is a type constraint for just signed/unsigned integers
+// and float32/float64.
+type NumericType interface {
+ IntType | UintType | constraints.Float
+}
+
+// FixedWidthType is a type constraint for raw values in Arrow that
+// can be represented as FixedWidth byte slices. Specifically this is for
+// using Go generics to easily re-type a byte slice to a properly-typed
+// slice. Booleans are excluded here since they are represented by Arrow
+// as a bitmap and thus the buffer can't be just reinterpreted as a []bool
+type FixedWidthType interface {
+ IntType | UintType |
+ FloatType | decimal128.Num | decimal256.Num |
+ DayTimeInterval | MonthDayNanoInterval
+}
+
+type TemporalType interface {
+ Date32 | Date64 | Time32 | Time64 |
+ Timestamp | Duration | DayTimeInterval |
+ MonthInterval | MonthDayNanoInterval
+}
+
+func sliceAs[Out, T interface{}](b []T) []Out {
+ len_bytes := len(b) * int(unsafe.Sizeof(b[0]))
+ cap_bytes := cap(b) * int(unsafe.Sizeof(b[0]))
Review Comment:
this'll blow up if `len(b) == 0`
--
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]
