[
https://issues.apache.org/jira/browse/CASSANDRA-11935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15589095#comment-15589095
]
Benjamin Lerer commented on CASSANDRA-11935:
--------------------------------------------
{quote}This is more for the record (not really asking anything here), but the
grammar is starting to be unreadable imo. I don't meant that as a criticism of
this patch, as I don't have a better idea for dealing with this without a much
larger refactor of the CQL code, but we may want to keep an eye on this moving
forward.{quote}
I also think that the grammar is becoming more and more complex. We should
probably look for a way of merging term and selector rules to simplify the
things and start considering migrating to {{ANTLR 4}}. Migrating to it will not
be a simple task as most of rules will have to be rewritten but the rules will
be more simple as ANTLR 4 is able to automatically rewrites left-recursive
rules.
bq. I want to be clear that I'm not confident in my own capacity to vet grammar
changes properly at this point
I am perfectly fine in taking all the blame if an issue show up. ;-)
{quote}Not sure about the hard-coding of MathContext.DECIMAL128 as
precision/rounding settings for BigDecimal. As in, I'm not knowledgable enough
on decimal arithmetic to decide if this is the best choice or not. But in
doubt, I would have maybe stick to the default BigDecimal behavior.{quote}
With the default BigDecimal behavior,
{{BigDecimal.valueOf(8.5).divide(BigDecimal.valueOf(3))}} will throw an
{{ArithmeticException: Non-terminating decimal expansion; no exact
representable decimal result.}}
Throwing an error in such a case really limit the usefulness of operations for
{{BigDecimal}}.
{{MathContext.DECIMAL128}} will round numbers at a precision of 34 digits
towards the nearest neighbor. It seems reasonable to me but we can change it if
you prefer different precision/rounding settings.
bq. not sure why we stop at int for integers. Why not return a tinyint if it
fits? I'd expect users to be surprised if c + 1 returns a int when c is a
tinyint/smallint, especially when c + c does return a tinyint/smallint. Worst,
with c still a tinyint, this means WHERE c = 2 works but WHERE c = 1 + 1
doesn't.
I implemented a fix for that but I am not confident that is the way to go. {{c
= 1 + 1}} will nearly always require a cast. By consequence, I do not think
that we can base our decision on this example.
It is clear that some user might be surprise if {{c + 1}} returns a int when
{{c}} is a {{tinyint/smallint}}. In the programming world, types like
{{tinyint}} or {{smallint}} are more to allow people to do some optimizations
and {{int}} is usually the default type for integer numbers. In fact most of
the relational databases, including
[ProgreSQL|https://www.postgresql.org/docs/current/static/sql-syntax-lexical.html#SQL-SYNTAX-CONSTANTS-NUMERIC]),
will convert litterals like {{1}} to integers.
So, I do not believe that this behavior will surprise a lot of users. On the
other hand, I fear that most of the users will be surprise by the result of
{{100 + 50}} if we narrow the type of {{100}} and {{50}} to {{tinyint}}.
I fixed the other issues and pushed new branches:
||branch||utests||dtests||
|[3.X|https://github.com/apache/cassandra/compare/trunk...blerer:11935-3.X]|[3.X|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-11935-3.X-testall/]|[3.X|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-11935-3.X-dtest/]|
|[trunk|https://github.com/apache/cassandra/compare/trunk...blerer:11935-trunk]|[trunk|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-11935-trunk-testall/]|[trunk|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-11935-trunk-dtest/]|
> Add support for arithmetic operators
> ------------------------------------
>
> Key: CASSANDRA-11935
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11935
> Project: Cassandra
> Issue Type: Sub-task
> Components: CQL
> Reporter: Benjamin Lerer
> Assignee: Benjamin Lerer
> Fix For: 3.x
>
>
> The goal of this ticket is to add support for arithmetic operators:
> * {{-}}: Change the sign of the argument
> * {{+}}: Addition operator
> * {{-}}: Minus operator
> * {{*}}: Multiplication operator
> * {{/}}: Division operator
> * {{%}}: Modulo operator
> This ticket we should focus on adding operator only for numeric types to keep
> the scope as small as possible. Dates and string operations will be adressed
> in follow up tickets.
> The operation precedence should be:
> # {{*}}, {{/}}, {{%}}
> # {{+}}, {{-}}
> Some implicit data conversion should be performed when operations are
> performed on different types (e.g. double + int).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
