prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
16k stars 5.36k forks source link

Decimal multiplication fails with DECIMAL scale must be in range [0, precision] #22383

Open mbasmanova opened 6 months ago

mbasmanova commented 6 months ago

Looks like there is a bug in how result type for decimal multiplication is calculated.

presto:di> select cast(1.2 as decimal(38, 30)) * cast(1.2 as decimal(38, 30));
Query 20240401_120207_79326_2jn6c failed: DECIMAL scale must be in range [0, precision]

See https://github.com/facebookincubator/velox/pull/9286 CC: @rui-mo @majetideepak @tdcmeehan

tdcmeehan commented 6 months ago

This works fine in Postgres and MySql:

select cast(1.2 as decimal(38, 30)) * cast(1.2 as decimal(38, 30));

MySQL (scale capped to 30): 1.440000000000000000000000000000

Postgres (scale is summed to 60): 1.440000000000000000000000000000000000000000000000000000000000

It fails in Presto because we're summing the scale, and as evidenced by the error message, we don't permit the scale to be greater than the precision, which remains at 38.

tdcmeehan commented 6 months ago

I'm wondering if this falls into an area of the spec that is ambiguous and open for different implementations across engines.

The error above is due to Presto following this behavior:

23) If an <exact numeric type> contains <scale>, then the value of <scale> shall not be greater than the value of <precision> contained in that <exact numeric type>.

So it's really a question of what to do with the scale when we multiply. I believe our behavior of summing them is also conformant to the spec:

iii) The precision of the result of multiplication is implementation-defined, and the scale is S1 +
S2.

In Postgres, it looks like it's just summing the scale, and I guess the precision is just bumped up to accommodate that. Mysql's results don't appear to be spec compliant. Presto's appear to be spec compliant.

It also appears Presto is spec-compliant, but perhaps the error message could be improved. Basically, Presto has documented that the max precision is 38--that's also OK as that's implementation-defined in the spec. What happens here is 30+30 exceeds Presto's maximum overall precision (it's happily accommodated at lower precision and scale factor).

mbasmanova commented 6 months ago

@tdcmeehan Tim, thank you for looking into this issue. It seems like multiplication of decimals is supported only when sum of scales doesn't exceed max precision (38) (query preparation/compilation time error). Otherwise, the operation is not supported. Similarly, the operation is not supported if the actual result of multiplication doesn't fit into max precision (runtime error). This sounds like a reasonable limitation. Like you mentioned above, it would be nice to improve error message. Does Presto document this limitation somewhere?

CC: @steveburnett

tdcmeehan commented 6 months ago

We document it implicitly through the note on precision having an upper bound of 38, but it would be nice to include real examples that cite the spec behavior, and then also point out how high scales can run into this limitation on precision. We can add this to the documentation backlog, it would be nice to make this behavior clear to users, as other databases document this much more clearly.

mbasmanova commented 6 months ago

I'm trying to understand the rational for decimal implementation in Presto. I wrote down my thinking below and would appreciate any clarification or correction. In particular, I do not quite understand the reasoning for the rules for division. Rules for multiplication seem a bit inconsistent with the rules for Addition and Subtraction.


Decimal type is designed to represent floating point numbers precisely. Mathematical operations on decimal values are exact, except for division. On the other hand, DOUBLE and REAL types are designed to represent floating point numbers approximately. Mathematical operations on double and real values are approximate.

Decimal type has 2 parameters: precision and scale. Precision is the total number of digits used to represent the number. Scale is the number of digits after the decimal point. Naturally, scale must not exceed precision. In addition, precision cannot exceed 38.

decimal(p, s)

p >= 1 && p <= 38
s >= 0 && s <= p

Addition and Subtraction

To represent the results of adding two decimal numbers we need to use max(s1, s2) digits after the decimal point and max(p1 - s1, p2 - s2) + 1 digits before the decimal point.

p = max(p1 - s1, p2 - s2) + 1 + max(s1, s2)
s = max(s1, s2)

It is easiest to understand this formula by thinking about column addition where we place two numbers one under the other and line up decimal points.

        1.001
     9999.5
-----------------
    10000.501

We can see that the result needs max(s1, s2) digits after the decimal point and max(p1 - s1, p2 - s2) + 1 digits before the decimal point.

The precision of the result may exceed 38. There are two options. One option is to say that addition and subtraction is supported as long as p <= 38 and reject operations that produce p > 38. Another option is to cap p at 38 and allow the operation to succeed as long as the actual result can be represented using 38 digits. In this case, users will see runtime errors when the actual result cannot be represented using 38 digits. Presto implements the second option.

