ClassCastException in testArrayCumSum: Double cannot be cast to Float

[athmaja-n]

Bug Description

The testArrayCumSum test in the presto-native-tests module fails with a ClassCastException due to type mismatch between Double and Float. The test expects Float values (corresponding to the SQL REAL type), but the array_cum_sum function returns Double values (corresponding to the SQL DOUBLE type) instead, causing the exception.

Test Case Location:

File: presto-native-execution/presto-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

Test Method: testArrayCumSum

Code Snippet (from AbstractTestQueriesNative.java):

sql = "select array_cum_sum(k) from (values (array[cast(5.1 as real), 6.1, 0.5]), (ARRAY[]), (CAST(NULL AS array(real))), (ARRAY [cast(null as real), 6.1, 0.5]), (ARRAY [cast(2.5 as real), 6.1, null, 3.2])) t(k)";
MaterializedResult raw = computeActual(sql);
List<MaterializedRow> rowList = raw.getMaterializedRows();
List<Float> actualFloat = (List<Float>) rowList.get(0).getField(0);  // Fails here due to Double-to-Float cast

Error Message:

java.lang.ClassCastException: class java.lang.Double cannot be cast to class java.lang.Float (java.lang.Double and java.lang.Float are in module java.base of loader 'bootstrap')

Expected Behavior

The array_cum_sum function should return cumulative sums for REAL values as Float. The test should pass without encountering any ClassCastException.

Current Behavior

The array_cum_sum function promotes REAL (floating-point Float in Java) values to DOUBLE during execution, leading to a ClassCastException when the test expects Float results.
Error Message:
java.lang.ClassCastException: class java.lang.Double cannot be cast to class java.lang.Float

Possible Solution

Ensure that array_cum_sum maintains the type consistency and does not promote REAL values to DOUBLE unless explicitly requested.

[zuyu]

@pramodsatya Regarding #23671, I do NOT think velox has a bug here.

Prestissimo result:

presto> select array_cum_sum(k) from (values (array[cast(5.1 as real), 6.1, 0.5]), (ARRAY[]), (CAST(NULL AS array(real))), (ARRAY [cast(null as real), 6.1, 0.5]), (ARRAY [cast(2.5 as real), 6.1, null, 3.2])) t(k);
                            _col0
-------------------------------------------------------------
 [5.099999904632568, 11.199999904632568, 11.699999904632568]
 []
 NULL
 [null, null, null]
 [2.5, 8.6, null, null]
(5 rows)

Presto result:

         _col0
------------------------
 [5.1, 11.2, 11.7]
 []
 NULL
 [null, null, null]
 [2.5, 8.6, null, null]
(5 rows)

[yingsu00]

@zuyu Does Presto work with the example @athmaja-n provided? What type was the output? Presto outputs double if the input type is double. See com/facebook/presto/operator/scalar/ArrayCumSum.java, which is calling

public static void writeNativeValue(Type type, BlockBuilder blockBuilder, Object value)
    {
        if (value == null) {
            blockBuilder.appendNull();
        }
        else if (type.getJavaType() == boolean.class) {
            type.writeBoolean(blockBuilder, (Boolean) value);
        }
        else if (type.getJavaType() == double.class) {
            type.writeDouble(blockBuilder, ((Number) value).doubleValue());
        }
        else if (type.getJavaType() == long.class) {
            type.writeLong(blockBuilder, ((Number) value).longValue());
        }
        else if (type.getJavaType() == Slice.class) {
            Slice slice;
            if (value instanceof byte[]) {
                slice = Slices.wrappedBuffer((byte[]) value);
            }
            else if (value instanceof String) {
                slice = Slices.utf8Slice((String) value);
            }
            else {
                slice = (Slice) value;
            }
            type.writeSlice(blockBuilder, slice, 0, slice.length());
        }
        else {
            type.writeObject(blockBuilder, value);
        }
    }

