sql: verify operator precedence of ~

[maddyblue]

postegres:

postgres=# select ~0+1;
 ?column? 
----------
       -2
(1 row)

postgres=# select (~0)+1;
 ?column? 
----------
        0
(1 row)

cockroach:

root@:26257/> select ~0+1;
+-----------+
| (~ 0) + 1 |
+-----------+
|         0 |
+-----------+
(1 row)

Seems like ~ may need to go lower. See #2305.

[dianasaur323]

assigning to @cuongdo for him to reassign.

[maddyblue]

I'll take this.

[dianasaur323]

Great, thanks Matt.

…

On Wed, Apr 12, 2017 at 2:14 PM, Matt Jibson ***@***.***> wrote: I'll take this. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#14814 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFgDWBGLafM53EfHKq6SjN4XRCsfEzwCks5rvRSBgaJpZM4M6maT> .

-- Diana Hsieh [email protected] 407-690-9048

[knz]

So this issue makes me sad. In increasing order of importance:

1. no other language has unary operators at a lower precedence level than binary
2. our internal policy was "make CockroachDB look and feel like pg unless we had a good reason not to". here we have a good reason not to: it is ugly.
3. is there a client app that is broken because of this? If not, why is there a problem?

[knz]

Also, why is this a v1.0 issue?

[dianasaur323]

I don't think this is necessarily a must-have for the v1.0, but my sense is that for some of these clean-ups on SQL language, if they don't take too much time, we can try squeezing them in for usability reasons. I don't feel strongly about keeping this in the 1.0 though. Re matching PostgreSQL, that seems to be the path we have taken?

[cuongdo]

This would be a backward-incompatible change, so we'd need to find a way mitigate that if we address this. Even more so after 1.0.

Also, if we decide to diverge from PG (which I believe is fine is some circumstances), we need a way of documenting those differences for those who use PG drivers and ORMs with us and expect PG semantics.

[tamird]

It's 1.0 because it's backwards-incompatible, I think, which seems correct.

…

On Apr 13, 2017 08:19, "Diana Hsieh" ***@***.***> wrote: I don't think this is necessarily a must-have for the v1.0, but my sense is that for some of these clean-ups on SQL language, if they don't take too much time, we can try squeezing them in for usability reasons. I don't feel strongly about keeping this in the 1.0 though. Re matching PostgreSQL, that seems to be the path we have taken? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#14814 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPMaoiFgV-Es7rZc0pqd6fXUBdBhiks5rvhLegaJpZM4M6maT> .

[jordanlewis]

This is a simple change with a big payoff. We generally claim compatibility with Postgres - why diverge here? Users who expect Postgres compatibility will probably expect that they can paste a complicated query that uses this feature and see no difference. However, bit not is unintuitive enough that they might notice no difference but still experience incorrect results and have a hard time tracking down the issue.

Then again, how frequently do people really use ~ in SQL queries? Perhaps the rarity of use is sufficient to justify divergence in the name of normal unary operator behavior.

[tamird]

+1 on not diverging. "ugly" is not a good reason.

…

On Thu, Apr 13, 2017 at 10:07 AM, Jordan Lewis ***@***.***> wrote: This is a simple change with a big payoff. We generally claim compatibility with Postgres - why diverge here? Users who expect Postgres compatibility will probably expect that they can paste a complicated query that uses this feature and see no difference. However, bit not is unintuitive enough that they might notice no difference but still experience incorrect results and have a hard time tracking down the issue. Then again, how frequently do people really use ~ in SQL queries? Perhaps the rarity of use is sufficient to justify divergence in the name of normal unary operator behavior. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#14814 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABdsPHJAAbamUwL62-C_2driph__ZTQzks5rviwXgaJpZM4M6maT> .

[dianasaur323]

Given the comments, it sounds like we should do it before the 1.0 and to maintain PostgreSQL compatibility.

[maddyblue]

I agree with everyone. It is surprising if a unary operator has a lower precedence than a binary operator, suggesting we shouldn't change this. I also agree that maintaining Postgres compat makes sense in general, and this concern weighs higher in my mind.

We are advertising Postgres ORM support. This means we need to closely support a Postgres dialect, which must include things like operator precedence and associativity. Is there an ORM that depends on the above behavior being correct? I'm not sure. But if there is and we differ, then that application will get back incorrect results without knowing why, and without an error message.

Overall: I think we should match Postgres' operators because we advertise ORM support.

[knz]

What about not supporting this unary operator for now, and simply support a built-in function to negate numbers instead. Let's see if anyone requests the operator in the future.
I'd be more comfortable with no operator at all (and a built-in function).

[maddyblue]

That's fine with me.

[jordanlewis]

This option is quite tempting. This way applications will be certain to encounter errors if they try to use bitwise not. ISTM that legitimate use of ~ is rare enough that we will not be hurting 99% of application developers by doing this.

[dianasaur323]

Closing as addressed by #14908. Please re-open if you disagree.