Zach Amsden has uploaded a new patch set (#2). Change subject: IMPALA-4813: Round on divide and multiply ......................................................................
IMPALA-4813: Round on divide and multiply Address rounding on divide and multiply when results are truncated. Testing: Manually ran some divides that should overflow, then added the results to the test. Made the decimal-test use rounding behavior by default, and now the error margin of the test has decreased. Still working on a test for multiply; this requires an actual scale overflow so generating the numbers will take a bit more work. Initial perf results: Multiply is totall uninteresting so far, all implementations return the same values in the same time: +-------------------------+-----------------------------------+ | sum(l_quantity * l_tax) | sum(l_extendedprice * l_discount) | +-------------------------+-----------------------------------+ | 61202493.3700 | 114698450836.4234 | +-------------------------+-----------------------------------+ Fetched 1 row(s) in 1.13s Divide shows no regression from prior with DECIMAL_V2 off: +-----------------------------+-----------------------------------+ | sum(l_quantity / l_tax) | sum(l_extendedprice / l_discount) | +-----------------------------+-----------------------------------+ | 46178777464.523809516381723 | 61076151920731.010714279183910 | +-----------------------------+-----------------------------------+ before: Fetched 1 row(s) in 13.08s after: Fetched 1 row(s) in 13.06s And with DECIMAL_V2 on: +-----------------------------+-----------------------------------+ | sum(l_quantity / l_tax) | sum(l_extendedprice / l_discount) | +-----------------------------+-----------------------------------+ | 46178777464.523809523847285 | 61076151920731.010714285714202 | +-----------------------------+-----------------------------------+ Fetched 1 row(s) in 16.06s So the performance regression is not as bad as expected. Still, divide performance could use some work. Change-Id: Ie6bfcbe37555b74598d409c6f84f06b0ae5c4312 --- M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/expr-test.cc M be/src/runtime/decimal-test.cc M be/src/runtime/decimal-value.h M be/src/runtime/decimal-value.inline.h 6 files changed, 104 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/6132/2 -- To view, visit http://gerrit.cloudera.org:8080/6132 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie6bfcbe37555b74598d409c6f84f06b0ae5c4312 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <zams...@cloudera.com>