[REVIEW] Adds fill, validityAsBooleanVector, replaceNulls, and isNull/isNotNull [skip ci]

[abellina]

This PR adds support in the java api for:

• Whole vector filling (internal api): to mirror cudf::fill for the full vector case
• ColumnVector.replaceNulls: to mirror cudf::replace_nulls
• ColumnVector.validityAsBooleanVector: Obtaining the validity bitmask as a boolean ColumnVector. Note: this function will go away if libcudf adds support for isNull as a column operation.
• ColumnVector.isNull/isNotNull: required when filtering nullable columns
• ColumnVector.fromScalar: given a Scalar and a size, returns a ColumnVector of that size filled with the scalar's value

[jrhemstad]

Out of curiosity, how is validityAsBooleanVector implemented? I don't believe we have this in libcudf yet, but we have planned to add it eventually.

[abellina]

@jrhemstad we filled a boolean vector with true (using cudf::fill), then we take the validity bitmask from the original vector and set it on this new vector. This allows us to call cudf::replace_nulls with false.

[revans2]

@abellina when the PR is ready for review please tag it, but also put the [REVIEW] in the title. Java is also not tied into the CI system yet, so until that happens you can put [skip ci] at the end to avoid running the ci tests.

[jrhemstad]

@jrhemstad we filled a boolean vector with true (using cudf::fill), then we take the validity bitmask from the original vector and set it on this new vector. This allows us to call cudf::replace_nulls with false.

Ah, okay, that makes sense. It'd certainly be more efficient to implement this in a custom kernel.

I think your solution makes sense in the medium term. Would you mind creating an issue to add this as a native libcudf feature?

[abellina]

@jrhemstad will do. It would be best if isNull and isNotNull also moved to libcudf as unary ops. I'll create issues for these as well.

[revans2]

I am a little nervous about fill being in place, simply because none of our other code expects to have the contents of the vector change underneath it and we have to do things to update the cached java-side code etc. The way we are using it now is fine, but we may want to discuss a bit more about if we do expose it publicly how are we going to handle it so we don't have issues later on, and if we don't expose it then we should fold fill and fromScalar into a single function and remove a lot of the overlap between them.

[harrism]

I had requested @abellina to file an issue first so we could discuss the best approach for this in libcudf before implementing it...

[harrism]

• Whole vector filling (internal api): to mirror cudf::fill for the full vector case

cudf::fill is not just "whole-vector" filling. You can use it to fill any contiguous range within a vector.

• ColumnVector.replaceNulls: to mirror cudf::replace_nulls
• ColumnVector.validityAsBooleanVector: Obtaining the validity bitmask as a boolean ColumnVector
• ColumnVector.isNull/isNotNull: required when filtering nullable columns
• ColumnVector.fromScalar: given a Scalar and a size, returns a ColumnVector of that size filled with the scalar's value

All of these things seem useful (except validityAsBooleanVector, which to me sounds identical to isNotNull) to have in libcudf, where we could ensure they are efficient.

[abellina]

@harrism, for cudf::fill I didn't include the range version because I experienced this #2142. I can predict the null_count if we fill the whole vector (all nulls, or no nulls), so that was easy for me to work around in this case.

validityAsBooleanVector and isNotNull are equivalent. The idea is to remove validityAsBooleanVector once isNotNull is implemented in libcudf. I should have made that clear in the description of this PR. Here's the promised issue: #2143

[revans2]

This is conditional on approval from @harrism too.

I really would like to see isNull and isNotNull go in, because it is blocking us doing other things with Spark, but at the same time, I want to be sure that we do it right eventually so waiting until we know how to do it right is fine.

[harrism]

I'm fine with you guys proceeding as long as you replace your code with calls to the libcudf version once it exists.

[abellina]

I added a commit to remove the overwrite I did for null_count (after fill), as we now have #2142 merged. @tgravescs could you give it a look?

A follow up pr should add the java cudf::fill functions.

[abellina]

Actually, please hold on merging this. I think I found something else.

[abellina]

@tgravescs ok it's good now. I had to set the bitmask to all 1's before sending it to cudf::fill in my case, as it should be a valid bitmask and corresponding null_count. This makes sense, and it broke one of my tests once I removed a workaround for null_count I had added, so I had to debug.

[abellina]

@tgravescs comments addressed.

[abellina]

@kkraus14 I think we are good to go. Also, should we just not do [skip ci] s.t. pull requests don't get stuck here?