Merge 'Fix tests in test/cql-pytest/ that fail on Cassandra' from Nad…

…av Har'El

As a general rule, tests in test/cql-pytest shouldn't just pass on Scylla - they also should not fail on Cassandra; A test that fails on Cassandra may indicate that the test is wrong, or that Scylla's behavior is wrong and the test just enshrines that wrong behavior. Each time we see a test fail on Cassandra we need to check if this is not the case. We also have special markers scylla_only and cassandra_bug to put on tests that we know _should_ fail on Cassandra because it is missing some Scylla-only feature or there is a bug in Cassandra, respectively. Such tests will be xfailed/skipped when running on Cassandra, and not report failures.

Unfortunately, over time more several tests got into our suite in that did not pass on Cassandra. In this series I went over all of them, and fixed each to pass - or be skipped - on Cassandra, in a way that each patch explains.

Fixes #16027

Closes #16033

* github.com:scylladb/scylladb:
  test/cql-pytest: fix test_describe.py to not fail on Cassandra
  test/cql-pytest: fix select_single_column_relation_test.py to not fail on Cassandra
  test/cql-pytest: fix compact_storage_test.py to not fail on Cassandra
  test/cql-pytest: fix test_secondary_index.py to not fail on Cassandra
  test/cql-pytest: fix test_materialized_view.py to not fail on Cassandra
  test/cql-pytest: fix test_keyspace.py to not fail on Cassandra
  test/cql-pytest: test_guardrail_replication_strategy.py is Scylla-only
  test/cql-pytest: partial fix for test_compaction_strategy_validation.py on Cassandra
  test/cql-pytest: fix test_filtering.py to not fail on Cassandra

test/cql-pytest/cassandra_tests/validation/operations/compact_storage_test.py

@@ -1046,7 +1046,7 @@ def testDeleteWithOneClusteringColumns(cql, test_keyspace, forceFlush):
         assertInvalidMessage(cql, table, "Only EQ and IN relation are supported on the partition key",
                              "DELETE FROM %s WHERE partitionKey > ? AND clustering = ?", 0, 1)
 
