[
https://issues.apache.org/jira/browse/CASSANDRA-15232?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902898#comment-16902898
]
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
consider:
# {{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
submission.
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
(v7.6.14#76016)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
