Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Fix Version/s: 0.8.1
    • Component/s: API, Core
    • Labels:
      None
    1. CASSANDRA-2473.patch
      23 kB
      Pavel Yaskevich
    2. CASSANDRA-2473-v2.patch
      33 kB
      Pavel Yaskevich
    3. CASSANDRA-2473-0.8.patch
      32 kB
      Pavel Yaskevich

      Activity

      Hide
      Aaron Turner added a comment -

      Yes, I just came to the same conclusion:

      rpm -qa | grep cassandra
      apache-cassandra08-0.8.1-1

      grep ' Cassandra version:' /var/log/cassandra/system.log | tail -1
      INFO [main] 2011-07-13 12:04:31,039 StorageService.java (line 368) Cassandra version: 0.8.0

      Show
      Aaron Turner added a comment - Yes, I just came to the same conclusion: rpm -qa | grep cassandra apache-cassandra08-0.8.1-1 grep ' Cassandra version:' /var/log/cassandra/system.log | tail -1 INFO [main] 2011-07-13 12:04:31,039 StorageService.java (line 368) Cassandra version: 0.8.0
      Hide
      Jonathan Ellis added a comment -

      Somehow you are not running the Cassandra version you think you are. samal on the mailing list confirms this works for him too on 0.8.1.

      Show
      Jonathan Ellis added a comment - Somehow you are not running the Cassandra version you think you are. samal on the mailing list confirms this works for him too on 0.8.1.
      Hide
      Aaron Turner added a comment -

      Release (datasax 0.8.1 rpm). cqlsh 1.0.3

      I've tried the same query via Thrift/execute_cql_query() and I get the same error.

      Show
      Aaron Turner added a comment - Release (datasax 0.8.1 rpm). cqlsh 1.0.3 I've tried the same query via Thrift/execute_cql_query() and I get the same error.
      Hide
      Pavel Yaskevich added a comment -

      I'm running branch cassandra-0.8.1 and your example works for me. Are you running release or dev build version?

      Show
      Pavel Yaskevich added a comment - I'm running branch cassandra-0.8.1 and your example works for me. Are you running release or dev build version?
      Hide
      Aaron Turner added a comment -

      cqlsh> CREATE COLUMNFAMILY Test (KEY varchar PRIMARY KEY) WITH default_validation = CounterColumnType and comparator = AsciiType;

      cqlsh> UPDATE Test SET TestCol = TestCol + 17 WHERE KEY = 'MyKey';
      Bad Request: line 1:34 no viable alternative at character '+'

      cqlsh> UPDATE Test SET 'TestCol' = 'TestCol' + 17 WHERE KEY = 'MyKey';
      Bad Request: line 1:38 no viable alternative at character '+'

      Show
      Aaron Turner added a comment - cqlsh> CREATE COLUMNFAMILY Test (KEY varchar PRIMARY KEY) WITH default_validation = CounterColumnType and comparator = AsciiType; cqlsh> UPDATE Test SET TestCol = TestCol + 17 WHERE KEY = 'MyKey'; Bad Request: line 1:34 no viable alternative at character '+' cqlsh> UPDATE Test SET 'TestCol' = 'TestCol' + 17 WHERE KEY = 'MyKey'; Bad Request: line 1:38 no viable alternative at character '+'
      Hide
      Jonathan Ellis added a comment -

      We have CQL tests where this scenario works fine. Can you post a complete cqlsh session demonstrating the problem?

      Show
      Jonathan Ellis added a comment - We have CQL tests where this scenario works fine. Can you post a complete cqlsh session demonstrating the problem?
      Hide
      Aaron Turner added a comment -

      I'm running 0.8.1 and I can't increment any counters using the syntax listed above:

      UPDATE MyCounter SET TestCol = TestCol + 12 WHERE KEY = 'MyKey';

      The error is: no viable alternative at character '+'

      I've tried quoting the column names to no avail.

      Show
      Aaron Turner added a comment - I'm running 0.8.1 and I can't increment any counters using the syntax listed above: UPDATE MyCounter SET TestCol = TestCol + 12 WHERE KEY = 'MyKey'; The error is: no viable alternative at character '+' I've tried quoting the column names to no avail.
      Hide
      Hudson added a comment -

      Integrated in Cassandra-0.8 #154 (See https://builds.apache.org/hudson/job/Cassandra-0.8/154/)
      add cql counter support
      patch by pyaskevich; reviewed by jbellis for CASSANDRA-2473

      jbellis : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132402
      Files :

      • /cassandra/branches/cassandra-0.8/test/system/test_cql.py
      • /cassandra/branches/cassandra-0.8/CHANGES.txt
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AbstractModification.java
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Operation.java
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g
      • /cassandra/branches/cassandra-0.8/drivers/py/cql/marshal.py
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/UpdateStatement.java
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/DeleteStatement.java
      • /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/BatchStatement.java
      Show
      Hudson added a comment - Integrated in Cassandra-0.8 #154 (See https://builds.apache.org/hudson/job/Cassandra-0.8/154/ ) add cql counter support patch by pyaskevich; reviewed by jbellis for CASSANDRA-2473 jbellis : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132402 Files : /cassandra/branches/cassandra-0.8/test/system/test_cql.py /cassandra/branches/cassandra-0.8/CHANGES.txt /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AbstractModification.java /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Operation.java /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/Cql.g /cassandra/branches/cassandra-0.8/drivers/py/cql/marshal.py /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/UpdateStatement.java /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/DeleteStatement.java /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/BatchStatement.java
      Hide
      Jonathan Ellis added a comment -

      committed

      Show
      Jonathan Ellis added a comment - committed
      Hide
      Pavel Yaskevich added a comment -

      rebased with cassandra-0.8, the latest commit d1185914ba34ea11c2bc6bbc4686070a9a9e67a8

      Show
      Pavel Yaskevich added a comment - rebased with cassandra-0.8, the latest commit d1185914ba34ea11c2bc6bbc4686070a9a9e67a8
      Hide
      Pavel Yaskevich added a comment -

      It was made for version 1.0 (branch: trunk) originally so I will rebase to 0.8 and attach a patch today.

      Show
      Pavel Yaskevich added a comment - It was made for version 1.0 (branch: trunk) originally so I will rebase to 0.8 and attach a patch today.
      Hide
      Jonathan Ellis added a comment -

      looks good eyeballing it, but doesn't apply to 0.8 branch for me

      Show
      Jonathan Ellis added a comment - looks good eyeballing it, but doesn't apply to 0.8 branch for me
      Hide
      Pavel Yaskevich added a comment -

      work branch: trunk, the latest commit 583ac45752f0290c30416afcd5f49f9314cd66eb

      Show
      Pavel Yaskevich added a comment - work branch: trunk, the latest commit 583ac45752f0290c30416afcd5f49f9314cd66eb
      Hide
      Jonathan Ellis added a comment -

      I think the best fit for CQL would actually be to do the decode on the server. (To int? To varint? Not actually sure what makes the most sense there.)

      Show
      Jonathan Ellis added a comment - I think the best fit for CQL would actually be to do the decode on the server. (To int? To varint? Not actually sure what makes the most sense there.)
      Hide
      Pavel Yaskevich added a comment - - edited

      Everything is ready except of marshal.py - need help with decoding counter column value, can somebody help? I have already prepared method unmarshal_counter(bytestr) with stub and everything else.

      Counter column support tests are in test_cql.py but without checking values because of marshaling problem...

      Show
      Pavel Yaskevich added a comment - - edited Everything is ready except of marshal.py - need help with decoding counter column value, can somebody help? I have already prepared method unmarshal_counter(bytestr) with stub and everything else. Counter column support tests are in test_cql.py but without checking values because of marshaling problem...
      Hide
      Sylvain Lebresne added a comment -

      Yep, I suck at SQL and don't know much about it. I 100% agree that we should make it as compatible as possible so let's roll with you version.

      Show
      Sylvain Lebresne added a comment - Yep, I suck at SQL and don't know much about it. I 100% agree that we should make it as compatible as possible so let's roll with you version.
      Hide
      Jonathan Ellis added a comment -

      My problem with that is it's actually not valid SQL. I'd rather keep SQL compatibility where that is reasonable and return an error when used incorrectly, than invent new syntax when it is not necessary.

      Show
      Jonathan Ellis added a comment - My problem with that is it's actually not valid SQL. I'd rather keep SQL compatibility where that is reasonable and return an error when used incorrectly, than invent new syntax when it is not necessary.
      Hide
      Pavel Yaskevich added a comment -

      I agree with Sylvain on this.

      Show
      Pavel Yaskevich added a comment - I agree with Sylvain on this.
      Hide
      Sylvain Lebresne added a comment -

      I agree with using native syntax, not a procedure call. But maybe
      UPDATE foo SET X += N WHERE key = K;
      would make it more clear that you can really only increment X, not set any value you want (it's easy to check and reject the request, but I like it when the syntax make is easy to not screw up)

      Show
      Sylvain Lebresne added a comment - I agree with using native syntax, not a procedure call. But maybe UPDATE foo SET X += N WHERE key = K; would make it more clear that you can really only increment X, not set any value you want (it's easy to check and reject the request, but I like it when the syntax make is easy to not screw up)
      Hide
      Jonathan Ellis added a comment -

      IMO the most natural way to express this is to start to allow the syntax

      UPDATE foo SET X = X + N WHERE key = K;

      (for counter column X)

      The alternatives would be to represent it as a procedure call:

      SELECT increment(foo, K, X, N);

      I don't like that much though, since it's a semantic mismatch (there is no actual function named increment involved). I'd rather save that for when we actually introduce functions/stored procedures.

      Show
      Jonathan Ellis added a comment - IMO the most natural way to express this is to start to allow the syntax UPDATE foo SET X = X + N WHERE key = K; (for counter column X) The alternatives would be to represent it as a procedure call: SELECT increment(foo, K, X, N); I don't like that much though, since it's a semantic mismatch (there is no actual function named increment involved). I'd rather save that for when we actually introduce functions/stored procedures.

        People

        • Assignee:
          Pavel Yaskevich
          Reporter:
          Eric Evans
          Reviewer:
          Jonathan Ellis
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development