diff --git a/cql3/attributes.cc b/cql3/attributes.cc index f2ea964a73..08cf7957c0 100644 --- a/cql3/attributes.cc +++ b/cql3/attributes.cc @@ -10,6 +10,7 @@ #include "cql3/attributes.hh" #include "cql3/column_identifier.hh" +#include namespace cql3 { @@ -55,9 +56,9 @@ int64_t attributes::get_timestamp(int64_t now, const query_options& options) { } } -int32_t attributes::get_time_to_live(const query_options& options) { +std::optional attributes::get_time_to_live(const query_options& options) { if (!_time_to_live.has_value() || _time_to_live_unset_guard.is_unset(options)) - return 0; + return std::nullopt; cql3::raw_value tval = expr::evaluate(*_time_to_live, options); if (tval.is_null()) { diff --git a/cql3/attributes.hh b/cql3/attributes.hh index dce6146afc..53280fbe87 100644 --- a/cql3/attributes.hh +++ b/cql3/attributes.hh @@ -45,7 +45,7 @@ public: int64_t get_timestamp(int64_t now, const query_options& options); - int32_t get_time_to_live(const query_options& options); + std::optional get_time_to_live(const query_options& options); db::timeout_clock::duration get_timeout(const query_options& options) const; diff --git a/cql3/statements/modification_statement.cc b/cql3/statements/modification_statement.cc index 50aa43685c..60dfea4794 100644 --- a/cql3/statements/modification_statement.cc +++ b/cql3/statements/modification_statement.cc @@ -17,6 +17,7 @@ #include "cql3/util.hh" #include "validation.hh" #include "db/consistency_level_validations.hh" +#include #include #include #include @@ -95,8 +96,9 @@ bool modification_statement::is_timestamp_set() const { return attrs->is_timestamp_set(); } -gc_clock::duration modification_statement::get_time_to_live(const query_options& options) const { - return gc_clock::duration(attrs->get_time_to_live(options)); +std::optional modification_statement::get_time_to_live(const query_options& options) const { + std::optional ttl = attrs->get_time_to_live(options); + return ttl ? std::make_optional(*ttl) : std::nullopt; } future<> modification_statement::check_access(query_processor& qp, const service::client_state& state) const { diff --git a/cql3/statements/modification_statement.hh b/cql3/statements/modification_statement.hh index 79a6855306..83be9b411e 100644 --- a/cql3/statements/modification_statement.hh +++ b/cql3/statements/modification_statement.hh @@ -127,7 +127,7 @@ public: bool is_timestamp_set() const; - gc_clock::duration get_time_to_live(const query_options& options) const; + std::optional get_time_to_live(const query_options& options) const; virtual future<> check_access(query_processor& qp, const service::client_state& state) const override; diff --git a/cql3/update_parameters.hh b/cql3/update_parameters.hh index 43e3b8cc4f..ea3e1e526f 100644 --- a/cql3/update_parameters.hh +++ b/cql3/update_parameters.hh @@ -86,7 +86,7 @@ public: }; // Note: value (mutation) only required to contain the rows we are interested in private: - const gc_clock::duration _ttl; + const std::optional _ttl; // For operations that require a read-before-write, stores prefetched cell values. // For CAS statements, stores values of conditioned columns. // Is a reference to an outside prefetch_data container since a CAS BATCH statement @@ -99,7 +99,7 @@ public: const query_options& _options; update_parameters(const schema_ptr schema_, const query_options& options, - api::timestamp_type timestamp, gc_clock::duration ttl, const prefetch_data& prefetched) + api::timestamp_type timestamp, std::optional ttl, const prefetch_data& prefetched) : _ttl(ttl) , _prefetched(prefetched) , _timestamp(timestamp) @@ -120,11 +120,7 @@ public: } atomic_cell make_cell(const abstract_type& type, const raw_value_view& value, atomic_cell::collection_member cm = atomic_cell::collection_member::no) const { - auto ttl = _ttl; - - if (ttl.count() <= 0) { - ttl = _schema->default_time_to_live(); - } + auto ttl = this->ttl(); return value.with_value([&] (const FragmentedView auto& v) { if (ttl.count() > 0) { @@ -136,11 +132,7 @@ public: }; atomic_cell make_cell(const abstract_type& type, const managed_bytes_view& value, atomic_cell::collection_member cm = atomic_cell::collection_member::no) const { - auto ttl = _ttl; - - if (ttl.count() <= 0) { - ttl = _schema->default_time_to_live(); - } + auto ttl = this->ttl(); if (ttl.count() > 0) { return atomic_cell::make_live(type, _timestamp, value, _local_deletion_time + ttl, ttl, cm); @@ -162,7 +154,7 @@ public: } gc_clock::duration ttl() const { - return _ttl.count() > 0 ? _ttl : _schema->default_time_to_live(); + return _ttl.value_or(_schema->default_time_to_live()); } gc_clock::time_point expiry() const { diff --git a/test/cql-pytest/cassandra_tests/validation/operations/insert_test.py b/test/cql-pytest/cassandra_tests/validation/operations/insert_test.py index 8fe59f7c52..0270524be8 100644 --- a/test/cql-pytest/cassandra_tests/validation/operations/insert_test.py +++ b/test/cql-pytest/cassandra_tests/validation/operations/insert_test.py @@ -161,7 +161,7 @@ def testInsertWithAStaticColumn(cql, test_keyspace, forceFlush): "INSERT INTO %s (partitionKey, clustering_2, staticValue) VALUES (0, 0, 'A')") # Reproduces #6447 and #12243: -@pytest.mark.xfail(reason="Issue #6447, #12243") +@pytest.mark.xfail(reason="Issue #12243") def testInsertWithDefaultTtl(cql, test_keyspace): secondsPerMinute = 60 with create_table(cql, test_keyspace, f"(a int PRIMARY KEY, b int) WITH default_time_to_live = {10*secondsPerMinute}") as table: diff --git a/test/cql-pytest/test_ttl.py b/test/cql-pytest/test_ttl.py index 631a83fe9b..593122f13f 100644 --- a/test/cql-pytest/test_ttl.py +++ b/test/cql-pytest/test_ttl.py @@ -7,6 +7,7 @@ ############################################################################# from util import new_test_table, unique_key_int +from cassandra.query import UNSET_VALUE import pytest import time @@ -45,10 +46,25 @@ def test_basic_default_ttl(cql, table_ttl_1): # ttl set for the table. Here we check that also in the special case of # "using ttl 0" (which means the item should never expire) it also overrides # the default TTL. Reproduces issue #9842. -@pytest.mark.xfail(reason="Issue #9842") def test_default_ttl_0_override(cql, table_ttl_100): p = unique_key_int() cql.execute(f'INSERT INTO {table_ttl_100} (p, v) VALUES ({p}, 1) USING TTL 0') # We can immediately check that this item's TTL is "null", meaning it # will never expire. There's no need to have any sleeps. assert list(cql.execute(f'SELECT ttl(v) from {table_ttl_100} where p={p}')) == [(None,)] + +# Whereas a zero TTL overrides the default TTL, an unset TTL doesn't - and +# leaves the default TTL. This means that the text in Cassandra's NEWS.txt +# which claims that "an unset bind ttl is treated as 'unlimited'" is not +# accurate - it's only unlimited if there is no default TTL for the table, +# and if there is, this default TTL is used instead of unlimited. In other +# words, an UNSET_VALUE TTL behaves exactly like not specifying a TTL at all. +# See also test_unset.py::test_unset_ttl() +def test_default_ttl_unset_override(cql, table_ttl_100): + p = unique_key_int() + stmt = cql.prepare(f'INSERT INTO {table_ttl_100} (p, v) VALUES ({p}, 1) USING TTL ?') + cql.execute(stmt, [UNSET_VALUE]) + # At this point, reading ttl(v) would normally return 99, but it's + # possible in case of delays to be somewhat lower. But it wouldn't + # be None - which would mean that the item will never expire. + assert list(cql.execute(f'SELECT ttl(v) from {table_ttl_100} where p={p}')) != [(None,)] diff --git a/test/cql-pytest/test_unset.py b/test/cql-pytest/test_unset.py index aed8a702dd..4d81cabece 100644 --- a/test/cql-pytest/test_unset.py +++ b/test/cql-pytest/test_unset.py @@ -115,6 +115,10 @@ def test_unset_list_append(cql, table1, cassandra_bug): # According to Cassandra's NEWS.txt, "an unset bind ttl is treated as # 'unlimited'". It shouldn't skip the write. +# Note that the NEWS.txt is not accurate: An unset ttl isn't really treated +# as unlimited, but rather as the default ttl set on the table. The default +# ttl is usually unlimited, but not always. We test that case in +# test_ttl.py::test_default_ttl_unset() def test_unset_ttl(cql, table1): p = unique_key_int() # First write using a normal TTL: