Handling circular type definitions

[randyzwitch]

The Thrift spec allows for circular type definition; this currently throws an error (I presume from top-down parsing of the .thrift file)

/* union */ struct TDatumVal {
  1: i64 int_val,
  2: double real_val,
  3: string str_val,
  4: list<TDatum> arr_val
}

struct TDatum {
  1: TDatumVal val,
  2: bool is_null
}

Result:

> mapdthrift = t_load("src/mapd.thrift")
Error in value[[3L]](cond) : No type found: TDatum, at line 64

[marekjagielski]

I would check if https://github.com/eleme/thriftpy handles circular types. I am guessing that not. Anyway, I think it would be possible to implement it. I would also start from contributing the change to thriftpy.

[randyzwitch]

thriftpy has this as an open issue as well

[randyzwitch]

https://github.com/eleme/thriftpy/issues/280

[ethe]

It has already been solved in thriftpy2, thanks!

[randyzwitch]

Thanks for letting us know @ethe!

@marekjagielski How are you generating the R code from the Python code? Is it something I can help with? I'd love to give thriftr another chance, as I'd love to avoid wrapping the Python thrift library using reticulate and just use plain R directly

[marekjagielski]

@randyzwitch , I am happy that you are willing to contribute. I will be busy next 2 months, so I will not be able to code on thriftr. There is no automatic generation of R code. I just went line by line and tried to translate the syntax. So this logic has to be added to thriftr: https://github.com/Thriftpy/thriftpy2/pull/21/files

[ethe]

There are some bugs in ThriftPy handling out-of-order definitions, see https://github.com/Thriftpy/thriftpy2/pull/42 and get more information.

[marekjagielski]

@ethe Thank you! I will incorporate these changes to thriftr.