Beside reporting errors, maybe a kernel wants to allocate memory through KernelContext::memory_pool [1] in Kernel::init? I'm not quite sure if this is a valid case. Would like to hear other comments.
[1] https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/kernel.h#L95 Yibo On 3/12/21 5:24 AM, Benjamin Kietzman wrote:
KernelContext is a tuple consisting of a pointers to an ExecContext and KernelState and an error Status. The context's error Status may be set by compute kernels (for example when divide-by-zero would occur) rather than returning a Result as in the rest of the codebase. IIUC the intent is to avoid branching on always-ok Statuses for kernels which don't have an error condition (for example addition without overflow checks). If there's a motivating performance reason for non standard error propagation then we should continue using KernelContext wherever we can benefit from it. However, several other APIs (such as Kernel::init) also use a KernelContext to report errors. IMHO, it would be better to avoid the added cognitive overhead of handling errors through KernelContext outside hot loops which benefit from it. Am I missing anything? Is there any reason (for example) Kernel::init shouldn't just return a Result<unique_ptr<KernelState>>? Ben Kietzman
