Benedict commented on CASSANDRA-15232:

Thanks [~Override], I agree that's the best way forward.

The patch is almost ready to commit.  I have a couple of nits for you to 

# {{CONTEXT_WITH_MAX_PRECISION}} and {{MAX_PRECISION}} could be merged into 
simply {{MAX_PRECISION}} (as a {{MathContext}}) for brevity, since 
{{MAX_PRECISION}} isn't used independently?
# {{leftOperandFirstDigitPos}} and {{rightOperandFirstDigitPos}} both subtract 
1, which is cancelled out by the subtraction of one from the other, and are no 
longer used for any other calculation.  Perhaps rename the variables to 
something that indicates we're just getting the normalised scale wrt each 
other's decimal point?  I haven't come up with an immediately good name, but 
I'm sure it's possible.  Of course, I hope the compiler eliminates the extra 
calculation for us, so it's not strictly necessary - but it helps to avoid 
triggering any human linters in future, and more clearly defines the purpose of 
the variables.

Otherwise the patch looks ready to commit to me, thanks again for another great 

Reviewing this ticket highlights another follow-up ticket, or discussion, 
around the mod operator: I'm not entirely sure makes a lot of sense over 
decimal values.  Is this definitely a function we want to provide?  I've only 
ever understood it to be defined over the integer domain.  I note that 
{{BigDecimal}} names it {{remainder}}, and in fact the Java Language Spec does 
the same, so it is not modulus we are offering here and we should rename it 
internally and in any docs anyway.

We should probably discuss this for all value types, particularly floating 
point types.  I honestly was unaware Java supported remainder for floating 
point, and the behaviour is unintuitive to say the least ({{0.4 % 0.25 == 
0.15}} ??)

I've also noted your other follow-up, it's good we're shaking out these minor 
issues before we release the feature, so your help here is greatly appreciated!

> Arithmetic operators over decimal truncate results
> --------------------------------------------------
>                 Key: CASSANDRA-15232
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15232
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: CQL/Semantics
>            Reporter: Benedict
>            Assignee: Liudmila Kornilova
>            Priority: Normal
>              Labels: pull-request-available
>             Fix For: 4.0
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
> The decimal operators hard-code a 128 bit precision for their computations.  
> Probably a precision needs to be configured or decided somehow, but it’s not 
> clear why 128bit was chosen.  Particularly for multiplication and addition, 
> it’s very unclear why we truncate, which is different to our behaviour for 
> e.g. sum() aggregates.  Probably for division we should also ensure that we 
> do not reduce the precision of the two operands.  A minimum of decimal128 
> seems reasonable, but a maximum does not.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to