IMPALA-5019: Decimal V2 addition

In this patch, we implement the new decimal return type rules for addition expressions. These rules become active when the query option DECIMAL_V2 is enabled. The algorithm for determining the type of the result is described in the JIRA. DECIMAL V1: +----------------------------------------------------------------+ | typeof(cast(1 as decimal(38,0)) + cast(0.1 as decimal(38,38))) | +----------------------------------------------------------------+ | DECIMAL(38,38) | +----------------------------------------------------------------+ DECIMAL V2: +----------------------------------------------------------------+ | typeof(cast(1 as decimal(38,0)) + cast(0.1 as decimal(38,38))) | +----------------------------------------------------------------+ | DECIMAL(38,6) | +----------------------------------------------------------------+ This patch required backend changes. We implement an algorithm where we handle the whole and fractional parts separately, and then combine them to get the final result. This is more complex and slower. We try to avoid this by first checking if the result would fit into int128. Testing: - Added expr tests. - Tested locally on my machine with a script that generates random decimal numbers and checks that Impala adds them correctly. Performance: For the common case, performance remains the same. select cast(2.2 as decimal(18, 1) + cast(2.2 as decimal(18, 1) BEFORE: 4.74s AFTER: 4.73s In this case, we check if it is necessary to do the complex addition, and it turns out to be not necessary. We see a slowdown because the result needs to be scaled down by dividing. select cast(2.2 as decimal(38, 19) + cast(2.2 as decimal(38, 19) BEFORE: 1.63s AFTER: 13.57s In following case, we take the most complex path and see the most signification perfmance hit. select cast(7.5 as decimal(38,37)) + cast(2.2 as decimal(38,37)) BEFORE: 1.63s AFTER: 20.57 Change-Id: I401049c56d910eb1546a178c909c923b01239336 Reviewed-on: http://gerrit.cloudera.org:8080/8309 Reviewed-by: Taras Bobrovytsky <[email protected]> Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public Jenkins
joemcdonnell · Nov 21, 2017 · 38758cd · 38758cd
1 parent 7bdc2fd
commit 38758cd
Show file tree

Hide file tree

Showing 5 changed files with 386 additions and 76 deletions.
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
@@ -1564,12 +1564,164 @@ DecimalTestCase decimal_cases[] = {
   // Test add/subtract operators
   { "1.23 + cast(1 as decimal(4,3))", {{ false, false, 2230, 5, 3 }}},
   { "1.23 - cast(0.23 as decimal(10,3))", {{ false, false, 1000, 11, 3 }}},
-  { "cast(1.23 as decimal(8,2)) + cast(1 as decimal(20,3))",
-    {{ false, false, 2230, 21, 3 }}},
-  { "cast(1.23 as decimal(30,2)) - cast(1 as decimal(4,3))",
-    {{ false, false, 230, 32, 3 }}},
-  { "cast(1 as decimal(38,0)) + cast(.2 as decimal(38,1))",
-    {{ false, false, 12, 38, 1 }}},
+  { "cast(1.23 as decimal(8,2)) + cast(1 as decimal(20,3))", {{ false, false, 2230, 21, 3 }}},
+  { "cast(1.23 as decimal(30,2)) - cast(1 as decimal(4,3))", {{ false, false, 230, 32, 3 }}},
+  { "cast(1 as decimal(38,0)) + cast(0.2 as decimal(38,1))", {{ false, false, 12, 38, 1 }}},
+  { "cast(100000000000000000000000000000000 as decimal(38,0)) + cast(0 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, true, 0, 38, 6 }}},
+  { "cast(-100000000000000000000000000000000 as decimal(38,0)) + cast(0 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, true, 0, 38, 6 }}},
+  { "cast(99999999999999999999999999999999 as decimal(38,0)) + cast(0 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, false, StringToInt128("99999999999999999999999999999999000000"), 38, 6 }}},
+  { "cast(-99999999999999999999999999999999 as decimal(38,0)) + cast(0 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, false, StringToInt128("-99999999999999999999999999999999000000"), 38, 6 }}},
+  { "cast(99999999999999999999999999.9999999999 as decimal(36,10)) + "
+    "cast(99999999999999999999999999.9999999999 as decimal(36,10))",
+    {{ false, false, StringToInt128("1999999999999999999999999999999999998"), 37, 10 }}},
+  { "cast(99999999999999999999999999.9999999999 as decimal(36,10)) + "
+    "cast(-99999999999999999999999999.9999999999 as decimal(36,10))",
+    {{ false, false, 0, 37, 10 }}},
+  { "cast(999999999999999999999999999.9999999999 as decimal(37,10)) + "
+    "cast(999999999999999999999999999.9999999999 as decimal(37,10))",
+    {{ false, false, StringToInt128("19999999999999999999999999999999999998"), 38, 10 }}},
+  { "cast(999999999999999999999999999.9999999999 as decimal(37,10)) + "
+    "cast(-999999999999999999999999999.9999999999 as decimal(37,10))",
+    {{ false, false, 0, 38, 10 }}},
+  // Largest simple case where we don't call AddLarge() or AubtractLarge().
+  // We are adding (2^125 - 1) + (2^125 - 1) here.
+  { "cast(42535295865117307932921825928971026431 as decimal(38,0)) + "
+    "cast(42535295865117307932921825928971026431 as decimal(38,0))",
+    {{ false, false, StringToInt128("85070591730234615865843651857942052862"), 38, 0 }}},
+  { "cast(-42535295865117307932921825928971026431 as decimal(38,0)) + "
+    "cast(-42535295865117307932921825928971026431 as decimal(38,0))",
+    {{ false, false, StringToInt128("-85070591730234615865843651857942052862"), 38, 0 }}},
+  { "cast(42535295865117307932921825928971026431 as decimal(38,0)) + "
+    "cast(-42535295865117307932921825928971026431 as decimal(38,0))",
+    {{ false, false, 0, 38, 0 }}},
+  // Smallest case where we call AddLarge(). We are adding (2^125) + (2^125 - 1) here.
+  { "cast(42535295865117307932921825928971026432 as decimal(38,0)) + "
+    "cast(42535295865117307932921825928971026431 as decimal(38,0))",
+    {{ false, false, StringToInt128("85070591730234615865843651857942052863"), 38, 0 }}},
+  { "cast(-42535295865117307932921825928971026432 as decimal(38,0)) + "
+    "cast(-42535295865117307932921825928971026431 as decimal(38,0))",
+    {{ false, false, StringToInt128("-85070591730234615865843651857942052863"), 38, 0 }}},
+  { "cast(42535295865117307932921825928971026432 as decimal(38,0)) + "
+    "cast(-42535295865117307932921825928971026431 as decimal(38,0))",
+    {{ false, false, StringToInt128("1"), 38, 0 }}},
+  { "cast(99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(8899999999999999999999999999999.999999 as decimal(38,6))",
+    {{ false, true, 0, 38, 6 }}},
+  { "cast(-99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(-8899999999999999999999999999999.999999 as decimal(38,6))",
+    {{ false, true, 0, 38, 6 }}},
+  { "cast(-99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(8899999999999999999999999999999.999999 as decimal(38,6))",
+    {{ false, false, StringToInt128("-91100000000000000000000000000000000000"), 38, 6 }}},
+  // Close to the maximum value.
+  { "cast(77777777777777777777777777777777.777777 as decimal(38,6)) + "
+    "cast(22222222222222222222222222222222.222222 as decimal(38,6))",
+    {{ false, false, StringToInt128("99999999999999999999999999999999999999"), 38, 6 }}},
+  { "cast(-77777777777777777777777777777777.777777 as decimal(38,6)) + "
+    "cast(22222222222222222222222222222222.222222 as decimal(38,6))",
+    {{ false, false, StringToInt128("-55555555555555555555555555555555555555"), 38, 6 }}},
+  { "cast(-77777777777777777777777777777777.777777 as decimal(38,6)) + "
+    "cast(-22222222222222222222222222222222.222222 as decimal(38,6))",
+    {{ false, false, StringToInt128("-99999999999999999999999999999999999999"), 38, 6 }}},
+  // Smallest result that overflows.
+  { "cast(77777777777777777777777777777777.777777 as decimal(38,6)) + "
+    "cast(22222222222222222222222222222222.222223 as decimal(38,6))",
+    {{ false, true, 0, 38, 6 }}},
+  { "cast(-77777777777777777777777777777777.777777 as decimal(38,6)) + "
+    "cast(-22222222222222222222222222222222.222223 as decimal(38,6))",
+    {{ false, true, 0, 38, 6 }}},
+  { "cast(11111111111111111111111111111111.777777 as decimal(38,6)) + "
+    "cast(11111111111111111111111111111111.555555 as decimal(38,6))",
+    {{ false, false, StringToInt128("22222222222222222222222222222223333332"), 38, 6 }}},
+  { "cast(-11111111111111111111111111111111.777777 as decimal(38,6)) + "
+    "cast(11111111111111111111111111111111.555555 as decimal(38,6))",
+    {{ false, false, StringToInt128("-222222"), 38, 6 }}},
+  { "cast(11111111111111111111111111111111.777777 as decimal(38,6)) + "
+    "cast(-11111111111111111111111111111111.555555 as decimal(38,6))",
+    {{ false, false, StringToInt128("222222"), 38, 6 }}},
+  { "cast(-11111111111111111111111111111111.777777 as decimal(38,6)) + "
+    "cast(-11111111111111111111111111111111.555555 as decimal(38,6))",
+    {{ false, false, StringToInt128("-22222222222222222222222222222223333332"), 38, 6 }}},
+  { "cast(3333333333333333333333333333333.8888884 as decimal(38,7)) + "
+    "cast(3333333333333333333333333333333.1111111 as decimal(38,7))",
+    {{ false, false, StringToInt128("66666666666666666666666666666669999995"), 38, 7 },
+     { false, false, StringToInt128("6666666666666666666666666666667000000"), 38, 6 }}},
+  { "cast(-3333333333333333333333333333333.8888884 as decimal(38,7)) + "
+    "cast(-3333333333333333333333333333333.1111111 as decimal(38,7))",
+    {{ false, false, StringToInt128("-66666666666666666666666666666669999995"), 38, 7 },
+     { false, false, StringToInt128("-6666666666666666666666666666667000000"), 38, 6 }}},
+  { "cast(3333333333333333333333333333333.9999995 as decimal(38,7)) + "
+    "cast(0 as decimal(38,7))",
+    {{ false, false, StringToInt128("33333333333333333333333333333339999995"), 38, 7 },
+     { false, false, StringToInt128("3333333333333333333333333333334000000"), 38, 6 }}},
+  { "cast(-3333333333333333333333333333333.9999995 as decimal(38,7)) + "
+    "cast(0 as decimal(38,7))",
+    {{ false, false, StringToInt128("-33333333333333333333333333333339999995"), 38, 7 },
+     { false, false, StringToInt128("-3333333333333333333333333333334000000"), 38, 6 }}},
+  // overflow due to rounding
+  { "cast(99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(0.0000005 as decimal(38,7))",
+    {{ false, true, 0, 38, 7 },
+     { false, true, 0, 38, 6 }}},
+  { "cast(-99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(-0.0000005 as decimal(38,7))",
+    {{ false, true, 0, 38, 7 },
+     { false, true, 0, 38, 6 }}},
+  { "cast(99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(-0.0000006 as decimal(38,7))",
+    {{ false, true, 0, 38, 7 },
+     { false, false, StringToInt128("99999999999999999999999999999999999998"), 38, 6 }}},
+  { "cast(99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(0.0000004 as decimal(38,7))",
+    {{ false, true, 0, 38, 7 },
+     { false, false, StringToInt128("99999999999999999999999999999999999999"), 38, 6 }}},
+  { "cast(-99999999999999999999999999999999.999999 as decimal(38,6)) + "
+    "cast(-0.0000004 as decimal(38,7))",
+    {{ false, true, 0, 38, 7 },
+     { false, false, StringToInt128("-99999999999999999999999999999999999999"), 38, 6 }}},
+  { "cast(99999999999999999999999999999999 as decimal(38,0)) + "
+    "cast(0.9999994 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, false, StringToInt128("99999999999999999999999999999999999999"), 38, 6 }}},
+  { "cast(-99999999999999999999999999999999 as decimal(38,0)) + "
+    "cast(-0.9999994 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, false, StringToInt128("-99999999999999999999999999999999999999"), 38, 6 }}},
+  { "cast(99999999999999999999999999999999 as decimal(38,0)) + "
+    "cast(0.9999995 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, true, 0, 38, 6 }}},
+  { "cast(-99999999999999999999999999999999 as decimal(38,0)) + "
+    "cast(-0.9999995 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, true, 0, 38, 6 }}},
+  { "cast(0.99999999999999999999999999999999999999 as decimal(38,38)) + "
+    "cast(-5e-38 as decimal(38,38))",
+    {{ false, false, StringToInt128("99999999999999999999999999999999999994"), 38, 38 },
+     { false, false, StringToInt128("9999999999999999999999999999999999999"), 38, 37 }}},
+  { "cast(0.99999999999999999999999999999999999999 as decimal(38,38)) + "
+    "cast(-4e-38 as decimal(38,38))",
+    {{ false, false, StringToInt128("99999999999999999999999999999999999995"), 38, 38 },
+     { false, false, StringToInt128("10000000000000000000000000000000000000"), 38, 37 }}},
+  { "cast(0.99999999999999999999999999999999999999 as decimal(38,38)) + "
+    "cast(0.99999999999999999999999999999999999999 as decimal(38,38))",
+    {{ false, true, 0, 38, 38 },
+     { false, false, StringToInt128("20000000000000000000000000000000000000"), 38, 37 }}},
+  { "cast(-0.99999999999999999999999999999999999999 as decimal(38,38)) + "
+    "cast(0.99999999999999999999999999999999999999 as decimal(38,38))",
+    {{ false, false, StringToInt128("0"), 38, 38 },
+     { false, false, StringToInt128("0"), 38, 37 }}},
+  { "cast(0 as decimal(38,38)) + cast(0 as decimal(38,38))",
+    {{ false, false, StringToInt128("0"), 38, 38 },
+     { false, false, StringToInt128("0"), 38, 37 }}},
   // Test multiply operator
   { "cast(1.23 as decimal(30,2)) * cast(1 as decimal(10,3))",
     {{ false, false, 123000, 38, 5 }}},

diff --git a/be/src/runtime/decimal-value.h b/be/src/runtime/decimal-value.h
@@ -195,9 +195,9 @@ class DecimalValue {
  private:
   T value_;
 
-  /// Returns in *x_val and *y_val, the adjusted values so that both
-  /// are at the same scale. The scale is the number of digits after the decimal.
-  /// Returns true if the adjusted causes overflow in which case the values in
+  /// Returns in *x_val and *y_val, the adjusted values so that both are at
+  /// max(x_scale, y_scale) scale. The scale is the number of digits after the decimal.
+  /// Returns true if the adjustment causes overflow in which case the values in
   /// x_scaled and y_scaled are unmodified.
   template <typename RESULT_T>
   static inline bool AdjustToSameScale(const DecimalValue& x, int x_scale,