Multiplication

To represent the results of multiplying two decimal numbers we need s1 + s2 digits after the decimal point and p1 + p2 digits overall.

p = p1 + p2
s = s1 + s2

To multiply two numbers we can multiply them as integers ignoring the decimal points, then add up the number of digits after the decimal point in the original numbers and place the decimal point that many digits away in the result.

To multiply 0.01 with 0.001, we can multiply 1 with 1, then place the decimal point 5 digits to the left: 0.00001. Hence, the scale of the result is the sum of scales of the inputs.

When multiplying two integers with p1 and p2 digits respectively we get a result that is strictly less than 10^p1 * 10^p2 = 10^(p1 + p2). Hence, we need at most p1 + p2 digits to represent the result.

Both scale and precision of the result may exceed 38. There are two options again. One option is to say that multiplication is supported as long as p <= 38 (by definition, s does not exceed p and therefore does not exceed 38 if p <= 38). Another option is to cap p and s at 38 and allow operation to succeed as long as the actual result can be represented as a decimal(38, s). In this case, users will see runtime errors when the actual result cannot be represented as a decimal(38, s). Presto doesn’t implement either of these options. There is a 3rd option. Reject the operation if s exceeds 38 and cap p at 38 when s <= 38. Presto implements this option. In this case some operations are rejected outright while others are allowed, but may produce runtime errors.

Division

Perfect division is not possible. For example, 1 / 3 cannot be represented as a decimal value.

When dividing a number with p1 digits over a number of s2 scale, we need p1 + s2 digits for the result. In the worst case we divide by 0.0000001, which effectively is a multiplication by 10^s2.

Presto also chooses to extend the scale of the result to a maximum of scales of the inputs. It is not clear why.

s = max(s1, s2)

To support increased scale, the result precision needs to be extended by the difference in s1 and s.

p = p1 + s2 + max(0, s2 - s1)

Like in Addition, the precision of the result may exceed 38. The choices are the same. Presto chooses to cap p at 38 and allow runtime errors.

mbasmanova commented 6 months ago

I do not quite understand the reasoning for the rules for division.

Specifically, I'm seeing that 0.01 / 0.001 should be 0.00001, but in Presto it is 0.000 because the result scale is max of input scales.

tdcmeehan commented 6 months ago

@mbasmanova I presume you mean you would expect the following to result in decimals with equal precision and scale?

SELECT .001 / 10;
SELECT .001 * .1;

I don't know why we didn't, for example, choose to follow the same scale rules as multiplication, but I do note that once again we see wildly different output between the reference systems I checked against (SQLite, Postgres and MySQL), and I do note that the spec states that the both precision and scale are implementation-defined. This is a perennial source of confusion (see e.g. #21610). My naive expectation is we should be able to match the same rules for multiplication, but once again I don't know why this isn't the case presently.

If you're asking in the interest of consistency between Velox evaluation and Presto's, I wonder if you can simply allow the implementations to diverge here, as we'll eventually settle on Velox's evaluation.

mbasmanova commented 6 months ago

@tdcmeehan Tim, thank you for sharing your thoughts. I'm trying to understand how Presto works now, whether this makes sense and if not what alternative implementation would make sense. I appreciate you helping me get through this process.

My thinking is that decimal type is designed to provide precise computation. I don't know if that's the right way of thinking about it though. If that's the case, then I don't understand why we support division at all. It is not possible to implement division precisely, no matter how large precision one is willing to support (1/3 = 0.(3) requires infinite number of digits to represent accurately). At the same time, it is possible to represent the result of multiplication exactly (as long as there is enough precision). It is a bit counterintuitive that division and multiplication are different in this context.

I don't know what is the real-world use case for decimal division. Do you?

tdcmeehan commented 6 months ago

An example is in https://github.com/prestodb/presto/issues/21610: actually, any numeric literal in a SQL query by default is converted into a corresponding decimal type, so simply hardcoding a division into a query based on constant, well-known values may bump into precision issues. This is also because the spec states that such numeric literals should be interpreted as exact types--i.e., decimal types.

Now, intentional usage of decimal types, perhaps it is desirable in financial contexts where the scale can exceed that of DOUBLE. I'll note modern tables formats all seem to support decimal precision as well, where division is needed for completeness. I suppose the same question can be said of division in general, where even float and double can't represent simple fractions precisely, I suppose the flexibility of the scale could be useful to control for error in a way that is harder than for inexact floating point numbers.

mbasmanova commented 6 months ago

For now, we'll keep Velox implementation to match Presto and document it: https://github.com/facebookincubator/velox/pull/9337