Fix unreachable else branch in gather bitmask logic

[eternallyproud]

Description

Fix unreachable else branch in gather() bitmask logic.

In the original code, needs_new_bitmask is already true when entering
the outer if block, so the reassignment
needs_new_bitmask = needs_new_bitmask || cudf::has_nested_nulls(source_table)
is always true (short-circuit), making the else branch that calls
set_all_valid_null_masks unreachable.

This PR introduces a separate variable has_possible_nulls to correctly
distinguish between:

1. Source table has actual nulls (or NULLIFY policy) → call gather_bitmask
2. Source table columns are nullable but contain no nulls → call set_all_valid_null_masks

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[davidwendt]

Would it be possible to add a test for this?

[eternallyproud]

Added a test that exercises the set_all_valid_null_masks path (nullable columns with no actual nulls + DONT_CHECK policy). Note that this bug doesn't affect correctness — gather_bitmask produces the same result in this case — but the fix avoids unnecessary bitmask computation by taking the lighter set_all_valid_null_masks path when possible.

[wence-]

I think the change to the logic looks good, but I have a question about whether this whole function is doing more work than it strictly needs to.

[wence-]

I think this change to the logic makes sense, but I am somewhat confused by it (and partially the original logic).

If we are to nullify out of bounds map indices, I can see that we need a new bitmask for each column.

Similarly, if a column has nulls (i.e. its null count is greater than zero) then we need to gather the null mask appropriately.

However, if we are not nullifying out of bounds indices and no column has nulls, why do we care if that column has a null mask (which is what cudf::has_nested_nullable_columns is checking). The gathered columns will also have no nulls, so it seems like we can get away with not even creating null masks for them.

IOW, why is this entire logic not:

auto const needs_new_bitmask = bounds_policy == NULLIFY || cudf::has_nested_nulls(source_table);
if (needs_new_bitmask) {
    gather_bitmask(...);
}
// no need to set valid null masks at all, by construction the result cannot have any nulls.