Merge 'cql: add missing functions for the COUNTER column type' from N…

…adav Har'El

We have had support for COUNTER columns for quite some time now, but some functionality was left unimplemented - various internal and CQL functions resulted in "unimplemented" messages when used, and the goal of this series is to fix those issues. The primary goal was to add the missing support for CASTing counters to other types in CQL (issue #14501), but we also add the missing CQL  `counterasblob()` and `blobascounter()` functions (issue #14742).

As usual, the series includes extensive functional tests for these features, and one pre-existing test for CAST that used to fail now begins to pass.

Fixes #14501
Fixes #14742

Closes #14745

* github.com:scylladb/scylladb:
  test/cql-pytest: test confirming that casting to counter doesn't work
  cql: support casting of counter to other types
  cql: implement missing counterasblob() and blobascounter() functions
  cql: implement missing type functions for "counters" type

cql3/functions/castas_fcts.cc

@@ -165,8 +165,6 @@ static data_value castas_fctn_from_dv_to_string(data_value from) {
     return from.type()->to_string_impl(from);
 }
 
-// FIXME: Add conversions for counters, after they are fully implemented...
-
 static constexpr unsigned next_power_of_2(unsigned val) {
     unsigned ret = 1;
     while (ret <= val) {
@@ -370,6 +368,26 @@ castas_fctn get_castas_fctn(data_type to_type, data_type from_type) {
         return castas_fctn_from_dv_to_string;
     case cast_switch_case_val(kind::utf8, kind::ascii):
         return castas_fctn_simple<sstring, sstring>;
+
+    case cast_switch_case_val(kind::byte, kind::counter):
+        return castas_fctn_simple<int8_t, int64_t>;
+    case cast_switch_case_val(kind::short_kind, kind::counter):
+        return castas_fctn_simple<int16_t, int64_t>;
+    case cast_switch_case_val(kind::int32, kind::counter):
+        return castas_fctn_simple<int32_t, int64_t>;
+    case cast_switch_case_val(kind::long_kind, kind::counter):
+        return castas_fctn_simple<int64_t, int64_t>;
+    case cast_switch_case_val(kind::float_kind, kind::counter):
+        return castas_fctn_simple<float, int64_t>;
+    case cast_switch_case_val(kind::double_kind, kind::counter):
+        return castas_fctn_simple<double, int64_t>;
+    case cast_switch_case_val(kind::varint, kind::counter):
+        return castas_fctn_simple<utils::multiprecision_int, int64_t>;
+    case cast_switch_case_val(kind::decimal, kind::counter):
+        return castas_fctn_from_integer_to_decimal<int64_t>;
+    case cast_switch_case_val(kind::ascii, kind::counter):
+    case cast_switch_case_val(kind::utf8, kind::counter):
+        return castas_fctn_to_string<int64_t>;
     }
     throw exceptions::invalid_request_exception(format("{} cannot be cast to {}", from_type->name(), to_type->name()));
 }

cql3/functions/functions.cc

@@ -105,11 +105,6 @@ functions::init() noexcept {
         if (type == cql3_type::blob) {
             continue;
         }
-        // counters are not supported yet
-        if (type.is_counter()) {
-            warn(unimplemented::cause::COUNTERS);
-            continue;
-        }
 
         declare(make_to_blob_function(type.get_type()));
         declare(make_from_blob_function(type.get_type()));

test/cql-pytest/cassandra_tests/functions/cast_fcts_test.py

@@ -243,7 +243,7 @@ def testCastsWithReverseOrder(cql, test_keyspace):
                 #assertRows(execute(cql, table, "SELECT CAST(" + f + "(CAST(b AS int)) AS text) FROM %s"),
         #           row("2.0"))
 
-@pytest.mark.xfail(reason="issue #14501")
+# Reproduces #14501:
 def testCounterCastsInSelectionClause(cql, test_keyspace):
     with create_table(cql, test_keyspace, "(a int primary key, b counter)") as table:
         execute(cql, table, "UPDATE %s SET b = b + 2 WHERE a = 1")

test/cql-pytest/test_cast_data.py

@@ -28,6 +28,16 @@ def table1(cql, test_keyspace):
     with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, cVarint varint") as table:
         yield table
 
+@pytest.fixture(scope="module")
+def table2(cql, test_keyspace):
+    with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, c counter") as table:
+        yield table
+
+@pytest.fixture(scope="module")
+def table3(cql, test_keyspace):
+    with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, i int, a ascii, bi bigint, b blob, bool boolean, d date, dec decimal, db double, dur duration, f float, addr inet, si smallint, t text, tim time, ts timestamp, tu timeuuid, ti tinyint, u uuid, vc varchar, vi varint") as table:
+        yield table
+
 # Utility function for emulating a wrapping cast of a big positive number
 # into a smaller signed integer of given number of bits. For example,
 # casting 511 to 8 bits results in -1.
@@ -89,4 +99,56 @@ def test_cast_from_large_varint_to_varchar(cql, table1, cassandra_bug):
     cql.execute(f'INSERT INTO {table1} (p, cVarint) VALUES ({p}, {v})')
     assert [(str(v),)] == list(cql.execute(f"SELECT CAST(cVarint AS varchar) FROM {table1} WHERE p={p}"))
 
+# Test casting a counter to various other types. Reproduces #14501.
+def test_cast_from_counter(cql, table2):
+    p = unique_key_int()
+    # Set the counter to 1000 in two increments, to make it less trivial to
+    # read correctly.
+    cql.execute(f'UPDATE {table2} SET c = c + 230 WHERE p = {p}')
+    cql.execute(f'UPDATE {table2} SET c = c + 770 WHERE p = {p}')
+    # We can read back the original number without a cast, or with a silly
+    # cast to the same type "counter".
+    assert [(1000,)] == list(cql.execute(f"SELECT c FROM {table2} WHERE p={p}"))
+    assert [(1000,)] == list(cql.execute(f"SELECT CAST(c AS counter) FROM {table2} WHERE p={p}"))
+    # Casting into smaller integer types results in wraparound
+    assert [(signed(1000,8),)] == list(cql.execute(f"SELECT CAST(c AS tinyint) FROM {table2} WHERE p={p}"))
+    assert [(1000,)] == list(cql.execute(f"SELECT CAST(c AS smallint) FROM {table2} WHERE p={p}"))
+    assert [(1000,)] == list(cql.execute(f"SELECT CAST(c AS int) FROM {table2} WHERE p={p}"))
+    assert [(1000,)] == list(cql.execute(f"SELECT CAST(c AS bigint) FROM {table2} WHERE p={p}"))
+    assert [(1000.0,)] == list(cql.execute(f"SELECT CAST(c AS float) FROM {table2} WHERE p={p}"))
+    assert [(1000.0,)] == list(cql.execute(f"SELECT CAST(c AS double) FROM {table2} WHERE p={p}"))
+    # Casting the counter to string types results in printing the number in
+    # decimal, as expected
+    assert [("1000",)] == list(cql.execute(f"SELECT CAST(c AS ascii) FROM {table2} WHERE p={p}"))
+    assert [("1000",)] == list(cql.execute(f"SELECT CAST(c AS text) FROM {table2} WHERE p={p}"))
+    # "varchar" is supposed to be an alias to "text" and should work, but
+    # suprisingly casting to varchar doesn't work on Cassandra, so we
+    # test it in a separate test below, test_cast_from_counter_to_varchar.
+    # Casting a counter to all other types is NOT allowed:
+    for t in ['blob', 'boolean', 'date', 'duration', 'inet',
+              'timestamp', 'timeuuid', 'uuid']:
+        with pytest.raises(InvalidRequest, match='cast'):
+            cql.execute(f"SELECT CAST(c AS {t}) FROM {table2} WHERE p={p}")
+
+# In test_cast_from_counter we checked that a counter can be cast to the
+# "text" type. Since "varchar" is just an alias for "text", casting
+# to varchar should work too, but in Cassandra it doesn't so this test
+# is marked a Cassandra bug.
+def test_cast_from_counter_to_varchar(cql, table2, cassandra_bug):
+    p = unique_key_int()
+    cql.execute(f'UPDATE {table2} SET c = c + 1000 WHERE p = {p}')
+    assert [("1000",)] == list(cql.execute(f"SELECT CAST(c AS varchar) FROM {table2} WHERE p={p}"))
+
+# Test casts from various types *to* counter type. This is a rather silly
+# operation - casting to "counter" doesn't make a real counter. It could
+# have been supported the same as casting to bigint, but Cassandra chose not
+# to support it and neither do we.
+# The only case that works is the do-nothing casting of a counter to counter,
+# and that case is already checked in test_cast_from_counter().
+def test_cast_to_counter(cql, table3):
+    p = unique_key_int()
+    for col in ['i', 'a', 'bi', 'b', 'bool', 'd', 'dec', 'db', 'dur', 'f', 'addr', 'si', 't', 'tim', 'ts', 'tu', 'ti', 'u', 'vc', 'vi']:
+        with pytest.raises(InvalidRequest, match='cannot be cast'):
+            cql.execute(f"SELECT CAST({col} AS counter) FROM {table3} WHERE p={p}")
+
 # TODO: test casts from more types.

test/cql-pytest/test_counter.py

@@ -0,0 +1,66 @@
+# Copyright 2023-present ScyllaDB
+#
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+###############################################################################
+# Tests for various operations on COUNTER columns.
+# See also tests for casting involving counter columns in test_cast_data.py
+###############################################################################
+
+import pytest
+from util import new_test_table, unique_key_int
+from cassandra.protocol import InvalidRequest
+
+@pytest.fixture(scope="module")
+def table1(cql, test_keyspace):
+    with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, i bigint, v int") as table:
+        yield table
+
+@pytest.fixture(scope="module")
+def table2(cql, test_keyspace):
+    with new_test_table(cql, test_keyspace, "p int PRIMARY KEY, c counter") as table:
+        yield table
+
+# Test that the function counterasblob() exists and works as expected -
+# same as bigintasblob on the same number (a counter is a 64-bit number).
+# Reproduces #14742
+def test_counter_to_blob(cql, table1, table2):
+    p = unique_key_int()
+    cql.execute(f'UPDATE {table1} SET i = 1000 WHERE p = {p}')
+    cql.execute(f'UPDATE {table2} SET c = c + 1000 WHERE p = {p}')
+    expected = b'\x00\x00\x00\x00\x00\x00\x03\xe8'
+    assert [(expected,)] == list(cql.execute(f"SELECT bigintasblob(i) FROM {table1} WHERE p={p}"))
+    assert [(expected,)] == list(cql.execute(f"SELECT counterasblob(c) FROM {table2} WHERE p={p}"))
+    # Although the representation of the counter and bigint types are the
+    # same (64-bit), you can't use the wrong "*asblob()" function:
+    with pytest.raises(InvalidRequest, match='counterasblob'):
+        cql.execute(f"SELECT counterasblob(i) FROM {table1} WHERE p={p}")
+    # The opposite order is allowed in Scylla because of #14319, so let's
+    # split it into a second test test_counter_to_blob2:
+@pytest.mark.xfail(reason="issue #14319")
+def test_counter_to_blob2(cql, table1, table2):
+    p = unique_key_int()
+    cql.execute(f'UPDATE {table2} SET c = c + 1000 WHERE p = {p}')
+    # Reproduces #14319:
+    with pytest.raises(InvalidRequest, match='bigintasblob'):
+        cql.execute(f"SELECT bigintasblob(c) FROM {table2} WHERE p={p}")
+
+# Test that the function blobascounter() exists and works as expected.
+# Reproduces #14742
+def test_counter_from_blob(cql, table1):
+    p = unique_key_int()
+    cql.execute(f'UPDATE {table1} SET i = 1000 WHERE p = {p}')
+    assert [(1000,)] == list(cql.execute(f"SELECT blobascounter(bigintasblob(i)) FROM {table1} WHERE p={p}"))
+
+# blobascounter() must insist to receive a properly-sized (8-byte) blob.
+# If it accepts a shorter blob (e.g., 4 bytes) and returns that to the driver,
+# it will confuse the driver (the driver will expect to read 8 bytes for the
+# bigint but will get only 4).
+# We have test_native_functions.py::test_blobas_wrong_size() that verifies
+# that this protection works for the bigint type, but it turns out it also
+# needs to be separately enforced for the counter type.
+def test_blobascounter_wrong_size(cql, table1):
+    p = unique_key_int()
+    cql.execute(f'UPDATE {table1} SET v = 1000 WHERE p = {p}')
+    with pytest.raises(InvalidRequest, match='blobascounter'):
+        cql.execute(f"SELECT blobascounter(intasblob(v)) FROM {table1} WHERE p={p}")

test/cql-pytest/test_native_functions.py

@@ -0,0 +1,74 @@
+# Copyright 2023-present ScyllaDB
+#
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+###############################################################################
+# Tests for various native (built-in) scalar functions that can be used in
+# various SELECT, INSERT or UPDATE requests. Note we also have tests for
+# some of these functions in many other test files. For example, the tests
+# for the cast() function are in test_cast_data.py.
+###############################################################################
+
+import pytest
+from util import new_test_table, unique_key_int
+from cassandra.protocol import InvalidRequest
+
+@pytest.fixture(scope="module")
+def table1(cql, test_keyspace):
+    with new_test_table(cql, test_keyspace, "p int, i int, g bigint, b blob, s text, t timestamp, u timeuuid, PRIMARY KEY (p)") as table:
+        yield table
+
+# Check that a function that can take a column name as a parameter, can also
+# take a constant. This feature is barely useful for WHERE clauses, and
+# even less useful for selectors, but should be allowed for both.
+# Reproduces #12607.
+@pytest.mark.xfail(reason="issue #12607")
+def test_constant_function_parameter(cql, table1):
+    p = unique_key_int()
+    cql.execute(f"INSERT INTO {table1} (p, b) VALUES ({p}, 0x03)")
+    assert [(p,)] == list(cql.execute(f"SELECT p FROM {table1} WHERE p={p} AND b=tinyintAsBlob(3) ALLOW FILTERING"))
+    assert [(b'\x04',)] == list(cql.execute(f"SELECT tinyintAsBlob(4) FROM {table1} WHERE p={p}"))
+
+# According to the documentation, "The `minTimeuuid` function takes a
+# `timestamp` value t, either a timestamp or a date string.". But although
+# both cases are supported with constant parameters in WHERE restrictions,
+# in a *selector* (the first part of the SELECT, saying what to select), it
+# turns out that ONLY a timestamp column is allowed. Although this is
+# undocumented behavior, both Cassandra and Scylla share it so we deem it
+# correct.
+def test_selector_mintimeuuid(cql, table1):
+    p = unique_key_int()
+    cql.execute(f"INSERT INTO {table1} (p, s, t, i) VALUES ({p}, '2013-02-02 10:00+0000', 123, 456)")
+    # We just check this works, not what the value is:
+    cql.execute(f"SELECT mintimeuuid(t) FROM {table1} WHERE p={p}")
+    # This doesn't work - despite the documentation, in a selector a
+    # date string is not supported by mintimeuuid.
+    with pytest.raises(InvalidRequest, match='of type timestamp'):
+        cql.execute(f"SELECT mintimeuuid(s) FROM {table1} WHERE p={p}")
+    # Other integer types also don't work, it must be a timestamp:
+    with pytest.raises(InvalidRequest, match='of type timestamp'):
+        cql.execute(f"SELECT mintimeuuid(i) FROM {table1} WHERE p={p}")
+
+# Cassandra allows the implicit (and wrong!) casting of a bigint returned
+# by writetime() to the timestamp type required by mintimeuuid(). Scylla
+# doesn't. I'm not sure which behavior we should consider correct, but it's
+# useful to have a test that demonstrates this incompatibility.
+# Reproduces #14319.
+@pytest.mark.xfail(reason="issue #14319")
+def test_selector_mintimeuuid_64bit(cql, table1):
+    p = unique_key_int()
+    cql.execute(f"INSERT INTO {table1} (p, g) VALUES ({p}, 123)")
+    cql.execute(f"SELECT mintimeuuid(g) FROM {table1} WHERE p={p}")
+    cql.execute(f"SELECT mintimeuuid(writetime(g)) FROM {table1} WHERE p={p}")
+
+# blobasbigint() must insist to receive a properly-sized (8-byte) blob.
+# If it accepts a shorter blob (e.g., 4 bytes) and returns that to the driver,
+# it will confuse the driver (the driver will expect to read 8 bytes for the
+# bigint but will get only 4).
+def test_blobas_wrong_size(cql, table1):
+    p = unique_key_int()
+    cql.execute(f"INSERT INTO {table1} (p, i) VALUES ({p}, 123)")
+    # Cassandra and Scylla print: "In call to function system.blobasbigint,
+    # value 0x0000007b is not a valid binary representation for type bigint".
+    with pytest.raises(InvalidRequest, match='blobasbigint'):
+        cql.execute(f"SELECT blobasbigint(intasblob(i)) FROM {table1} WHERE p={p}")