Can you check what the output type is if the input is float? If Presto output float instead of double, then it's better to make PrestoC++ behave the same. Otherwise, I'd say we can just update the test to expect doubles.

[pramodsatya]

Thanks for the update @zuyu.
@yingsu00, the Presto C++ function implementation should be returning array[float] for array[float] inputs, based on the function registration here. I suspect it is an issue with the testcase as you pointed out.

[zuyu]

This is NOT a bug. Prestissimo by default parses a decimal as a double. This could be turn off by set session parse_decimal_literals_as_double = false;

presto> explain select array_cum_sum(k) from (values (array[cast(5.1 as real), 6.1, 0.5]), (ARRAY[]), (CAST(NULL AS array(real))), (ARRAY [cast(null as real), 6.1, 0.5]), (ARRAY [cast(2.5 as real), 6.1, null, 3.2])) t(k);
                                                         Query Plan
----------------------------------------------------------------------------------------------------------------------
 - Output[PlanNodeId 8][_col0] => [array_cum_sum:array(double)]
         Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 460.00, memory: 0.00, network: 0.00}
         _col0 := array_cum_sum (1:16)
     - Project[PlanNodeId 4][projectLocality = LOCAL] => [array_cum_sum:array(double)]
             Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 460.00, memory: 0.00, network: 0.00}
             array_cum_sum := array_cum_sum(field) (1:40)
         - LocalExchange[PlanNodeId 198][ROUND_ROBIN] () => [field:array(double)]
                 Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 205.00, memory: 0.00, network: 0.00}
             - Values[PlanNodeId 0] => [field:array(double)]
                     Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 0.00, memory: 0.00, network: 0.00}
                     ([Block: position count: 3; size: 80 bytes])
                     ([Block: position count: 0; size: 105 bytes])
                     (null)
                     ([Block: position count: 3; size: 99 bytes])
                     ([Block: position count: 4; size: 108 bytes])
(1 row)

presto> set session parse_decimal_literals_as_double = false;
SET SESSION

presto> explain select array_cum_sum(k) from (values (array[cast(5.1 as real), 6.1, 0.5]), (ARRAY[]), (CAST(NULL AS array(real))), (ARRAY [cast(null as real), 6.1, 0.5]), (ARRAY [cast(2.5 as real), 6.1, null, 3.2])) t(k);
                                                         Query Plan
----------------------------------------------------------------------------------------------------------------------
 - Output[PlanNodeId 8][_col0] => [array_cum_sum:array(real)]
         Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 460.00, memory: 0.00, network: 0.00}
         _col0 := array_cum_sum (1:16)
     - Project[PlanNodeId 4][projectLocality = LOCAL] => [array_cum_sum:array(real)]
             Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 460.00, memory: 0.00, network: 0.00}
             array_cum_sum := array_cum_sum(field) (1:40)
         - LocalExchange[PlanNodeId 198][ROUND_ROBIN] () => [field:array(real)]
                 Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 205.00, memory: 0.00, network: 0.00}
             - Values[PlanNodeId 0] => [field:array(real)]
                     Estimates: {source: CostBasedSourceInfo, rows: 5 (255B), cpu: 0.00, memory: 0.00, network: 0.00}
                     ([Block: position count: 3; size: 68 bytes])
                     ([Block: position count: 0; size: 101 bytes])
                     (null)
                     ([Block: position count: 3; size: 87 bytes])
                     ([Block: position count: 4; size: 92 bytes])
(1 row)

[ethanyzhang]

@pramodsatya what do we do for this test case?

[pramodsatya]

@zuyu, thanks for the update.

@ethanyzhang, I will disable the parse-decimal-literals-as-double flag and check if this test passes. I feel we should set this flag to false in the native query runner (by passing it as an argument) only for this test. Keeping parse-decimal-literals-as-double=true as the default config would be better as other e2e tests require it to be set to true.
We should remove this config as in #24065 eventually but I am not sure if it is feasible at this point, would need to check exactly how many existing e2e tests would be affected by removing this config. @zuyu, what do you think?