diff --git a/CHANGES.txt b/CHANGES.txt index cdfd2483cc..7afed1e9ef 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,6 +7,7 @@ * Fix CREATE TABLE for CQL2 (CASSANDRA-8144) * Avoid boxing in ColumnStats min/max trackers (CASSANDRA-8109) Merged from 2.0: + * Handle negative timestamp in writetime method (CASSANDRA-8139) * Pig: Remove errant LIMIT clause in CqlNativeStorage (CASSANDRA-8166) * Throw ConfigurationException when hsha is used with the default rpc_max_threads setting of 'unlimited' (CASSANDRA-8116) diff --git a/doc/native_protocol_v3.spec b/doc/native_protocol_v3.spec index 13b6ac617a..89a99ad288 100644 --- a/doc/native_protocol_v3.spec +++ b/doc/native_protocol_v3.spec @@ -187,8 +187,8 @@ Table of Contents To describe the layout of the frame body for the messages in Section 4, we define the following: - [int] A 4 bytes integer - [long] A 8 bytes integer + [int] A 4 bytes signed integer + [long] A 8 bytes signed integer [short] A 2 bytes unsigned integer [string] A [short] n, followed by n bytes representing an UTF-8 string. @@ -321,7 +321,9 @@ Table of Contents conditional update/insert. 0x20: With default timestamp. If present, should be present. is a [long] representing the default timestamp for the query - in microseconds (negative values are forbidden). If provided, this will + in microseconds (negative values are discouraged but supported for + backward compatibility reasons except for the smallest negative + value (-2^63) that is forbidden). If provided, this will replace the server side assigned timestamp as default timestamp. Note that a timestamp in the query itself will still override this timestamp. This is entirely optional. diff --git a/src/java/org/apache/cassandra/cql3/QueryOptions.java b/src/java/org/apache/cassandra/cql3/QueryOptions.java index c946e8b3dd..5431a425a5 100644 --- a/src/java/org/apache/cassandra/cql3/QueryOptions.java +++ b/src/java/org/apache/cassandra/cql3/QueryOptions.java @@ -101,7 +101,7 @@ public abstract class QueryOptions public long getTimestamp(QueryState state) { long tstamp = getSpecificOptions().timestamp; - return tstamp >= 0 ? tstamp : state.getTimestamp(); + return tstamp != Long.MIN_VALUE ? tstamp : state.getTimestamp(); } /** @@ -326,12 +326,12 @@ public abstract class QueryOptions int pageSize = flags.contains(Flag.PAGE_SIZE) ? body.readInt() : -1; PagingState pagingState = flags.contains(Flag.PAGING_STATE) ? PagingState.deserialize(CBUtil.readValue(body)) : null; ConsistencyLevel serialConsistency = flags.contains(Flag.SERIAL_CONSISTENCY) ? CBUtil.readConsistencyLevel(body) : ConsistencyLevel.SERIAL; - long timestamp = -1L; + long timestamp = Long.MIN_VALUE; if (flags.contains(Flag.TIMESTAMP)) { long ts = body.readLong(); - if (ts < 0) - throw new ProtocolException("Invalid negative (" + ts + ") protocol level timestamp"); + if (ts == Long.MIN_VALUE) + throw new ProtocolException(String.format("Out of bound timestamp, must be in [%d, %d] (got %d)", Long.MIN_VALUE + 1, Long.MAX_VALUE, ts)); timestamp = ts; } @@ -402,7 +402,7 @@ public abstract class QueryOptions flags.add(Flag.PAGING_STATE); if (options.getSerialConsistency() != ConsistencyLevel.SERIAL) flags.add(Flag.SERIAL_CONSISTENCY); - if (options.getSpecificOptions().timestamp >= 0) + if (options.getSpecificOptions().timestamp != Long.MIN_VALUE) flags.add(Flag.TIMESTAMP); return flags; } diff --git a/src/java/org/apache/cassandra/cql3/UpdateParameters.java b/src/java/org/apache/cassandra/cql3/UpdateParameters.java index d31b8d90d3..62ec09c05e 100644 --- a/src/java/org/apache/cassandra/cql3/UpdateParameters.java +++ b/src/java/org/apache/cassandra/cql3/UpdateParameters.java @@ -44,6 +44,7 @@ public class UpdateParameters private final Map prefetchedLists; public UpdateParameters(CFMetaData metadata, QueryOptions options, long timestamp, int ttl, Map prefetchedLists) + throws InvalidRequestException { this.metadata = metadata; this.options = options; @@ -51,6 +52,11 @@ public class UpdateParameters this.ttl = ttl; this.localDeletionTime = (int)(System.currentTimeMillis() / 1000); this.prefetchedLists = prefetchedLists; + + // We use MIN_VALUE internally to mean the absence of of timestamp (in Selection, in sstable stats, ...), so exclude + // it to avoid potential confusion. + if (timestamp == Long.MIN_VALUE) + throw new InvalidRequestException(String.format("Out of bound timestamp, must be in [%d, %d]", Long.MIN_VALUE + 1, Long.MAX_VALUE)); } public Cell makeColumn(CellName name, ByteBuffer value) throws InvalidRequestException diff --git a/src/java/org/apache/cassandra/cql3/statements/Selection.java b/src/java/org/apache/cassandra/cql3/statements/Selection.java index 7893b4d7a0..de05f44ed3 100644 --- a/src/java/org/apache/cassandra/cql3/statements/Selection.java +++ b/src/java/org/apache/cassandra/cql3/statements/Selection.java @@ -303,7 +303,7 @@ public abstract class Selection current.add(isDead(c) ? null : value(c)); if (timestamps != null) { - timestamps[current.size() - 1] = isDead(c) ? -1 : c.timestamp(); + timestamps[current.size() - 1] = isDead(c) ? Long.MIN_VALUE : c.timestamp(); } if (ttls != null) { @@ -503,7 +503,7 @@ public abstract class Selection if (isWritetime) { long ts = rs.timestamps[idx]; - return ts >= 0 ? ByteBufferUtil.bytes(ts) : null; + return ts != Long.MIN_VALUE ? ByteBufferUtil.bytes(ts) : null; } int ttl = rs.ttls[idx]; diff --git a/test/unit/org/apache/cassandra/cql3/TimestampTest.java b/test/unit/org/apache/cassandra/cql3/TimestampTest.java new file mode 100644 index 0000000000..6673904d54 --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/TimestampTest.java @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.cassandra.cql3; + +import org.junit.Test; + +public class TimestampTest extends CQLTester +{ + @Test + public void testNegativeTimestamps() throws Throwable + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, v int)"); + + execute("INSERT INTO %s (k, v) VALUES (?, ?) USING TIMESTAMP ?", 1, 1, -42L); + assertRows(execute("SELECT writetime(v) FROM %s WHERE k = ?", 1), + row(-42L) + ); + + assertInvalid("INSERT INTO %s (k, v) VALUES (?, ?) USING TIMESTAMP ?", 2, 2, Long.MIN_VALUE); + } +}