Add in support for Decimal Multiply and DIV

[revans2]

Multiply probably needs a follow on issue filed to be able to fully support what cudf currently can support for multiply. The issue is around PromotePrecision and CheckOverflow. In Spark during the logical planning phase

https://github.com/apache/spark/blob/116f4cab6b05f4286a08b9f03b8ddfb48ec464cf/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/DecimalPrecision.scala#L125-L135

it will look at the two inputs to the multiply. Cast them so that the scale for both is what ever the greater of the two would be. and the part on the other side of the decimal point (what they call the range) is which ever would be larger. This means that if I multiply 1.01 precision 3 scale 2, by 10.0 precision 3 scale 1. That both of them are widened to precision 4 scale 2. 01.01 * 10.00. Then they will do the multiply. At this step spark lies about what the output type actually is. Multiply says that the output type is the same as the type of the left hand operator, but in reality spark is using java's big decimal for the multiplication so the precision is dynamic based off of what is needed to store the result and the scale is the sum of the two input scales, or 10.1000 precision 6 scale 4. In the final step CheckOverflow will force the scale and precision to be what spark wants them to be. which is the sum of the precisions of the inputs +1, and the sum of the scales of the inputs. This means it will adjust the scale of 10.1000 down to 3 (scale 2 + scale 1) 10.100 and then verify that the precision is less than 7 (precision 3 + precision 3 + 1).

In an ideal world we would just get the original inputs do the multiply, which would produce the correct result. I am not 100% sure why Spark will go through all of these extra steps and lie about the result. I do know that if the resulting precision or scale would go above what 128-bits could support then they use this to return a null on the overflow or throw an exception, and to truncate the scale if necessary.

My original plan for translating this to cudf was to look at the plan and throw out all of the PromotePrecision, CheckOverflow, and Casts because the cudf multiply will just do the right thing so long as we verify that the precision will fit in the output we expect. Sadly because of other optimizations in the logical plan before it becomes a part of the physical plan I have no good way to do this. If we are multiplying by a literal value the literal will have been turned into a single decimal literal value, and in some cases casts can disapear if they are a NOOP. So if I multiply by a literal byte (1.0) by a column of decimal(5, 2) that I happen to cast to decimal(7, 2) first

SELECT CAST(1 AS BYTE) * CAST(FOO AS DECIMAL(7, 2)) AS RESULT

the byte gets automatically cast to a decimal(3,0) and then cast again as a part of widening to a decimal(7, 2) and the final plan looks like

CheckOverflow(
  Multiply(
    Literal(00001.00, DecimalType(7,2))), 
    PromotePrecision(
      Cast(AttributeReference("foo", DecimalType(5, 2))), DecimalType(7, 2)))
  ), DecimalType(11, 2))

So I have no good way of knowing that the Literal was actually a Decimal(3, 0). I could do pattern matching and see the resulting precision and scale. I could also see the cast to a Decimal(7,2) for "foo" but I cannot tell if that cast was automatically inserted by Spark to make the Decimal(5, 2) match whatever the literal was or if it was something inserted by the user on purpose. This is just one example, but there are lots more.

Instead I opted to just follow what spark does. I let GpuCast happen as spark wants to widen the inputs and then I let cudf do the multiplication, but I have extra checks in place to be sure that we do not overflow. Finally GpuCheckOverflow adjusts the scale if needed to get back to what spark wants.

The impact of this means that some multiplication problems that cudf can support we will have to fall back to the CPU for because we don't know for sure that we could support them. CUDF can support any multiplication where the precision of both inputs + 1 is <= 18. Now it is a lot more complicated. If we want more documentation for this in the official release docs I can do that too. I suspect that for the remaining binary ops we will run into similar problems so I might hold off on the documentation until I have finished looking at them.

I might also file an issue for us to look at spark/cudf and see if there is something that we can do to avoid some of these issues.

I just added in support for DIV. It was a lot simpler because the output of spark for a DIV is always a long, so some of the complexity of multiply is not there. The main problem is that because of widening the children it still could result in some operations that we cannot do on the GPU even if we could without that step. If it becomes a customer issue we can look into it later.

[jlowe]

Minor doc edits and a question about documenting the problem in an issue, but otherwise lgtm.

[razajafri]

Have we created a bug against cudf to track this?

[revans2]

This is expected behavior right now. If we want to push on it we can, but it is likely to make the cudf binary bigger without much of a real benefit.

[razajafri]

same as above

[razajafri]

nit: :s/were/where

[revans2]

build