-        assertInvalidMessage(cql, table, "Cannot use CONTAINS on non-collection column",
+        assertInvalidMessageRE(cql, table, "Cannot use CONTAINS on non-collection column|Cannot use DELETE with CONTAINS",
                              "DELETE FROM %s WHERE partitionKey CONTAINS ? AND clustering = ?", 0, 1)
 
         # Non primary key in the where clause
@@ -3965,7 +3965,7 @@ def testUpdate(cql, test_keyspace, forceFlush):
         assertInvalidMessage(cql, table, "Only EQ and IN relation are supported on the partition key",
                              "UPDATE %s SET value = ? WHERE partitionKey > ? AND clustering_1 = ?", 7, 0, 1)
 
-        assertInvalidMessage(cql, table, "Cannot use CONTAINS on non-collection column",
+        assertInvalidMessageRE(cql, table, "Cannot use CONTAINS on non-collection column|Cannot use UPDATE with CONTAINS",
                              "UPDATE %s SET value = ? WHERE partitionKey CONTAINS ? AND clustering_1 = ?", 7, 0, 1)
 
         assertInvalidMessage(cql, table, "value",
@@ -4095,7 +4095,7 @@ def testUpdateWithTwoClusteringColumns(cql, test_keyspace, forceFlush):
         assertInvalidMessage(cql, table, "Only EQ and IN relation are supported on the partition key",
                              "UPDATE %s SET value = ? WHERE partitionKey > ? AND clustering_1 = ? AND clustering_2 = ?", 7, 0, 1, 1)
 
-        assertInvalidMessage(cql, table, "Cannot use CONTAINS on non-collection column",
+        assertInvalidMessageRE(cql, table, "Cannot use CONTAINS on non-collection column|Cannot use UPDATE with CONTAINS",
                              "UPDATE %s SET value = ? WHERE partitionKey CONTAINS ? AND clustering_1 = ? AND clustering_2 = ?", 7, 0, 1, 1)
 
         assertInvalid(cql, table,

test/cql-pytest/cassandra_tests/validation/operations/select_single_column_relation_test.py

@@ -89,7 +89,10 @@ def testClusteringColumnRelations(cql, test_keyspace):
                    ["first", 2, 6, 2],
                    ["first", 3, 7, 3])
 
-        assert_empty(execute(cql, table, "select * from %s where a = ? and b in ? and c in ?", "first", None, [7, 6]))
+        # Scylla does allow IN NULL (see commit 52bbc1065c8) so this test
+        # is commented out
+        #assert_invalid_message(cql, table, "Invalid null value for column b",
+        #                     "select * from %s where a = ? and b in ? and c in ?", "first", None, [7, 6])
 
         assert_rows(execute(cql, table, "select * from %s where a = ? and c >= ? and b in (?, ?)", "first", 6, 3, 2),
                    ["first", 2, 6, 2],

test/cql-pytest/test_compaction_strategy_validation.py

@@ -6,38 +6,48 @@
 # Tests for compaction strategy validation
 #############################################################################
 
-from cassandra_tests.porting import create_keyspace, create_table, execute, assert_invalid_throw_message, ConfigurationException
-
-def assert_throws(cql, msg, cmd):
-    with create_keyspace(cql, "replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }") as ks:
-        with create_table(cql, ks, "(a int PRIMARY KEY, b int) WITH compaction = { 'class' : 'SizeTieredCompactionStrategy' }") as table:
-            assert_invalid_throw_message(cql, table, msg, ConfigurationException, cmd)
-
-def test_common_options(cql):
-    assert_throws(cql, "tombstone_threshold value (-0.4) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'tombstone_threshold' : -0.4 }")
-    assert_throws(cql, "tombstone_threshold value (5.5) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'tombstone_threshold' : 5.5 }")
-    assert_throws(cql, "tombstone_compaction_interval value (-7000ms) must be positive", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'tombstone_compaction_interval' : -7 }")
-
-def test_size_tiered_compaction_strategy_options(cql):
-    assert_throws(cql, "min_sstable_size value (-1) must be non negative", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_sstable_size' : -1 }")
-    assert_throws(cql, "bucket_low value (0) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_low' : 0.0 }")
-    assert_throws(cql, "bucket_low value (1.3) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_low' : 1.3 }")
-    assert_throws(cql, "bucket_high value (0.7) must be greater than 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_high' : 0.7 }")
-    assert_throws(cql, "cold_reads_to_omit value (-8.1) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'cold_reads_to_omit' : -8.1 }")
-    assert_throws(cql, "cold_reads_to_omit value (3.5) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'cold_reads_to_omit' : 3.5 }")
-    assert_throws(cql, "min_threshold value (1) must be bigger or equal to 2", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_threshold' : 1 }")
-
-def test_time_window_compaction_strategy_options(cql):
-    assert_throws(cql, "Invalid window unit SECONDS for compaction_window_unit", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'compaction_window_unit' : 'SECONDS' }")
-    assert_throws(cql, "compaction_window_size value (-8) must be greater than 1", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'compaction_window_size' : -8 }")
-    assert_throws(cql, "Invalid timestamp resolution SECONDS for timestamp_resolution", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'timestamp_resolution' : 'SECONDS' }")
-    assert_throws(cql, "enable_optimized_twcs_queries value (no) must be \"true\" or \"false\"", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'enable_optimized_twcs_queries' : 'no' }")
-    assert_throws(cql, "max_threshold value (1) must be bigger or equal to 2", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'max_threshold' : 1 }")
-
-def test_leveled_compaction_strategy_options(cql):
-    assert_throws(cql, "sstable_size_in_mb value (-5) must be positive", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'sstable_size_in_mb' : -5 }")
-
-def test_not_allowed_options(cql):
-    assert_throws(cql, "Invalid compaction strategy options {{abc, -54.54}} for chosen strategy type", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'abc' : -54.54 }")
-    assert_throws(cql, "Invalid compaction strategy options {{dog, 3}} for chosen strategy type", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'dog' : 3 }")
-    assert_throws(cql, "Invalid compaction strategy options {{compaction_window_size, 4}} for chosen strategy type", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'compaction_window_size' : 4 }")
+import pytest
+from util import new_test_table
+from cassandra.protocol import ConfigurationException
+
+@pytest.fixture(scope="module")
+def table1(cql, test_keyspace):
+    with new_test_table(cql, test_keyspace, "a int PRIMARY KEY, b int", "WITH compaction = { 'class' : 'SizeTieredCompactionStrategy' }") as table:
+        yield table
+
+# NOTE: The following tests which use this assert_throws() all try to 
+# check the specific wording of the error text, and it sometimes differs
+# between Scylla and Cassandra - so we need to allow both: msg is a regular
+# expression, so you can use the "|" character to allow two options.
+def assert_throws(cql, table1, msg, cmd):
+    with pytest.raises(ConfigurationException, match=msg):
+        cql.execute(cmd.replace('%s', table1))
+
+def test_common_options(cql, table1):
+    assert_throws(cql, table1, r"tombstone_threshold value \(-0.4\) must be between 0.0 and 1.0|tombstone_threshold must be greater than 0, but was -0.400000", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'tombstone_threshold' : -0.4 }")
+    assert_throws(cql, table1, r"tombstone_threshold value \(5.5\) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'tombstone_threshold' : 5.5 }")
+    assert_throws(cql, table1, r"tombstone_compaction_interval value \(-7000ms\) must be positive", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'tombstone_compaction_interval' : -7 }")
+
+def test_size_tiered_compaction_strategy_options(cql, table1):
+    assert_throws(cql, table1, r"min_sstable_size value \(-1\) must be non negative|min_sstable_size must be non negative: -1", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_sstable_size' : -1 }")
+    assert_throws(cql, table1, r"bucket_low value \(0\) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_low' : 0.0 }")
+    assert_throws(cql, table1, r"bucket_low value \(1.3\) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_low' : 1.3 }")
+    assert_throws(cql, table1, r"bucket_high value \(0.7\) must be greater than 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_high' : 0.7 }")
+    assert_throws(cql, table1, r"cold_reads_to_omit value \(-8.1\) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'cold_reads_to_omit' : -8.1 }")
+    assert_throws(cql, table1, r"cold_reads_to_omit value \(3.5\) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'cold_reads_to_omit' : 3.5 }")
+    assert_throws(cql, table1, r"min_threshold value \(1\) must be bigger or equal to 2", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_threshold' : 1 }")
+
+def test_time_window_compaction_strategy_options(cql, table1):
+    assert_throws(cql, table1, "Invalid window unit SECONDS for compaction_window_unit|SECONDS is not valid for compaction_window_unit", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'compaction_window_unit' : 'SECONDS' }")
+    assert_throws(cql, table1, r"compaction_window_size value \(-8\) must be greater than 1|-8 must be greater than 1 for compaction_window_size", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'compaction_window_size' : -8 }")
+    assert_throws(cql, table1, "Invalid timestamp resolution SECONDS for timestamp_resolution", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'timestamp_resolution' : 'SECONDS' }")
+    assert_throws(cql, table1, r"enable_optimized_twcs_queries value \(no\) must be \"true\" or \"false\"", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'enable_optimized_twcs_queries' : 'no' }")
+    assert_throws(cql, table1, r"max_threshold value \(1\) must be bigger or equal to 2", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'max_threshold' : 1 }")
+
+def test_leveled_compaction_strategy_options(cql, table1):
+    assert_throws(cql, table1, r"sstable_size_in_mb value \(-5\) must be positive|sstable_size_in_mb must be larger than 0, but was -5", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'sstable_size_in_mb' : -5 }")
+
+def test_not_allowed_options(cql, table1):
+    assert_throws(cql, table1, r"Invalid compaction strategy options {{abc, -54.54}} for chosen strategy type|Properties specified \[abc\] are not understood by SizeTieredCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'abc' : -54.54 }")
+    assert_throws(cql, table1, r"Invalid compaction strategy options {{dog, 3}} for chosen strategy type|Properties specified \[dog\] are not understood by TimeWindowCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'dog' : 3 }")
+    assert_throws(cql, table1, r"Invalid compaction strategy options {{compaction_window_size, 4}} for chosen strategy type|Properties specified \[compaction_window_size\] are not understood by LeveledCompactionStrategy", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'compaction_window_size' : 4 }")

test/cql-pytest/test_describe.py

@@ -402,43 +402,68 @@ def test_desc_cluster(scylla_only, cql, test_keyspace):
 
     assert sorted(desc_endpoints) == sorted(ring_endpoints)
 
+def is_scylla(cql):
+    return any('scylla' in name for name in [row.table_name for row in cql.execute("SELECT * FROM system_schema.tables WHERE keyspace_name = 'system'")])
+
 # Test that 'DESC INDEX' contains create statement od index
 def test_desc_index(cql, test_keyspace):
     with new_test_table(cql, test_keyspace, "a int primary key, b int, c int") as tbl:
         tbl_name = get_name(tbl)
         create_idx_b = f"CREATE INDEX ON {tbl}(b)"
         create_idx_c = f"CREATE INDEX named_index ON {tbl}(c)"
-        create_idx_ab = f"CREATE INDEX ON {tbl}((a), b)"
+        # Only Scylla supports local indexes
+        has_local = is_scylla(cql)
+        if has_local:
+            create_idx_ab = f"CREATE INDEX ON {tbl}((a), b)"
 
         cql.execute(create_idx_b)
         cql.execute(create_idx_c)
-        cql.execute(create_idx_ab)
+        if has_local:
+            cql.execute(create_idx_ab)
 
         b_desc = cql.execute(f"DESC INDEX {test_keyspace}.{tbl_name}_b_idx").one().create_statement
-        ab_desc = cql.execute(f"DESC INDEX {test_keyspace}.{tbl_name}_b_idx_1").one().create_statement
+        if has_local:
+            ab_desc = cql.execute(f"DESC INDEX {test_keyspace}.{tbl_name}_b_idx_1").one().create_statement
         c_desc = cql.execute(f"DESC INDEX {test_keyspace}.named_index").one().create_statement
 
-        assert f"CREATE INDEX {tbl_name}_b_idx ON {tbl}(b)" in b_desc
-        assert f"CREATE INDEX {tbl_name}_b_idx_1 ON {tbl}((a), b)" in ab_desc
-        assert create_idx_c in c_desc
+        # Cassandra inserts a space between the table name and parantheses,
+        # Scylla doesn't. This difference doesn't matter because both are
+        # valid CQL commands
+        if is_scylla(cql):
+            maybe_space = ''
+        else:
+            maybe_space = ' '
+        assert f"CREATE INDEX {tbl_name}_b_idx ON {tbl}{maybe_space}(b)" in b_desc
+        if has_local:
+            assert f"CREATE INDEX {tbl_name}_b_idx_1 ON {tbl}((a), b)" in ab_desc
+
+        assert f"CREATE INDEX named_index ON {tbl}{maybe_space}(c)" in c_desc
 
 # Test that 'DESC TABLE' contains description of indexes
 def test_index_desc_in_table_desc(cql, test_keyspace):
     with new_test_table(cql, test_keyspace, "a int primary key, b int, c int") as tbl:
         create_idx_b = f"CREATE INDEX ON {tbl}(b)"
         create_idx_c = f"CREATE INDEX named_index ON {tbl}(c)"
-        create_idx_ab = f"CREATE INDEX ON {tbl}((a), b)"
+        has_local = is_scylla(cql)
+        if is_scylla(cql):
+            maybe_space = ''
+        else:
+            maybe_space = ' '
+        if has_local:
+            create_idx_ab = f"CREATE INDEX ON {tbl}((a), b)"
 
         cql.execute(create_idx_b)
         cql.execute(create_idx_c)
-        cql.execute(create_idx_ab)
+        if has_local:
+            cql.execute(create_idx_ab)
 
         tbl_name = get_name(tbl)
         desc = "\n".join([d.create_statement for d in cql.execute(f"DESC TABLE {tbl}")])
 
-        assert f"CREATE INDEX {tbl_name}_b_idx ON {tbl}(b)" in desc
-        assert create_idx_c in desc
-        assert f"CREATE INDEX {tbl_name}_b_idx_1 ON {tbl}((a), b)" in desc
+        assert f"CREATE INDEX {tbl_name}_b_idx ON {tbl}{maybe_space}(b)" in desc
+        assert f"CREATE INDEX named_index ON {tbl}{maybe_space}(c)" in desc
+        if has_local:
+            assert f"CREATE INDEX {tbl_name}_b_idx_1 ON {tbl}((a), b)" in desc
 
 # -----------------------------------------------------------------------------
 # "Generic describe" is a describe statement without specifying what kind of object 
@@ -492,7 +517,8 @@ def test_generic_desc_user_defined(scylla_only, cql, test_keyspace):
 
 
 # Test that 'DESC FUNCTION'/'DESC AGGREGATE' doesn't show UDA/UDF and doesn't crash Scylla
-def test_desc_udf_uda(cql, test_keyspace):
+# The test is marked scylla_only because it uses Lua as UDF language.
+def test_desc_udf_uda(cql, test_keyspace, scylla_only):
     with new_function(cql, test_keyspace, "(a int, b int) RETURNS NULL ON NULL INPUT RETURNS int LANGUAGE LUA AS 'return a+b'") as fn:
         with new_aggregate(cql, test_keyspace, f"(int) SFUNC {fn} STYPE int") as aggr:
 
@@ -507,7 +533,8 @@ def test_desc_udf_uda(cql, test_keyspace):
 # The new server-side describe was missing it.
 # Example: caching = {'keys': 'ALL', 'rows_per_partition': 'ALL'}
 # Reproduces #14895
-def test_whitespaces_in_table_options(cql, test_keyspace):
+# The test is marked scylla_only because it uses a Scylla-only property "cdc".
+def test_whitespaces_in_table_options(cql, test_keyspace, scylla_only):
     regex = "\\{[^}]*[:,][^\\s][^}]*\\}" # looks for any colon or comma without space after it inside a { }
 
     with new_test_table(cql, test_keyspace, "a int primary key", "WITH cdc = {'enabled': true}") as tbl:
@@ -591,6 +618,8 @@ def test_type_quoting(cql):
         assert f"CREATE TYPE \"{ks_name}\".\"{name}\"" in desc
         assert f"CREATE TYPE \"{ks_name}\".{name}" not in desc
         assert f"CREATE TYPE {ks_name}.\"{name}\"" not in desc
+        # The following used to be buggy in Cassandra, but fixed in
+        # CASSANDRA-17918 (in Cassandra 4.1.2):
         assert f"\"{field_name}\" text" in desc
         assert f"{field_name} text" not in desc
 
@@ -873,9 +902,11 @@ def new_random_table(cql, keyspace, udts=[]):
     extras["min_index_interval"] = min_idx_interval
     extras["max_index_interval"] = random.randrange(min_idx_interval, 10000)
     extras["memtable_flush_period_in_ms"] = random.randrange(0, 10000)
-    extras["paxos_grace_seconds"] = random.randrange(1000, 100000)
 
-    extras["tombstone_gc"] = f"{{'mode': 'timeout', 'propagation_delay_in_seconds': '{random.randrange(100, 100000)}'}}"
+    if is_scylla(cql):
+        # Extra properties which ScyllaDB supports but Cassandra doesn't
+        extras["paxos_grace_seconds"] = random.randrange(1000, 100000)
+        extras["tombstone_gc"] = f"{{'mode': 'timeout', 'propagation_delay_in_seconds': '{random.randrange(100, 100000)}'}}"
 
     extra_options = [f"{k} = {v}" for (k, v) in extras.items()]
     extra_str = " AND ".join(extra_options)