From f8aaeb5e875f836627fa77c2f95d4f69dd844ad1 Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Wed, 20 May 2026 14:37:01 +0300 Subject: [PATCH] cql: atomic add/subtract operations with LWT ScyllaDB has special counter columns for which atomic add/subtract operations like `SET a = a + 1` are allowed. Such operations have not been allowed on ordinary non-counter columns, as they would not be properly atomic - the read an the write are separate, and concurrent operations can have incorrect results. This patch makes it allowed to use such atomic add/subtract operations in *LWT* statements. Some examples: UPDATE ... SET a = a - 1 IF a > 0 UPDATE ... SET a = a + 1 IF EXISTS UPDATE ... SET a = a + 1 a != NULL The row updated in the operation, and the updated column (a) should be initialized before the update - arithmetic operations on missing column values silently leave the column null (no error is generated). This add/subtract operations is allowed on any numeric column - integer or floating point of any size. The ability of LWT to fetch the old values of a column and use it to calculate the new value has long been available in our internal CAS implementation - and has been in use for years in Alternator - but until this patch it was not exposed in CQL's LWT. This patch does not add new syntax to CQL - the "SET a = a + b" and "SET a = a - b" syntax that already existed for counters is now allowed for non-counters. This is a new Scylla-only feature that does not exist in Cassandra. Fixes #10568 Signed-off-by: Nadav Har'El --- cql3/operation.cc | 35 ++- cql3/operation.hh | 24 +- cql3/statements/modification_statement.cc | 4 + cql3/statements/modification_statement.hh | 6 + cql3/statements/update_statement.cc | 4 + docs/features/lwt.rst | 32 +++ docs/kb/lwt-differences.rst | 35 +++ .../validation/operations/update_test.py | 4 +- test/cqlpy/test_lwt.py | 253 +++++++++++++++++- 9 files changed, 384 insertions(+), 13 deletions(-) diff --git a/cql3/operation.cc b/cql3/operation.cc index 826703bd52..d70c3d5bd3 100644 --- a/cql3/operation.cc +++ b/cql3/operation.cc @@ -20,6 +20,7 @@ #include "types/list.hh" #include "types/set.hh" #include "types/user.hh" +#include "types/types.hh" #include "service/broadcast_tables/experimental/lang.hh" namespace cql3 { @@ -151,10 +152,19 @@ operation::addition::prepare(data_dictionary::database db, const sstring& keyspa auto ctype = dynamic_pointer_cast(receiver.type); if (!ctype) { - if (!receiver.is_counter()) { - throw exceptions::invalid_request_exception(format("Invalid operation ({}) for non counter column {}", to_string(receiver), receiver.name_as_text())); + if (receiver.is_counter()) { + return make_shared(receiver, std::move(v)); } - return make_shared(receiver, std::move(v)); + // Allow arithmetic on numeric non-counter columns in LWT updates + // (issue #10568). Build an expression that constants:setter will use. + if (receiver.type->is_arithmetic()) { + expr::expression arith_expr = expr::binary_operator( + expr::column_value{&receiver}, + expr::oper_t::ADD, + v); + return make_shared(receiver, std::move(arith_expr)); + } + throw exceptions::invalid_request_exception(format("Invalid operation ({}) for non counter column {}", to_string(receiver), receiver.name_as_text())); } else if (!ctype->is_multi_cell()) { throw exceptions::invalid_request_exception(format("Invalid operation ({}) for frozen collection column {}", to_string(receiver), receiver.name_as_text())); } @@ -184,12 +194,21 @@ shared_ptr operation::subtraction::prepare(data_dictionary::database db, const sstring& keyspace, const column_definition& receiver) const { auto ctype = dynamic_pointer_cast(receiver.type); if (!ctype) { - if (!receiver.is_counter()) { - throw exceptions::invalid_request_exception(format("Invalid operation ({}) for non counter column {}", to_string(receiver), receiver.name_as_text())); + if (receiver.is_counter()) { + auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification); + verify_no_aggregate_functions(v, "SET clause"); + return make_shared(receiver, std::move(v)); } - auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification); - verify_no_aggregate_functions(v, "SET clause"); - return make_shared(receiver, std::move(v)); + if (receiver.type->is_arithmetic()) { + auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification); + verify_no_aggregate_functions(v, "SET clause"); + expr::expression arith_expr = expr::binary_operator( + expr::column_value{&receiver}, + expr::oper_t::SUB, + v); + return make_shared(receiver, std::move(arith_expr)); + } + throw exceptions::invalid_request_exception(format("Invalid operation ({}) for non counter column {}", to_string(receiver), receiver.name_as_text())); } if (!ctype->is_multi_cell()) { throw exceptions::invalid_request_exception( diff --git a/cql3/operation.hh b/cql3/operation.hh index 9629626007..febc73b5e9 100644 --- a/cql3/operation.hh +++ b/cql3/operation.hh @@ -66,13 +66,33 @@ public: } /** - * @return whether the operation requires a read of the previous value to be executed - * (only lists setterByIdx, discard and discardByIdx requires that). + * @return whether the operation requires a read of the previous value to + * be executed. In traditional CQL only a few specific list operations + * perform read before the write, *non-atomically*: + * - lists::setter_by_index (SET c[i] = v) + * - lists::discarder (SET c = c - [v]) + * - lists::discarder_by_index (DELETE c[i]) + * + * We're gradually adding ScyllaDB-only CQL extensions to allow additional + * expressions that need to read the old value of the row, e.g., + * SET r = r + 1. The operations will set require_read() to true, but will + * also set requires_lwt() to true to require that this operation must be + * execute in an LWT update - and this will guarantee that so the read- + * modify-write operation is atomic. */ virtual bool requires_read() const { return false; } + /** + * @return whether the operation is only valid in an LWT (conditional) + * update. For example, non-counter arithmetic (e.g. SET r = r + 1 on a + * regular column) requires LWT so that the read-modify-write is atomic. + */ + virtual bool requires_lwt() const { + return false; + } + /** * Collects the column specification for the bind variables of this operation. * diff --git a/cql3/statements/modification_statement.cc b/cql3/statements/modification_statement.cc index c6035cb99b..5e41010726 100644 --- a/cql3/statements/modification_statement.cc +++ b/cql3/statements/modification_statement.cc @@ -761,6 +761,10 @@ void modification_statement::add_operation(::shared_ptr op) { } } + if (op->requires_lwt()) { + _requires_lwt = true; + } + if (op->column.is_counter()) { auto is_raw_counter_shard_write = op->is_raw_counter_shard_write(); if (_is_raw_counter_shard_write && _is_raw_counter_shard_write != is_raw_counter_shard_write) { diff --git a/cql3/statements/modification_statement.hh b/cql3/statements/modification_statement.hh index 5543ac87d5..9dc55d73d9 100644 --- a/cql3/statements/modification_statement.hh +++ b/cql3/statements/modification_statement.hh @@ -80,6 +80,9 @@ private: // True if any of update operations requires a prefetch. // Pre-computed during statement prepare. bool _requires_read = false; + // True if any of the update operations requires LWT (an IF condition) for + // atomicity, e.g. SET col = col + 1 on a non-counter column. + bool _requires_lwt = false; bool _if_not_exists = false; bool _if_exists = false; @@ -186,6 +189,9 @@ public: // a prefetch of the old cell. bool requires_read() const { return _requires_read; } + // True if any of the update operations requires LWT for atomicity. + bool requires_lwt() const { return _requires_lwt; } + // Columns used in this statement conditions or operations. const column_set& columns_to_read() const { return _columns_to_read; } diff --git a/cql3/statements/update_statement.cc b/cql3/statements/update_statement.cc index c156255e7a..f00ec77d8b 100644 --- a/cql3/statements/update_statement.cc +++ b/cql3/statements/update_statement.cc @@ -497,6 +497,10 @@ update_statement::prepare_internal(data_dictionary::database db, schema_ptr sche } prepare_conditions(db, *schema, ctx, *stmt); stmt->process_where_clause(db, _where_clause, ctx); + if (stmt->requires_lwt() && !stmt->has_conditions()) { + throw exceptions::invalid_request_exception( + "SET with a column expression (e.g. col = col + 1) requires an LWT condition (e.g., IF col != NULL or IF EXISTS) to ensure atomic read-before-write"); + } return stmt; } diff --git a/docs/features/lwt.rst b/docs/features/lwt.rst index 60ce9622d7..9d58a53fce 100644 --- a/docs/features/lwt.rst +++ b/docs/features/lwt.rst @@ -340,6 +340,38 @@ You can use lightweight transactions for any of the following activities: (8 rows) +Arithmetic SET in a LWT update (ScyllaDB extension) +---------------------------------------------------- + +ScyllaDB allows ``SET col = col + value`` and ``SET col = col - value`` on +non-counter numeric columns in a conditional ``UPDATE``. Because the statement +is executed as an LWT, the read-modify-write is atomic: no separate read and +no race condition. An ``IF`` condition is required; without it ScyllaDB +rejects the statement. + +If the column is null, the arithmetic result is also null (standard SQL +null-propagation semantics), so the column remains unset. To catch an +uninitialized column, use an ``IF`` condition that checks the column directly +(e.g. ``IF r != null``) rather than ``IF EXISTS``. + +.. code-block:: cql + + -- Initialize a row, with numeric regular column r set to 0: + INSERT INTO mytable (pk, ck, r) VALUES (1, 1, 0) IF NOT EXISTS; + + -- Increment r atomically: + UPDATE mytable SET r = r + 1 WHERE pk = 1 AND ck = 1 IF EXISTS; + + -- Increment r atomically - and visibly fail if r was not initialized: + UPDATE mytable SET r = r + 1 WHERE pk = 1 AND ck = 1 IF r != null; + + -- Decrement r by 5 only when the value is large enough: + UPDATE mytable SET r = r - 5 WHERE pk = 1 AND ck = 1 IF r >= 5; + +.. note:: + + This syntax is a ScyllaDB extension and is not supported by Apache Cassandra. + Update a table using a LWT -------------------------- diff --git a/docs/kb/lwt-differences.rst b/docs/kb/lwt-differences.rst index e0e7f126fc..fe2c7414fc 100644 --- a/docs/kb/lwt-differences.rst +++ b/docs/kb/lwt-differences.rst @@ -12,6 +12,7 @@ How is it different? * For batch statement, ScyllaDB allows mixing `IF EXISTS`, `IF NOT EXISTS`, and other conditions for the same row. * Unlike Cassandra, ScyllaDB uses per-core data partitioning, so the RPC that is done to perform a transaction talks directly to the right core on a peer replica, avoiding the concurrency overhead. This is, of course, true, if ScyllaDB’s own shard-aware driver is used - otherwise we add an extra hop to the right core at the coordinator node. * ScyllaDB does not store hints for lightweight transaction writes, since this is redundant as all such writes are already present in system.paxos table. +* ScyllaDB supports arithmetic ``SET`` operations (``col = col + value`` and ``col = col - value``) on non-counter numeric columns inside LWT conditional updates. Cassandra does not support this syntax. More on :doc:`Lightweight Transactions (LWT) ` @@ -19,6 +20,40 @@ More on :doc:`Lightweight Transactions (LWT) ` Additional Notes ================ +Arithmetic SET operations in LWT updates +---------------------------------------- + +ScyllaDB allows ``SET col = col + value`` and ``SET col = col - value`` on non-counter numeric +columns inside a conditional ``UPDATE``. The Paxos read-modify-write cycle guarantees atomicity: +the old value is read in the same transaction round, the arithmetic is applied, and the result +is written — all without a separate read or a race condition. + +If the column is null, the arithmetic result is also null (standard SQL null-propagation +semantics), so the column remains unset. To catch an uninitialized column, use an ``IF`` +condition that checks the column value (e.g. ``IF score != null``) rather than ``IF EXISTS``. + +This syntax is a ScyllaDB extension; Cassandra rejects it with an ``InvalidRequest`` error. +An ``IF`` condition (``IF EXISTS`` or a column predicate) is required; omitting the ``IF`` +clause is rejected even in ScyllaDB, because without LWT atomicity the read-modify-write +would not be safe. + +Example — atomic counter on a regular column: + +.. code-block:: cql + + -- Initialize a row, with score=0 + INSERT INTO mytable (pk, ck, score) VALUES (1, 1, 0) IF NOT EXISTS; + + -- Increment atomically: + UPDATE mytable SET score = score + 1 WHERE pk = 1 AND ck = 1 IF EXISTS; + + -- Increment only when score is already set (fails if score is null): + UPDATE mytable SET score = score + 1 WHERE pk = 1 AND ck = 1 IF score != null; + + -- Decrement only when the value is large enough: + UPDATE mytable SET score = score - 5 WHERE pk = 1 AND ck = 1 IF score >= 5; + + Mixing LWT IF clauses in BATCH statements ----------------------------------------- diff --git a/test/cqlpy/cassandra_tests/validation/operations/update_test.py b/test/cqlpy/cassandra_tests/validation/operations/update_test.py index bec978472c..d94612fa2d 100644 --- a/test/cqlpy/cassandra_tests/validation/operations/update_test.py +++ b/test/cqlpy/cassandra_tests/validation/operations/update_test.py @@ -575,6 +575,6 @@ def testThatUpdatesWithEmptyInRestrictionDoNotCreateMutations(cql, test_keyspace def testAdderNonCounter(cql, test_keyspace): with create_table(cql, test_keyspace, f"(pk int PRIMARY KEY, a int, b text)") as table: - # if error ever includes "b" its safe to update this test - with pytest.raises(InvalidRequest, match=re.escape('Invalid operation (a = a + 1) for non counter column a')): + # Cassandra complains about column "a", Scylla about "b", both are fine + with pytest.raises(InvalidRequest, match='Invalid operation.* for non counter column'): execute(cql, table, "UPDATE %s SET a = a + 1, b = b + 'fail' WHERE pk = 1") diff --git a/test/cqlpy/test_lwt.py b/test/cqlpy/test_lwt.py index 8b28ae3bd2..f1d2b2222a 100644 --- a/test/cqlpy/test_lwt.py +++ b/test/cqlpy/test_lwt.py @@ -10,7 +10,7 @@ import re import pytest -from cassandra.protocol import InvalidRequest +from cassandra.protocol import InvalidRequest, SyntaxException from .util import new_test_table, unique_key_int @@ -208,3 +208,254 @@ def test_lwt_insert_json_if_not_exists(cql, table1): # The following assert failed in #8682 (the INSERT was done despite the # row existing). assert list(cql.execute(f'SELECT * FROM {table1} WHERE p={p}')) == [(p, 1, None, 1)] + +# Test that the counter syntax SET i = i + 1 is not allowed on non-counter +# columns. This prepares us to test the same thing for LWT updates, in the +# next test. +def test_counter_syntax_non_counter(cql, table1): + p = unique_key_int() + # Without an LWT condition, arithmetic on non-counter columns is rejected. + with pytest.raises(InvalidRequest): + cql.execute(f'UPDATE {table1} SET r = r + 1 WHERE p={p} AND c=1') + with pytest.raises(InvalidRequest): + cql.execute(f'UPDATE {table1} SET r = r - 1 WHERE p={p} AND c=1') + +# Test that arithmetic SET without an IF clause is rejected at prepare time, +# not silently cached and only rejected at execution (in the previous test, +# test_counter_syntax_non_counter, we tested execution). +def test_counter_syntax_non_counter_prepare(cql, table1): + # PREPARE without IF clause must fail immediately, not succeed and then + # fail later at EXECUTE time. + with pytest.raises(InvalidRequest): + cql.prepare(f'UPDATE {table1} SET r = r + 1 WHERE p = ? AND c = ?') + with pytest.raises(InvalidRequest): + cql.prepare(f'UPDATE {table1} SET r = r - 1 WHERE p = ? AND c = ?') + +# Test that the counter syntax SET r = r + 1 IS allowed in an LWT update +# on non-counter integer columns (issue #10568). This is a Scylla extension +# (Cassandra rejects it). The underlying CAS mechanism reads the old value +# and writes the incremented result atomically. +def test_lwt_counter_syntax(cql, table1, scylla_only): + p = unique_key_int() + # Insert a row with r explicitly set to 0. Arithmetic on a null column + # is an error, so the column must have a value before using arithmetic SET. + cql.execute(f'INSERT INTO {table1} (p, c, r) VALUES ({p}, 1, 0)') + # Increment r from 0 to 1: + rs = list(cql.execute(f'UPDATE {table1} SET r = r + 1 WHERE p={p} AND c=1 IF EXISTS')) + assert len(rs) == 1 and rs[0].applied + assert list(cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1')) == [(1,)] + # Increment again by 3. r is now 1, so it will increment to 4: + rs = list(cql.execute(f'UPDATE {table1} SET r = r + 3 WHERE p={p} AND c=1 IF EXISTS')) + assert len(rs) == 1 and rs[0].applied + assert list(cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1')) == [(4,)] + # Subtraction also works, decrement r by 2 so it will go from 4 to 2. This + # time we'll use a condition on r itself, the condition is on r before the + # update. + rs = list(cql.execute(f'UPDATE {table1} SET r = r - 2 WHERE p={p} AND c=1 IF r = 4')) + assert len(rs) == 1 and rs[0].applied + assert list(cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1')) == [(2,)] + # Try a more sophisticated condition on the arithmetic operation: + # Decrement N from r, but only if r>=N. Try it for one N where it + # fails (3) and one where it succeeds (1). + rs = list(cql.execute(f'UPDATE {table1} SET r = r - 3 WHERE p={p} AND c=1 IF r >= 3')) + assert len(rs) == 1 and not rs[0].applied + assert list(cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1')) == [(2,)] + rs = list(cql.execute(f'UPDATE {table1} SET r = r - 1 WHERE p={p} AND c=1 IF r >= 1')) + assert len(rs) == 1 and rs[0].applied + assert list(cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1')) == [(1,)] + +# Arithmetic on a null just results in a null, so "r = r + 1" just does nothing +# if r was never initialized - it is NOT caught as an error. This is how +# expressions work in SQL, but can be considered a footgun; In contrast, +# DynamoDB does throw an error when an expression uses an uninitialized +# attribute. +def test_lwt_counter_syntax_null_column(cql, table1, scylla_only): + p = unique_key_int() + cql.execute(f'INSERT INTO {table1} (p, c) VALUES ({p}, 1) IF NOT EXISTS') + # At this point, the row (p, 1) exists but has r is null. + rs = list(cql.execute(f'UPDATE {table1} SET r = r + 1 WHERE p={p} AND c=1 IF EXISTS')) + # The condition IF EXISTS was true (the row exists), so the LWT was applied. + assert len(rs) == 1 and rs[0].applied + # But the column r was not written: r + 1 where r is null results in null, + # so r should still be unset. + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r is None + + # Verify the same for subtraction: + rs = list(cql.execute(f'UPDATE {table1} SET r = r - 1 WHERE p={p} AND c=1 IF EXISTS')) + assert len(rs) == 1 and rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r is None + + # We can achieve the same thing with a condition on r (r != null) + # instead of on the row (IF EXISTS). But the difference in this case is + # that a condition on r allows the user to catch an uninitialized r, by + # noticing that the LWT condition failed. + rs = list(cql.execute(f'UPDATE {table1} SET r = r + 1 WHERE p={p} AND c=1 IF r != null')) + assert len(rs) == 1 and not rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r is None + # After initializing r, the condition passes and the increment takes effect. + cql.execute(f'UPDATE {table1} SET r = 0 WHERE p={p} AND c=1') + rs = list(cql.execute(f'UPDATE {table1} SET r = r + 1 WHERE p={p} AND c=1 IF r != null')) + assert len(rs) == 1 and rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r == 1 + +# Test that the LWT counter syntax is allowed for all numeric types, not just +# integers. This is a Scylla extension (issue #10568). +def test_lwt_counter_syntax_numeric_types(cql, test_keyspace, scylla_only): + # All CQL numeric types that should support arithmetic SET in LWT updates. + numeric_types = ['tinyint', 'smallint', 'int', 'bigint', 'varint', 'float', 'double', 'decimal'] + # Build a table with one column per numeric type, all named col_. + col_defs = ', '.join(f'col_{t} {t}' for t in numeric_types) + schema = f'p int PRIMARY KEY, {col_defs}' + with new_test_table(cql, test_keyspace, schema) as table: + p = unique_key_int() + # Initialize all columns to 0; arithmetic requires a non-null value. + col_names = ', '.join(f'col_{t}' for t in numeric_types) + zero_vals = ', '.join(['0'] * len(numeric_types)) + cql.execute(f'INSERT INTO {table} (p, {col_names}) VALUES ({p}, {zero_vals})') + for t in numeric_types: + col = f'col_{t}' + # Increment from 0 to 1 using IF EXISTS. + rs = list(cql.execute(f'UPDATE {table} SET {col} = {col} + 1 WHERE p = {p} IF EXISTS')) + assert len(rs) == 1 and rs[0].applied, f'increment from 0 failed for type {t}' + row = cql.execute(f'SELECT {col} FROM {table} WHERE p = {p}').one() + assert getattr(row, col) == 1, f'expected 1 after increment for type {t}, got {getattr(row, col)}' + # Increment again from 1 to 2. + rs = list(cql.execute(f'UPDATE {table} SET {col} = {col} + 1 WHERE p = {p} IF EXISTS')) + assert len(rs) == 1 and rs[0].applied, f'second increment failed for type {t}' + row = cql.execute(f'SELECT {col} FROM {table} WHERE p = {p}').one() + assert getattr(row, col) == 2, f'expected 2 after second increment for type {t}, got {getattr(row, col)}' + # Subtract 1 from 2, leaving 1. + rs = list(cql.execute(f'UPDATE {table} SET {col} = {col} - 1 WHERE p = {p} IF EXISTS')) + assert len(rs) == 1 and rs[0].applied, f'subtraction failed for type {t}' + row = cql.execute(f'SELECT {col} FROM {table} WHERE p = {p}').one() + assert getattr(row, col) == 1, f'expected 1 after subtraction for type {t}, got {getattr(row, col)}' + +# Currently, the syntax "SET r = p + 1" (different column on LHS and RHS) is +# NOT allowed - the CQL grammar only allows "X = X +/- value", so mismatching +# columns is a syntax error, regardless of whether the statement has an IF +# clause. We may decide to allow this syntax in the future, in which case +# this test should be changed - but for now we don't support it. +def test_lwt_counter_syntax_mismatched_column(cql, table1): + p = unique_key_int() + # The grammar rejects r = p + 1 (p != r) as a SyntaxException. + with pytest.raises(SyntaxException, match='Only expressions of the form X = X'): + cql.execute(f'UPDATE {table1} SET r = p + 1 WHERE p={p} AND c=1 IF EXISTS') + with pytest.raises(SyntaxException, match='Only expressions of the form X = X'): + cql.execute(f'UPDATE {table1} SET r = p - 1 WHERE p={p} AND c=1 IF EXISTS') + # Also rejected without an IF clause: + with pytest.raises(SyntaxException, match='Only expressions of the form X = X'): + cql.execute(f'UPDATE {table1} SET r = p + 1 WHERE p={p} AND c=1') + +# We checked the LWT counter syntax SET r = r + 1 on regular columns, let's +# check that it's *not* allowed for key columns: p and c are numeric but still +# not allowed because they cannot be set by an UPDATE. +def test_lwt_counter_forbidden_key_columns(cql, table1): + p = unique_key_int() + with pytest.raises(InvalidRequest, match='PRIMARY KEY'): + cql.execute(f'UPDATE {table1} SET p = p + 1 WHERE p={p} AND c=1 IF EXISTS') + with pytest.raises(InvalidRequest, match='PRIMARY KEY'): + cql.execute(f'UPDATE {table1} SET c = c + 1 WHERE p={p} AND c=1 IF EXISTS') + +# Test that the LWT counter syntax works in prepared statements, including +# the operand coming from a bind variable. +def test_lwt_counter_syntax_prepared(cql, table1, scylla_only): + p = unique_key_int() + cql.execute(f'INSERT INTO {table1} (p, c, r) VALUES ({p}, 1, 10)') + # Prepare a statement with a bind variable for the increment delta. + inc_stmt = cql.prepare(f'UPDATE {table1} SET r = r + ? WHERE p = ? AND c = ? IF EXISTS') + dec_stmt = cql.prepare(f'UPDATE {table1} SET r = r - ? WHERE p = ? AND c = ? IF EXISTS') + # Increment r by 5: 10 -> 15. + rs = list(cql.execute(inc_stmt, [5, p, 1])) + assert len(rs) == 1 and rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r == 15 + # Decrement r by 3: 15 -> 12. + rs = list(cql.execute(dec_stmt, [3, p, 1])) + assert len(rs) == 1 and rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r == 12 + # Execute the same prepared statement again with a different delta: 12 -> 17. + rs = list(cql.execute(inc_stmt, [5, p, 1])) + assert len(rs) == 1 and rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r == 17 + # A failed condition leaves r unchanged. + rs = list(cql.execute(dec_stmt, [100, p, 999])) # c=999 does not exist + assert len(rs) == 1 and not rs[0].applied + assert cql.execute(f'SELECT r FROM {table1} WHERE p={p} AND c=1').one().r == 17 + +# Test that the LWT counter syntax (add and subtract) catches overflows and +# underflows for fixed-width integer types (tinyint, smallint, int, bigint) +# and doesn't allow them to wrap around. +def test_lwt_counter_syntax_overflow(cql, test_keyspace, scylla_only): + # (type name, bits) for each fixed-width signed integer type. + integer_types = [ + ('tinyint', 8), + ('smallint', 16), + ('int', 32), + ('bigint', 64), + ] + col_defs = ', '.join(f'col_{t} {t}' for t, _ in integer_types) + schema = f'p int PRIMARY KEY, {col_defs}' + with new_test_table(cql, test_keyspace, schema) as table: + p = unique_key_int() + for t, bits in integer_types: + col = f'col_{t}' + max_val = 2**(bits-1) - 1 + min_val = -(2**(bits-1)) + # Incrementing past the maximum must be rejected, not wrap around. + cql.execute(f'UPDATE {table} SET {col} = {max_val} WHERE p = {p}') + with pytest.raises(InvalidRequest, match='overflow'): + cql.execute(f'UPDATE {table} SET {col} = {col} + 1 WHERE p = {p} IF {col} = {max_val}') + # Likewise, decrementing past the minimum must be rejected. + cql.execute(f'UPDATE {table} SET {col} = {min_val} WHERE p = {p}') + with pytest.raises(InvalidRequest, match='overflow'): + cql.execute(f'UPDATE {table} SET {col} = {col} - 1 WHERE p = {p} IF {col} = {min_val}') + # The subtraction -1 - MININT should *not* overflow, it has a valid + # result MAXINT. This forced us to implement a separate SUB + # operation, not just ADD and NEG (unary minus), because NEG on + # MININT overflows. + cql.execute(f'UPDATE {table} SET {col} = -1 WHERE p = {p}') + stmt = cql.prepare(f'UPDATE {table} SET {col} = {col} - ? WHERE p = ? IF {col} = -1') + cql.execute(stmt, [min_val, p]) + assert cql.execute(f'SELECT {col} FROM {table} WHERE p = {p}').one()[0] == max_val + +# Test that adding a float literal (3.5) to an int column is rejected at +# because 3.5 is not a valid integer value. +def test_lwt_counter_syntax_float_on_integer(cql, table1, scylla_only): + p = unique_key_int() + cql.execute(f'INSERT INTO {table1} (p, c, r) VALUES ({p}, 1, 0)') + with pytest.raises(InvalidRequest, match='of type int'): + cql.execute(f'UPDATE {table1} SET r = r + 3.5 WHERE p={p} AND c=1 IF EXISTS') + with pytest.raises(InvalidRequest, match='of type int'): + cql.execute(f'UPDATE {table1} SET r = r - 3.5 WHERE p={p} AND c=1 IF EXISTS') + # Verify the type check is enforced at prepare time if 3.5 is a constant, + # or at execute time if 3.5 is a bind variable: + with pytest.raises(InvalidRequest, match='of type int'): + cql.prepare(f'UPDATE {table1} SET r = r + 3.5 WHERE p = ? AND c = ? IF EXISTS') + with pytest.raises(InvalidRequest, match='of type int'): + cql.prepare(f'UPDATE {table1} SET r = r - 3.5 WHERE p = ? AND c = ? IF EXISTS') + inc_stmt = cql.prepare(f'UPDATE {table1} SET r = r + ? WHERE p = ? AND c = ? IF EXISTS') + dec_stmt = cql.prepare(f'UPDATE {table1} SET r = r - ? WHERE p = ? AND c = ? IF EXISTS') + # When 3.5 is a bind variable, the Python driver catches the type mismatch + # itself before sending the request to Scylla, raising a TypeError. + # We don't intend the Python driver, but this check verifies that the + # server correctly told the driver which type it expects for the bind + # variable. + with pytest.raises(TypeError): + cql.execute(inc_stmt, [3.5, p, 1]) + with pytest.raises(TypeError): + cql.execute(dec_stmt, [3.5, p, 1]) + +# Test that trying to add "decimal" values with wildly different scales is +# rejected with an error, not allowed to proceed with ridiculous amount of +# CPU and memory usage. Reproduces SCYLLADB-1576. +# This test needs to be skipped while SCYLLADB-1576 is not fixed, otherwise +# it will cause the test suite to hang or crash. +@pytest.mark.skip_bug(reason="SCYLLADB-1576: hangs or OOMs instead of rejecting") +def test_lwt_counter_syntax_decimal_magnitude_difference(cql, test_keyspace, scylla_only): + # 1e100000000 is stored compactly as (unscaled=1, scale=-100000000), but + # adding 1 to it forces alignment of decimal points, potentially allocating + # 100 million digits and running out of memory. + with new_test_table(cql, test_keyspace, 'p int PRIMARY KEY, d decimal') as table: + p = unique_key_int() + cql.execute(f"INSERT INTO {table} (p, d) VALUES ({p}, 1e100000000)") + with pytest.raises(InvalidRequest): + cql.execute(f"UPDATE {table} SET d = d + 1 WHERE p = {p} IF EXISTS")