nullif operating on ArrayRef

[bjchambers]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

The nullif kernel currently only applies to PrimitiveArray<T>. This makes it impossible to use with non-primitive types, which often arise when working with structured / nested data. Additionally, it seems like the nullif kernel should have a similar signature to filter -- where filter selects only the non-true rows, nullif nulls out the true rows.

Describe the solution you'd like
The signature of nullif changed to nullif(values: ArrayRef, condition: BooleanArray) -> Result<ArrayRef>.
Possibly the addition of the complement null_unless.

I have a draft that I think would work (can open a PR), but wanted to make sure that such a kernel is desirable, and understand whether it should use a different name to avoid API breakages (generic_nullif or something like that).

[jorgecarleitao]

Yep, that aligns well with the other kernels, and it is in general preferable as it avoids others having to do the same.

I would use &dyn Array instead of ArrayRef as its argument, since there is no need to require Arcing an array for using the kernel. With this signature, we can write

let array: PrimtiveArray<i32> = ...;
let array = nullif(&array)?;

instead of

let array: PrimtiveArray<i32> = ...;
let array = nullif(Arc::new(array))?;

[bjchambers]

I can do that. Although there are cases where nullif could return a reference. For instance if the boolean column is all false and non-null. I guess maybe the Arc doesn't provide large wins of the underlying data is Arc'ed. I ran into a problem with equality of sliced struct arrays. I'll post a draft PR and maybe file another issue.

…

On Tue, Jun 29, 2021, 11:38 PM Jorge Leitao ***@***.***> wrote: Yep, that aligns well with the other kernels, and it is in general preferable as it avoids others having to do the same. I would use &dyn Array instead of ArrayRef as its argument, since there is no need to require Arcing an array for using the kernel. With this signature, we can write let array: PrimtiveArray<i32> = ...;let array = nullif(&array)?; instead of let array: PrimtiveArray<i32> = ...;let array = nullif(&Arc::new(array))?; — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#510 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAIY6ECTXUDNHCOYERT5NTTVK3VDANCNFSM47RGUOQQ> .

[bjchambers]

Draft of the updated kernel is up. It still uses ArrayRef because there are a few cases where the reference to the input array is cloned and I'm not sure there is a good way to handle that taking &dyn Array (eg., wouldn't it have to clone the &dyn Arary to produce a new Array)?

Note that one of the added tests is failing for what I believe is a bug in struct equality (#514).