Reported through build2's ci (using my wip packaging) :
In the configurations we build and test on github, float32_t and float64_t usually are aliases to double and float. However in some other configurations we didnt test, these names are not aliases, they are concrete separate types (note also the possibility coming from using our float16_t implementation depending on if the standard library available provides or not fixed floating point types). In our tests, we use double directly in some places where we end up with arrow_traits<double>. When float64_t is an alias to double (like on our ci), arrow_traits<double> and arrow_traits<float64_t> are the same type. When it's not (some configurations on build2's CI) these are different types.
At the moment we only define arrow_traits<float64_t>, therefore arrow_traits<double> is never found at compile-time and support for double is therefore non-existent in these configurations, leading to compilation errors when trying to use double in sparrow.
Attempt to fix that issue by adding arrow_traits<double> (and float) leads to duplicate type definition errors in configurations where float64_t are aliases to double, so it's the reverse problem.
In my experiments, the best way to fix this seems to be to have only one arrow_traits<std::floating_point> template which handles all the possible floating point types standard at least. In that case, type_id needs to be set to a value depending on the size of the type. My current experiment half-works (it reveals other similar issues) and I intend to complete it to completely solve the issue.
Note that the same solution can be set for integers, I'll try to see if that works too. That would prevent hitting similar issues on exotic platforms with integers and actually reduce the code size for arrow traits provided by default.
Reported through
build2's ci (using my wip packaging) :In the configurations we build and test on github,
float32_tandfloat64_tusually are aliases todoubleandfloat. However in some other configurations we didnt test, these names are not aliases, they are concrete separate types (note also the possibility coming from using our float16_t implementation depending on if the standard library available provides or not fixed floating point types). In our tests, we usedoubledirectly in some places where we end up witharrow_traits<double>. Whenfloat64_tis an alias todouble(like on our ci),arrow_traits<double>andarrow_traits<float64_t>are the same type. When it's not (some configurations onbuild2's CI) these are different types.At the moment we only define
arrow_traits<float64_t>, thereforearrow_traits<double>is never found at compile-time and support fordoubleis therefore non-existent in these configurations, leading to compilation errors when trying to usedoublein sparrow.Attempt to fix that issue by adding
arrow_traits<double>(andfloat) leads to duplicate type definition errors in configurations wherefloat64_tare aliases todouble, so it's the reverse problem.In my experiments, the best way to fix this seems to be to have only one
arrow_traits<std::floating_point>template which handles all the possible floating point types standard at least. In that case,type_idneeds to be set to a value depending on the size of the type. My current experiment half-works (it reveals other similar issues) and I intend to complete it to completely solve the issue.Note that the same solution can be set for integers, I'll try to see if that works too. That would prevent hitting similar issues on exotic platforms with integers and actually reduce the code size for arrow traits provided by default.