Skip to content
This repository has been archived by the owner on Aug 24, 2021. It is now read-only.

Expose CSV options to Go #63

Open
tebeka opened this issue Mar 16, 2020 · 5 comments
Open

Expose CSV options to Go #63

tebeka opened this issue Mar 16, 2020 · 5 comments

Comments

@tebeka
Copy link
Member

tebeka commented Mar 16, 2020

The arrow CSV package has the following options: ReadOptions, ParseOptions & ConvertOptions. Expose them to Go.

@tebeka
Copy link
Member Author

tebeka commented Mar 16, 2020

Maybe use functional options

@tebeka
Copy link
Member Author

tebeka commented Mar 17, 2020

Problem

These option structs are C++ structs, they have a static Defaults function that
returns a struct (not pointer). When creating an arrow::csv::Reader you need
to pass options by reference.

cgo can’t work directly with C++, which means we can use them directly as
C.ParseOptions.

Possible Solutions

new function

A C function that will get all the parameters for an options struct and create
it.

Problems

  • Need to update the function every time options struct is updated at the C++
    level
  • Probably need to be part of making a reader since we need to pass the struct
    by reference
  • A lot of parameters1, since we have several such options struct

Use Pointers

Have C wrappers that will create a pointer to options struct and have setters.

Problems

  • Who frees the memory?
  • A lot of setter functions
  • Extra memory allocation

C Shadow struct

Have a C struct that will be a copy of the C++ struct. Can be used directly
from Go and passed around by value.

Problems

  • Need to be in par with C++
  • Duplication (code & memory)
    • Since it’s a creator function, don’t see memory as a bit problem

SWIG

Use swig to generate wrappers.

Problems

  • Learning curve
  • Looks like need to be more systematic approach
  • Not sure how it’ll work with C++ calling into Go (GoStream class)

1: If you have a procedure with 10 parameters, you probably missed some. - Alan Perlis

@tebeka
Copy link
Member Author

tebeka commented Mar 17, 2020

@yonidavidson Care to weigh in on the above?

@yonidavidson
Copy link
Collaborator

This can be a good POC for SWIG, yet I don't think SWIG will be our silver bullet so It might not be worth to add complexity to our system with it.
I think a functional API is a good choice, it gives you an ability to have a good default and to expose the parameters in a need to have way (easy to extend).

@tebeka
Copy link
Member Author

tebeka commented Mar 17, 2020

I'll probably go with C shadow struct for now. I'll defer SWIG for when we go all-in on it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants