From 5e33d9346bfa1823868074093ec3dc9f5defbabf Mon Sep 17 00:00:00 2001 From: Eliran Sinvani Date: Sun, 21 Jan 2024 11:32:26 +0200 Subject: [PATCH 1/2] query processor: treat view changes at least as table changes When a base table changes and altered, so does the views that might refer to the added column (which includes "SELECT *" views and also views that might need to use this column for rows lifetime (virtual columns). However the query processor implementation for views change notification was an empty function. Since views are tables, the query processor needs to at least treat them as such (and maybe in the future, do also some MV specific stuff). This commit adds a call to `on_update_column_family` from within `on_update_view`. The side effect true to this date is that prepared statements for views which changed due to a base table change will be invalidated. Fixes #16392 Signed-off-by: Eliran Sinvani --- cql3/query_processor.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cql3/query_processor.cc b/cql3/query_processor.cc index a38fa133ca..0b8d9b6e0b 100644 --- a/cql3/query_processor.cc +++ b/cql3/query_processor.cc @@ -1084,6 +1084,9 @@ void query_processor::migration_subscriber::on_update_aggregate(const sstring& k void query_processor::migration_subscriber::on_update_view( const sstring& ks_name, const sstring& view_name, bool columns_changed) { + // scylladb/scylladb#16392 - Materialized views are also tables so we need at least handle + // them as such when changed. + on_update_column_family(ks_name, view_name, columns_changed); } void query_processor::migration_subscriber::on_update_tablet_metadata() { From 0e5a8cad627ccc8c0a4e6a7f87216fd15504eb0f Mon Sep 17 00:00:00 2001 From: Eliran Sinvani Date: Wed, 17 Jan 2024 19:24:28 +0200 Subject: [PATCH 2/2] Add test for mv prepared statements invalidation on base alter Issue #16392 describes a bug where when a base table is altered, it's materialized views prepared statements are not invalidated which in turn causes them to return missing data. This test reproduces this bug and serves as a regression test for this problem. Signed-off-by: Eliran Sinvani --- test/cql-pytest/test_materialized_view.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/cql-pytest/test_materialized_view.py b/test/cql-pytest/test_materialized_view.py index 202ef834a6..195c778ca5 100644 --- a/test/cql-pytest/test_materialized_view.py +++ b/test/cql-pytest/test_materialized_view.py @@ -808,3 +808,17 @@ def test_mv_with_only_primary_key_rows(scylla_only, cql, test_keyspace): nodetool.flush(cql, view) assert(set([row.id for row in cql.execute(f'SELECT id FROM {view}')]) == set([1, 2, 3])) # We now believe that empty value serialization/deserialization is correct + +# This test is regression testing added after fixing: +# https://github.com/scylladb/scylladb/issues/16392 - the gist of the issue is that +# prepared statements on views are not invalidated when the base table changes. +def test_mv_prepared_statement_with_altered_base(cql, test_keyspace): + with new_test_table(cql, test_keyspace, 'id int PRIMARY KEY, v1 int') as base: + with new_materialized_view(cql, table=base, select='*', pk='id', where='id IS NOT NULL') as view: + base_query = cql.prepare(f"SELECT * FROM {base} WHERE id=?") + view_query = cql.prepare(f"SELECT * FROM {view} WHERE id=?") + cql.execute(f"INSERT INTO {base} (id,v1) VALUES (0,0)") + assert cql.execute(base_query,[0]) == cql.execute(view_query,[0]) + cql.execute(f"ALTER TABLE {base} ADD (v2 int)") + cql.execute(f"INSERT INTO {base} (id,v1,v2) VALUES (1,1,1)") + assert list(cql.execute(base_query,[1])) == list(cql.execute(view_query,[1])) \ No newline at end of file