cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Lerer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-11935) Add support for arithmetic operators
Date Wed, 19 Oct 2016 15:49:58 GMT

    [ 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)

Mime
View raw message