GH-46177: [C++][Compute] Correct the behavior of cast compute functions regarding string types

[andishgar]

Rationale for this change
There are eight compute functions for casting between string types. All, except the fixed-to-offset string function, assume the second buffer is not preallocated. However, the second buffer is preallocated when the output does not involve String/Binary view types

What changes are included in this PR?
1-I changed the way the kernel is created not to allocate Buffer for the second buffers
2- Makes the behaviour of FixedSize to String types cast compute function likes the others.

Are these changes tested?
I run the relevant unit test.

Are there any user-facing changes?
No

• GitHub Issue: [C++][Compute] Incorrect document for MemAllocation::PREALLOCATE for string types #46177

[github-actions]

⚠️ GitHub issue #46177 has been automatically assigned in GitHub to PR creator.

[bkietz]

If we're not preallocating buffers which could be preallocated, I think it'd be better to correct the other kernels to do so rather than remove an optimization from what is probably the most-used kernel

[andishgar]

If we're not preallocating buffers which could be preallocated, I think it'd be better to correct the other kernels to do so rather than remove an optimization from what is probably the most-used kernel

My changes impact 8 kernels. Based on your feedback, I understand I should rewrite the entire casting compute functions related binary to binary. Let me explain each change in detail.

1- Offset String -> Offset String
The allocated offset buffer is dropped in zero-copy casting

arrow/cpp/src/arrow/compute/kernels/scalar_cast_string.cc

Line 308 in e6d0edc

RETURN_NOT_OK(ZeroCopyCastExec(ctx, batch, out));

If the input and output offset types differ, a new buffer is allocated

arrow/cpp/src/arrow/compute/kernels/scalar_cast_string.cc

Lines 251 to 253 in e6d0edc

	ARROW_ASSIGN_OR_RAISE(
	output->buffers[1],
	ctx->Allocate((output->length + output->offset + 1) * sizeof(output_offset_type)));

arrow/cpp/src/arrow/compute/kernels/scalar_cast_string.cc

Lines 279 to 281 in e6d0edc

	ARROW_ASSIGN_OR_RAISE(output->buffers[1],
	ctx->Allocate((output->length + output->offset + 1) *
	sizeof(output_offset_type)));

2- String View -> Offset String
The implementation overlooks the allocation of a second buffer

arrow/cpp/src/arrow/compute/kernels/scalar_cast_string.cc

Line 341 in e6d0edc

OffsetBuilder offset_builder(ctx->memory_pool());

Additionally, there’s a risk of generating utf8() and binary() types with lengths exceeding 2 billion, as discussed here.

3- Offset String -> String View
This requires updates to the compute infrastructure first (issue). Additional challenges with slicing and casting are noted here.

4- String View -> String View
The code works correctly with the current logic in the compute function., however if we resolve this, MemAllocation::NO_PREALLOCATE should be used to prevent extra casting

5- Fixed -> String View
It has the same issue of Offset string-> String View

6- Fixed -> Offset string
Is it possible to have a separate path for scalar and array?

arrow/cpp/src/arrow/compute/kernels/scalar_cast_string.cc

Lines 597 to 612 in e6d0edc

	// Data buffer (index 1) for FWBinary becomes data buffer for VarBinary
	// (index 2). After ARROW-16757, we need to copy this memory instead of
	// zero-copy it because a Scalar value promoted to an ArraySpan may be
	// referencing a temporary buffer whose scope does not extend beyond the
	// kernel execution. In that scenario, the validity bitmap above can be
	// zero-copied because it points to static memory (either a byte with a 1 or
	// a 0 depending on whether the value is null or not).
	std::shared_ptr<Buffer> input_data = input.GetBuffer(1);
	if (input_data != nullptr) {
	ARROW_ASSIGN_OR_RAISE(output->buffers[2], input_data->CopySlice(0, input_data->size(),
	ctx->memory_pool()));
	} else {
	// TODO(wesm): it should already be nullptr, so we may be able to remove
	// this
	output->buffers[2] = nullptr;
	}

7-Fixed -> Fixed
The second buffer is allocated, However, there is no need

8- String| String View -> Fixed
First : The second buffer is allocated but ignored by the implementation.
Second: Why the data buffer is copied for casting from offset string to FixedSizeString

arrow/cpp/src/arrow/compute/kernels/scalar_cast_string.cc

Lines 645 to 661 in e6d0edc

	RETURN_NOT_OK(VisitArraySpanInline<I>(
	input,
	[&](std::string_view v) {
	if (v.size() != static_cast<size_t>(builder.byte_width())) {
	return Status::Invalid("Failed casting from ", input.type->ToString(), " to ",
	options.to_type.ToString(), ": widths must match");
	}
	builder.UnsafeAppend(v);
	return Status::OK();
	},
	[&] {
	builder.UnsafeAppendNull();
	return Status::OK();
	}));
	return builder.FinishInternal(&std::get<std::shared_ptr<ArrayData>>(out->value));
	}

[andishgar]

Should I applay above changes?