353solutions / carrow

Go wrapper for Apache Arrow C++
https://arrow.apache.org/
BSD 3-Clause "New" or "Revised" License
15 stars 0 forks source link

Generate Append functions by template #34

Open yonidavidson opened 5 years ago

yonidavidson commented 5 years ago

This requires a bit more refactoring in the C side in order to get a generic structure for this type of functions.

For example:

func (b *TimestampArrayBuilder) Append(val time.Time) error {
    r := C.array_builder_append_timestamp(b.ptr, C.longlong(val.UnixNano()))
    if r.err != nil {
        return errFromResult(r)
    }
    return nil
}
Go type: time.Time
C.array_builder_append_timestamp

should pass a pointer (and the C function will cast it) val.UnixNano should be defined as a Mutator function for the template.

tebeka commented 5 years ago

@yonidavidson Can you format the code above?

tebeka commented 5 years ago

C.array_builder_append_timestamp should pass a pointer I don't understand why

yonidavidson commented 5 years ago

C.array_builder_append_timestamp should pass a pointer I don't understand why

In

func (b *Float64ArrayBuilder) Append(val float64) error {
    r := C.array_builder_append_float(b.ptr, C.double(val))
    if r.err != nil {
        return errFromResult(r)
    }
    return nil
}

we are sending a C.double(val) in

func (b *Integer64ArrayBuilder) Append(val int64) error {
    r := C.array_builder_append_int(b.ptr, C.longlong(val))
    if r.err != nil {
        return errFromResult(r)
    }
    return nil
}

we are sending a C.longlong(val) I think that that the knowledge about C.TYPE should be set by the C layer (we call a different function each time) and the data is passed as a pointer this will allow the following function signature:

func (b *Integer64ArrayBuilder) Append(val int64) error {
    r := C.array_builder_append_int(b.ptr, val.ptr)
    if r.err != nil {
        return errFromResult(r)
    }
    return nil
}

To be honest, now that I am writing this down I am not sure that the Go GC allows can promise that memory in the Go run time will be preserved during the C function run ( and C layer will need to copy that memory and not store it - a point of failure ) so maybe I'll need to think on a different technique .