felipecrv commented on code in PR #39019:
URL: https://github.com/apache/arrow/pull/39019#discussion_r1425745066
##########
go/arrow/cdata/cdata_exports.go:
##########
@@ -328,6 +336,15 @@ func allocateBufferPtrArr(n int) (out []*C.void) {
return
}
+func allocateBufferSizeArr(n int) (out []C.int64_t) {
+ s := (*reflect.SliceHeader)(unsafe.Pointer(&out))
+ s.Data = uintptr(C.calloc(C.size_t(n),
C.size_t(unsafe.Sizeof(int64(0)))))
+ s.Len = n
+ s.Cap = n
+
+ return
Review Comment:
is this necessary?
##########
go/arrow/internal/flatbuf/DictionaryBatch.go:
##########
@@ -73,9 +73,9 @@ func (rcv *DictionaryBatch) Data(obj *RecordBatch)
*RecordBatch {
return nil
}
-/// If isDelta is true the values in the dictionary are to be appended to a
-/// dictionary with the indicated id. If isDelta is false this dictionary
-/// should replace the existing dictionary.
+// / If isDelta is true the values in the dictionary are to be appended to a
+// / dictionary with the indicated id. If isDelta is false this dictionary
+// / should replace the existing dictionary.
Review Comment:
I think you should undo these changes `git checkout -p origin/main` and have
a separate PR where we make the who Go codebase auto-formattable.
##########
go/arrow/compute/fieldref.go:
##########
@@ -346,17 +346,18 @@ func FieldRefList(elems ...interface{}) FieldRef {
// NewFieldRefFromDotPath parses a dot path into a field ref.
//
// dot_path = '.' name
-// | '[' digit+ ']'
-// | dot_path+
+//
+// | '[' digit+ ']'
+// | dot_path+
//
// Examples
//
-// ".alpha" => FieldRefName("alpha")
-// "[2]" => FieldRefIndex(2)
-// ".beta[3]" => FieldRefList("beta", 3)
-// "[5].gamma.delta[7]" => FieldRefList(5, "gamma", "delta", 7)
-// ".hello world" => FieldRefName("hello world")
-// `.\[y\]\\tho\.\` => FieldRef(`[y]\tho.\`)
+// ".alpha" => FieldRefName("alpha")
+// "[2]" => FieldRefIndex(2)
+// ".beta[3]" => FieldRefList("beta", 3)
+// "[5].gamma.delta[7]" => FieldRefList(5, "gamma", "delta", 7)
+// ".hello world" => FieldRefName("hello world")
+// `.\[y\]\\tho\.\` => FieldRef(`[y]\tho.\`)
Review Comment:
Code should be indented or start end and with ` ``` `
##########
go/arrow/internal/flatbuf/Binary.go:
##########
@@ -22,7 +22,7 @@ import (
flatbuffers "github.com/google/flatbuffers/go"
)
-/// Opaque binary data
+// / Opaque binary data
Review Comment:
Auto formatter noise? How can we prevent this?
##########
go/arrow/internal/debug/log_on.go:
##########
@@ -14,6 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+//go:build debug
Review Comment:
no space after `//`?
##########
go/arrow/compute/fieldref.go:
##########
@@ -346,17 +346,18 @@ func FieldRefList(elems ...interface{}) FieldRef {
// NewFieldRefFromDotPath parses a dot path into a field ref.
//
// dot_path = '.' name
-// | '[' digit+ ']'
-// | dot_path+
+//
+// | '[' digit+ ']'
+// | dot_path+
Review Comment:
the `|` should align with the `=` on the previous line
##########
go/arrow/cdata/cdata.go:
##########
@@ -683,6 +743,43 @@ func (imp *cimporter) importListLike() (err error) {
return
}
+func (imp *cimporter) importListViewLike() (err error) {
+ offsetSize := int64(imp.dt.Layout().Buffers[1].ByteWidth)
+
+ if err = imp.checkNumChildren(1); err != nil {
+ return err
+ }
+
+ if err = imp.checkNumBuffers(3); err != nil {
+ return err
+ }
+
+ var nulls, offsets, sizes *memory.Buffer
+ if nulls, err = imp.importNullBitmap(0); err != nil {
+ return
+ }
+ if nulls != nil {
+ defer nulls.Release()
+ }
+
+ if offsets, err = imp.importOffsetsBuffer(1, offsetSize); err != nil {
Review Comment:
Is `importOffsetsBuffer` importing only `len` offsets instead of `len + 1`?
##########
go/arrow/compute/fieldref.go:
##########
@@ -282,31 +282,31 @@ type refImpl interface {
//
// Nested fields can be referenced as well, given the schema:
//
-// arrow.NewSchema([]arrow.Field{
-// {Name: "a", Type: arrow.StructOf(arrow.Field{Name: "n",
Type: arrow.Null})},
-// {Name: "b", Type: arrow.PrimitiveTypes.Int32},
-// })
+// arrow.NewSchema([]arrow.Field{
+// {Name: "a", Type:
arrow.StructOf(arrow.Field{Name: "n", Type: arrow.Null})},
+// {Name: "b", Type: arrow.PrimitiveTypes.Int32},
+// })
//
// the following all indicate the nested field named "n":
//
-// FieldRefPath(FieldPath{0, 0})
-// FieldRefList("a", 0)
-// FieldRefList("a", "n")
-// FieldRefList(0, "n")
-// NewFieldRefFromDotPath(".a[0]")
+// FieldRefPath(FieldPath{0, 0})
+// FieldRefList("a", 0)
+// FieldRefList("a", "n")
+// FieldRefList(0, "n")
+// NewFieldRefFromDotPath(".a[0]")
//
// FieldPaths matching a FieldRef are retrieved with the FindAll* functions
// Multiple matches are possible because field names may be duplicated within
// a schema. For example:
//
-// aIsAmbiguous := arrow.NewSchema([]arrow.Field{
-// {Name: "a", Type: arrow.PrimitiveTypes.Int32},
-// {Name: "a", Type: arrow.PrimitiveTypes.Float32},
-// })
-// matches := FieldRefName("a").FindAll(aIsAmbiguous)
-// assert.Len(matches, 2)
-//
assert.True(matches[0].Get(aIsAmbiguous).Equals(aIsAmbiguous.Field(0))
-//
assert.True(matches[1].Get(aIsAmbiguous).Equals(aIsAmbiguous.Field(1))
+// aIsAmbiguous := arrow.NewSchema([]arrow.Field{
+// {Name: "a", Type: arrow.PrimitiveTypes.Int32},
+// {Name: "a", Type: arrow.PrimitiveTypes.Float32},
+// })
+// matches := FieldRefName("a").FindAll(aIsAmbiguous)
+// assert.Len(matches, 2)
+// assert.True(matches[0].Get(aIsAmbiguous).Equals(aIsAmbiguous.Field(0))
+// assert.True(matches[1].Get(aIsAmbiguous).Equals(aIsAmbiguous.Field(1))
Review Comment:
If this is markdown, code should be indented 4 spaces.
##########
go/arrow/compute/fieldref.go:
##########
@@ -282,31 +282,31 @@ type refImpl interface {
//
// Nested fields can be referenced as well, given the schema:
//
-// arrow.NewSchema([]arrow.Field{
-// {Name: "a", Type: arrow.StructOf(arrow.Field{Name: "n",
Type: arrow.Null})},
-// {Name: "b", Type: arrow.PrimitiveTypes.Int32},
-// })
+// arrow.NewSchema([]arrow.Field{
+// {Name: "a", Type:
arrow.StructOf(arrow.Field{Name: "n", Type: arrow.Null})},
+// {Name: "b", Type: arrow.PrimitiveTypes.Int32},
+// })
Review Comment:
Why the indentation increased here?
--
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]
