Hi Wenbo, Sorry I wasn't clear enough. There are two issues with the code snippet.
As Aldrin pointed out, you are working with uint8_t*, which I don't think is right. The buffer stores bytes, not pointers. auto *out_values = out->array_span_mutable()->GetValues<uint8_t*>(1); > It should be GetValues<uint8_t>(1), not GetValues<uint8_t*>(1) memcpy(*out++, hash, 32); > It should be memcpy(out, hash, 32); out += 32; Also, I see that you are not handling nulls at all. I assume you want to propagate nulls from the input. In that case you'll want to set kernel.null_handling=NullHandling::INTERSECTION, so that the executor will handle the validity bitmap for you. Best, Jin On Mon, Jul 17, 2023 at 10:41 PM Weston Pace <weston.p...@gmail.com> wrote: > > I may be missing something, but why copy to *out_values++ instead of > > *out_values and add 32 to out_values afterwards? Otherwise I agree this > is > > the way to go. > > I agree with Jin. You should probably be incrementing `out` by 32 each > time `VisitValue` is called. > > On Mon, Jul 17, 2023 at 6:38 AM Aldrin <octalene....@pm.me.invalid> wrote: > > > Oh wait, I see now that you're incrementing with a uint8_t*. That could > be > > fine for your own use, but you might want to make sure it aligns with the > > type of your output (Int64Array vs Int32Array). > > > > Sent from Proton Mail for iOS > > > > > > On Mon, Jul 17, 2023 at 06:20, Aldrin <octalene....@pm.me.INVALID > > <On+Mon,+Jul+17,+2023+at+06:20,+Aldrin+%3C%3Ca+href=>> wrote: > > > > Hi Wenbo, > > > > An ArraySpan is like an ArrayData but does not own the data, so the > > ColumnarFormat doc that Jon shared is relevant for both. > > > > In the case of a binary format, the output ArraySpan must have at least 2 > > buffers: the offsets and the contiguous binary data (values). If the > output > > of your UDF is something like an Int32Array with no nulls, then I think > > you're writing output correctly. > > > > But, since your pointer is a uint8_t, I think Jin is right and `++` is > > going to move your pointer 1 byte instead of 32 bytes like you intend. > > > > Sent from Proton Mail for iOS > > > > > > On Mon, Jul 17, 2023 at 05:06, Wenbo Hu <huwenbo1...@gmail.com > > <On+Mon,+Jul+17,+2023+at+05:06,+Wenbo+Hu+%3C%3Ca+href=>> wrote: > > > > Hi Jin, > > > > > but why copy to *out_values++ instead of > > > *out_values and add 32 to out_values afterwards? > > I'm implementing the sha256 function as a scalar function, but it > > always inputs with an array, so on visitor pattern, I'll write a 32 > > byte hash into the pointer and move to the next for next visit. > > Something like: > > ``` > > > > struct BinarySha256Visitor { > > BinarySha256Visitor(uint8_t **out) { > > this->out = out; > > } > > arrow::Status VisitNull() { > > return arrow::Status::OK(); > > } > > > > arrow::Status VisitValue(std::string_view v) { > > > > uint8_t hash[32]; > > sha256(v, hash); > > > > memcpy(*out++, hash, 32); > > > > return arrow::Status::OK(); > > } > > > > uint8_t ** out; > > }; > > > > arrow::Status Sha256Func(cp::KernelContext *ctx, const cp::ExecSpan > > &batch, cp::ExecResult *out) { > > arrow::ArraySpanVisitor<arrow::BinaryType> visitor; > > > > auto *out_values = out->array_span_mutable()->GetValues<uint8_t*>(1); > > BinarySha256Visitor visit(out_values); > > ARROW_RETURN_NOT_OK(visitor.Visit(batch[0].array, &visit)); > > > > return arrow::Status::OK(); > > } > > ``` > > Is it as expected? > > > > Jin Shang <shangjin1...@gmail.com> 于2023年7月17日周一 19:44写道: > > > > > > Hi Wenbo, > > > > > > I'd like to known what's the *three* `buffers` are in ArraySpan. What > are > > > > `1` means when `GetValues` called? > > > > > > The meaning of buffers in an ArraySpan depends on the layout of its > data > > > type. FixedSizeBinary is a fixed-size primitive type, so it has two > > > buffers, one validity buffer and one data buffer. So GetValues(1) would > > > return a pointer to the data buffer. > > > Layouts of data types can be found here[1]. > > > > > > what is the actual type should I get from `GetValues`? > > > > > > > Buffer data is stored as raw bytes (uint8_t) but can be reinterpreted > as > > > any type to suit your need. The template parameter for GetValue is > simply > > > forwarded to reinterpret_cast. There are discussions[2] on the > soundness > > of > > > using uint8_t to represent bytes but it is what we use now. Since you > are > > > only doing a memcpy, uint8_t should be good. > > > > > > Maybe, `auto *out_values = out->array_span_mutable()->GetValues(uint8_t > > > > *>(1);` and `memcpy(*out_values++, some_ptr, 32);`? > > > > > > > I may be missing something, but why copy to *out_values++ instead of > > > *out_values and add 32 to out_values afterwards? Otherwise I agree this > > is > > > the way to go. > > > > > > [1] > > > > > > https://arrow.apache.org/docs/format/Columnar.html#buffer-listing-for-each-layout > > > [2] https://github.com/apache/arrow/issues/36123 > > > > > > > > > On Mon, Jul 17, 2023 at 4:44 PM Wenbo Hu <huwenbo1...@gmail.com> > wrote: > > > > > > > Hi, > > > > I'm using Acero as the stream executor to run large scale data > > > > transformation. The core data used in UDF is `ArraySpan` in > > > > `ExecSpan`, but not much document on ArraySpan. I'd like to known > > > > what's the *three* `buffers` are in ArraySpan. What are `1` means > when > > > > `GetValues` called? > > > > For input data, I can use a `ArraySpanVisitor` to iterator over > > > > different input types. But for output data, I don't know how to write > > > > to the`array_span_mutable()` if it is not a simple c_type. > > > > For example, I'm implementing a sha256 udf, which input is > > > > `arrow::utf8()` and the output is `arrow::fixed_size_binary(32)`, > then > > > > how can I directly write to the out buffers and what is the actual > > > > type should I get from `GetValues`? > > > > Maybe, `auto *out_values = > > > > out->array_span_mutable()->GetValues(uint8_t *>(1);` and > > > > `memcpy(*out_values++, some_ptr, 32);`? > > > > > > > > -- > > > > --------------------- > > > > Best Regards, > > > > Wenbo Hu, > > > > > > > > > > > > -- > > --------------------- > > Best Regards, > > Wenbo Hu, > > > > >