[C++] Implement safe casts for primitive types

[asfimport]

In some situations, you want to cast the data in a PrimitiveArray to a different (but similar) data type, e.g. from uint32_t to int32_t or uint32_t to uint8_t. This can either be done by reinterpreting the data or needs to involve a copy if the size of the underlying type changes. There is already an implementation for this in parquet-cpp that could be pulled out into Arrow: https://github.com/apache/parquet-cpp/blob/9a0407e684c0a6299d0e6ab98c11c1162915c0ee/src/parquet/arrow/writer.cc#L71

Reporter: Uwe Korn / @xhochy
Assignee: Wes McKinney / @wesm

Related issues:

• [C++] Type casting function kernel suite (Is contained by)
• [C++] Expose type casts as generic callable object that can write into pre-allocated memory (is related to)
• [Python] pyarrow.Array.from_pandas should take a type parameter (is depended upon by)

_{Note: This issue was originally created as ARROW-229. Please see the migration documentation for further details.}

[asfimport]

Wes McKinney / @wesm:
Repurposed this JIRA and expanded scope somewhat. We need to create a module of safe casts (i.e. between integer types, or integers and floats). We can also add an option to perform unsafe cast without warnings / errors (the equivalent of using static_cast<T>). We need this for some other features in the queue

[asfimport]

Ankit Aggarwal:
Created the pull request
PR : #986

[asfimport]

Wes McKinney / @wesm:
Thank you for your patch. The scope of what I am intending here is fairly large. I am expecting to add a new public API:

Status SafeCast(const Array& input, const std::shared_ptr<DataType>& target_type,
                MemoryPool* pool,
                std::shared_ptr<Array>* out);

This will mean some dynamic dispatch work and significantly more testing to check casts that would cause data loss.

[asfimport]

Ankit Aggarwal:
Scope: To define safe casts functions for the each type (arrow::Type). For example - TIME32 to TIME64, Union Array to Dictionary Array

A possible implementation of how it can be done:
Tag dispatching and calling the SetData function for the required type (arrow::Array) if it is array type or reinterpret_cast for the numerical type and assigning it to the output pointer.

Status SafeCast(const Array& input, const std::shared_ptr<DataType>& target_type,
                MemoryPool* pool,
                std::shared_ptr<Array>* out);

In the API suggested why do we need the MemoryPool pointer?
Do we need to check whether the SafeCast is possible or not?

[asfimport]

Wes McKinney / @wesm:
Any function that is capable of allocating memory must be passed a MemoryPool so that allocations can be accounted for. If the input type and the target type are the same, then no memory is allocated, otherwise new memory is allocated.

It would most consistent to use the visitor pattern for this dynamic dispatch as we have done elsewhere in the library.

[asfimport]

Ankit Aggarwal:
Hi Wes

I have created a gist for this
"https://gist.github.com/AnkitAggarwalPEC/7b13e51952f89ac826ea240d0e44b610"

There is a couple of design issue that I'm facing:

1. When the data type is same. I thought it would be better to use directly "SetData" function but it is declared as protected, so It cannot be called. By making it public it solves the issue.
2. When the data type is not same, I'm trying to use the MakeBuilder function to get the required data type builder and use the Append function accordingly but Append is not declared in the base class of Array Builder. Should we change that or try an another way?

Ankit

[asfimport]

Wes McKinney / @wesm:
hi [~ankit5012], would it be OK if I took an initial pass at writing an implementation of this, and we can discuss further in code review on GitHub?

This is one of the most complex issues currently in the Arrow JIRA as it touches most aspects of C++ library: memory management, array containers, array builders, multiple dispatch, etc. Because Arrow is not a compile-time C++ library, we must compile cast functions for each pair of supported input and output type so that the specific implementations are available in the shared library.

TensorFlow's implementation of cast might give us some ideas, see all of the files starting with cast_op_impl* in

https://github.com/tensorflow/tensorflow/tree/master/tensorflow/core/kernels

And the dynamic dispatch here:

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/cast_op.cc#L99

Arrow is actually more complicated since we have non-numeric and nested types to think about. I'd like to do something for this JIRA for numeric, non-nested types, while leaving the door open for extending to more complicated types (e.g. you could imagine casting List<Int64> to List<Int32>).

[asfimport]

Ankit Aggarwal:
Okay. There will be a no issue if you took an initial pass

[asfimport]

Uwe Korn / @xhochy:
I'm happy with the proposed interface but I would also like to see

Status SafeCast(const Array& input, const std::shared_ptr<DataType>& target_type,
                MemoryPool* pool,
                std::shared_ptr<Array>* out);

template <typename SourceType, typename TargetType>
Status TypedSafeCast(const TypedArray<ArrowType>& input, MemoryPool* pool,
                std::shared_ptr<Array>* out);

Where we have e.g. TypedArray<Int64Type> = NumericArray<Int64Type>. This would enable typed C++ code to directly pass along its type information. I would expect an implementation of SafeCast to actually just call TypedSafeCast.

[asfimport]

Wes McKinney / @wesm:
The latter is tricky because it means that the TypedSafeCast symbols must be exported in the shared library, which places some constraints on the shape of the implementation. I will see how much progress I can make and we can discuss further in the PR

[asfimport]

Wes McKinney / @wesm:
Started on this in: #1027. We can iterate on the implementation details; I'll write enough unit tests to feel good about numeric casts and then we can add Python bindings in a subsequent patch.

[asfimport]

Wes McKinney / @wesm:
Issue resolved by pull request 1027
#1027