From 2f2f01b045311b26c102c17206b0bcd9338a823a Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Mon, 31 Oct 2022 17:17:25 +0200 Subject: [PATCH] materialized views: fix view writes after base table schema change When we write to a materialized view, we need to know some information defined in the base table such as the columns in its schema. We have a "view_info" object that tracks each view and its base. This view_info object has a couple of mutable attributes which are used to lazily-calculate and cache the SELECT statement needed to read from the base table. If the base-table schema ever changes - and the code calls set_base_info() at that point - we need to forget this cached statement. If we don't (as before this patch), the SELECT will use the wrong schema and writes will no longer work. This patch also includes a reproducing test that failed before this patch, and passes afterwords. The test creates a base table with a view that has a non-trivial SELECT (it has a filter on one of the base-regular columns), makes a benign modification to the base table (just a silly addition of a comment), and then tries to write to the view - and before this patch it fails. Fixes #10026 Fixes #11542 --- db/view/view.cc | 3 +++ test/cql-pytest/test_materialized_view.py | 25 +++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/db/view/view.cc b/db/view/view.cc index fda3d708f3..8c08b8393f 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -128,6 +128,9 @@ const column_definition* view_info::view_column(const column_definition& base_de void view_info::set_base_info(db::view::base_info_ptr base_info) { _base_info = std::move(base_info); + // Forget the cached objects which may refer to the base schema. + _select_statement = nullptr; + _partition_slice = std::nullopt; } // A constructor for a base info that can facilitate reads and writes from the materialized view. diff --git a/test/cql-pytest/test_materialized_view.py b/test/cql-pytest/test_materialized_view.py index bc9f75148d..6a80215bfa 100644 --- a/test/cql-pytest/test_materialized_view.py +++ b/test/cql-pytest/test_materialized_view.py @@ -495,3 +495,28 @@ def test_is_not_null_requirement(cql, test_keyspace): with pytest.raises(InvalidRequest, match="IS NOT NULL"): with new_materialized_view(cql, table, select='*', pk='p1,p2,c1,c2,v', where='p1 is not null and p2 is not null and c1 is not null and c2 is not null') as mv: pass + +# Reproducer for issue #11542 and #10026: We have a table with with a +# materialized view with a filter and some data, at which point we modify +# the base table (e.g., add some silly comment) and then try to modify the +# data. The last modification used to fail, logging "Column definition v +# does not match any column in the query selection". +# The same test without the silly base-table modification works, and so does +# the same test without the filter in the materialized view that uses the +# base-regular column v. So does the same test without pre-modification data. +# +# This test is Scylla-only because Cassandra does not support filtering +# on a base-regular column v that is only a key column in the view. +def test_view_update_and_alter_base(cql, test_keyspace, scylla_only): + with new_test_table(cql, test_keyspace, 'p int primary key, v int') as table: + with new_materialized_view(cql, table, '*', 'v, p', 'v >= 0 and p is not null') as mv: + cql.execute(f'INSERT INTO {table} (p,v) VALUES (1,1)') + # In our tests, MV writes are synchronous, so we can read + # immediately + assert len(list(cql.execute(f"SELECT v from {mv}"))) == 1 + # Alter the base table, with a silly comment change that doesn't + # change anything important - but still the base schema changes. + cql.execute(f"ALTER TABLE {table} WITH COMMENT = '{unique_name()}'") + # Try to modify an item. This failed in #11542. + cql.execute(f'UPDATE {table} SET v=-1 WHERE p=1') + assert len(list(cql.execute(f"SELECT v from {mv}"))) == 0