Back out major API change fix to query builder type promotion

Author: poodlewars

cpp/arcticdb/processing/operation_types.hpp

@@ -139,10 +139,11 @@ struct type_arithmetic_promoted_type {
                     double,
                     float
                 >,
-                // Otherwise, if only one type is floating point, always promote to double
-                // For example when combining int32 and float32 the result can only fit in float64 without loss of precision
-                // Special cases like int16 and float32 can fit in float32, but we always promote up to float64 (as does Pandas)
-                double
+                // Otherwise, if only one type is floating point, promote to this type
+                std::conditional_t<std::is_floating_point_v<LHS>,
+                    LHS,
+                    RHS
+                >
             >,
             // Otherwise, both types are integers
             std::conditional_t<std::is_unsigned_v<LHS> && std::is_unsigned_v<RHS>,

cpp/arcticdb/processing/test/test_arithmetic_type_promotion.cpp

@@ -133,25 +133,25 @@ TEST(ArithmeticTypePromotion, Plus) {
     static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, uint32_t, PlusOperator>::type, int64_t>);
     static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, uint64_t, PlusOperator>::type, int64_t>);
     // Mixed integral and floating point types should promote to the floating point type
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  float, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, float, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint32_t, float, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint64_t, float, PlusOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint8_t,  PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint16_t, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint32_t, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint64_t, PlusOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int8_t,  float, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int16_t, float, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int32_t, float, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, float, PlusOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int8_t,  PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int16_t, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int32_t, PlusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int64_t, PlusOperator>::type, double>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  float, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, float, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint32_t, float, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint64_t, float, PlusOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint8_t,  PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint16_t, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint32_t, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint64_t, PlusOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int8_t,  float, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int16_t, float, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int32_t, float, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, float, PlusOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int8_t,  PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int16_t, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int32_t, PlusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int64_t, PlusOperator>::type, float>);
 
     static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  double, PlusOperator>::type, double>);
     static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, double, PlusOperator>::type, double>);
@@ -262,25 +262,25 @@ TEST(ArithmeticTypePromotion, Minus) {
     static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, uint32_t, MinusOperator>::type, int64_t>);
     static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, uint64_t, MinusOperator>::type, int64_t>);
     // Mixed integral and floating point types should promote to the floating point type
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  float, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, float, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint32_t, float, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint64_t, float, MinusOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint8_t,  MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint16_t, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint32_t, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint64_t, MinusOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int8_t,  float, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int16_t, float, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int32_t, float, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, float, MinusOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int8_t,  MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int16_t, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int32_t, MinusOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int64_t, MinusOperator>::type, double>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  float, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, float, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint32_t, float, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint64_t, float, MinusOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint8_t,  MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint16_t, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint32_t, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint64_t, MinusOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int8_t,  float, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int16_t, float, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int32_t, float, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, float, MinusOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int8_t,  MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int16_t, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int32_t, MinusOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int64_t, MinusOperator>::type, float>);
 
     static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  double, MinusOperator>::type, double>);
     static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, double, MinusOperator>::type, double>);
@@ -391,25 +391,25 @@ TEST(ArithmeticTypePromotion, Times) {
     static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, uint32_t, TimesOperator>::type, int64_t>);
     static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, uint64_t, TimesOperator>::type, int64_t>);
     // Mixed integral and floating point types should promote to the floating point type
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  float, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, float, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint32_t, float, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint64_t, float, TimesOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint8_t,  TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint16_t, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint32_t, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint64_t, TimesOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int8_t,  float, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int16_t, float, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int32_t, float, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, float, TimesOperator>::type, double>);
-
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int8_t,  TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int16_t, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int32_t, TimesOperator>::type, double>);
-    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int64_t, TimesOperator>::type, double>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  float, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, float, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint32_t, float, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<uint64_t, float, TimesOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint8_t,  TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint16_t, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint32_t, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, uint64_t, TimesOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int8_t,  float, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int16_t, float, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int32_t, float, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<int64_t, float, TimesOperator>::type, float>);
+
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int8_t,  TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int16_t, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int32_t, TimesOperator>::type, float>);
+    static_assert(std::is_same_v<type_arithmetic_promoted_type<float, int64_t, TimesOperator>::type, float>);
 
     static_assert(std::is_same_v<type_arithmetic_promoted_type<uint8_t,  double, TimesOperator>::type, double>);
     static_assert(std::is_same_v<type_arithmetic_promoted_type<uint16_t, double, TimesOperator>::type, double>);

python/tests/unit/arcticdb/version_store/test_column_type_changes.py

@@ -269,10 +269,12 @@ def test_querybuilder_project_int_gt_32_float(lmdb_version_store_tiny_segment, i
     received.sort_index(inplace=True)
     expected = df
     expected["new_col"] = expected["col1"] + expected["col2"]
-    # We have to promote to float64 even with 32 bit ints to avoid losing precision of the int column
+    # We should promote to float64 even with 32 bit ints to avoid losing precision of the int column
+    # but that will need a major release (ArcticDB 6.0.0)
+    # Monday: 8604919374
     assert expected.dtypes["new_col"] == np.float64
-    assert received.dtypes["new_col"] == np.float64
-    assert_frame_equal(expected, received)
+    assert received.dtypes["new_col"] == float_type
+    assert_frame_equal(expected, received, check_dtype=False)
 
 
 @pytest.mark.parametrize("flip_addition", [True, False])
@@ -303,10 +305,12 @@ def test_querybuilder_project_int32_float32_boundary(lmdb_version_store_tiny_seg
     received.sort_index(inplace=True)
     expected = df
     expected["new_col"] = expected["col1"] + expected["col2"]
-    # We have to promote to float64 even with 32 bit ints to avoid losing precision of the int column
+    # We should promote to float64 even with int32 to avoid losing precision of the int column
+    # but that will need a major release (ArcticDB 6.0.0)
+    # Monday: 8604919374
     assert expected.dtypes["new_col"] == np.float64
-    assert received.dtypes["new_col"] == np.float64
-    assert_frame_equal(expected, received)
+    assert received.dtypes["new_col"] == np.float32
+    assert_frame_equal(expected, received, check_dtype=False)
 
 
 @pytest.mark.parametrize("integral_type", [np.int8, np.int16, np.uint8, np.uint16])
@@ -332,8 +336,8 @@ def test_querybuilder_project_int_lt_16_float(lmdb_version_store_tiny_segment, i
     # We could promote up to float32 without losing precision of these int types but we go all the way up to float64
     # to match Pandas.
     assert expected.dtypes["new_col"] == np.float64
-    assert received.dtypes["new_col"] == np.float64
-    assert_frame_equal(expected, received)
+    assert received.dtypes["new_col"] == float_type
+    assert_frame_equal(expected, received, check_dtype=False)
 
 
 @pytest.mark.parametrize("original_type", [np.int16, np.uint16, np.int32, np.uint32, np.int64, np.uint64])