Doing this check inside the convert_to_smaller_scale_decimal and only by-pass the validation part?
In that case conversion logic can be in one place.

Thanks for the valuable feedback! Originally I thought we can only apply this to same type casts, but I think this can also be applied between decimal types since we're checking the "validity" condition based on the output precision, so from what I can tell we should be able to merge the functions. 👍

same here, only bypassing the validation based on this check inside convert_to_bigger_or_equal_scale_decimal?

nit: input_prec can be first param and then input_scale, for readability. Decimal(precision, scale)

spelling: s/reudction/reduction
Also, can you add the example from the PR into this comment here. It makes things absolutely clear.

Have we already checked at this stage that the output precision is less than or equal to the max allowed for decimal 128?

Good catch, I didn't find any place where this is validated before. We can easily add a check at the top of the function to handle these case 👍

I've looked into this a little bit more, and it seems that the precision and scale are checked when building an array using the validate_decimal_precision_and_scale function. My feeling is that the arrays should always be constructed correctly, but anyway I added a the validation at the top of the cast functions.

is this required validate_decimal_precision_and_scale? previously it wasn't there, hence confirming.

Well TBH, this mostly a safeguard for what I mentioned here: #7021 (comment)
In the previous version of the code any problem with scale or precision was caught in the kernel via validate_decimal_precision / is_valid_decimal_precision.
For the newly added code this is not the case anymore, but there could be a problem if the output_precision exceeds the maximum allowed precision of the type.
To not change the existing behavior too much, I'll probably only check this inside the is_infallible_cast branch. I guess I will need to add some handling for the is_safe parameter there too.
This would not be needed if we can make sure that at this point the output_precision is always valid for the type. I investigated that, but I'm not 100% convinced of that. Maybe you have some opinions on that?

Having the validation inside is_infallible_cast branch, sounds like a good idea, but that would mean then all 3 branches will have validation now. Is the performance benchmark is still better than previous code?

The validation inside the is_infallible_cast branch is conceptually a little different to what is going on in the other branches. We can check the validity before running the kernel. It would look something like this

if is_infallible_cast {
        let precision_check =
            validate_decimal_precision_and_scale::<O>(output_precision, output_scale);
        if precision_check.is_err() {
            if cast_options.safe {
                return Ok(array.unary_opt(|_| None));
            }
            precision_check?
        }
        let g = |x: I::Native| f(x).unwrap(); // unwrapping is safe since the result is guaranteed
                                              // to fit into the target type
        array.unary(g)

so the performance on the benchmarks is not affected. It might be affected if the output_precision / output_scale are invalid, but even in that case I think it will be faster (though I didn't run a benchmark yet).

The main problem is really that it is potentially possible to call the functions convert_to_smaller_scale_decimal or convert_to_bigger_or_equal_scale_decimal with an invalid output_precision (e.g. precision 39 for Decimal128Type).
This would be an example:

 convert_to_smaller_scale_decimal::<Decimal256Type, Decimal128Type>(
                &array,
                50, // input scale: valid
                38, // input_precision: valid
                39, // output_precision: invalid, DECIMAL128_MAX_PRECISION = 38
                37, // output_scale: valid
                &CastOptions {
                    safe: true,
                    ..Default::default()
                },
            );

In the current version on the main branch this would return an array of all nulls, since the output_precision is invalid for every entry in the array.
From my POV there's already a precondition that has failed, when you try to convert to a decimal type with a precision outside of the allowed range, but to stay consistent with the current behaviour on the main branch, I think we need some special handling in the is_infallible_cast branch.

I have just realized that there is a precision check at the bottom of the decimal casting functions (here and here, via this). My suggestion would be to remove pub(crate) from convert_to_smaller_scale_decimal or convert_to_bigger_or_equal_scale_decimal, add another test case for invalid output_precision for the conversion functions and not add an additional layer of validations on top.
@parthchandra since you originally raised the concern about the validation, do you have an opinion on that?

I don't have a strong opinion on this. I agree with your previous observation that the array should have been already validated at the time of building so maybe it is unnecessary to add the additional validation.

After all the discussion I've decided to keep the validation and to put it into the is_infallible_branch. To me it doesn't seem to make sense to run the kernel if the result can not succeed. Given that the same would happen if called through the cast_decimal_to_decimal function, I think we don't need any special handling for CastOptions.safe = true.

may be add a comment like above, unwrapping is safe as the result will fit

Can we have one test for input scale = 0 and output_scale = 0 with any precision.?