From 6aae14c11ff7967c67d7fe69cfe27e348e3b0ce4 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Tue, 22 Sep 2020 11:24:56 +0200 Subject: [PATCH] Allow owner change of continuous aggregate Implements support for changing the owner of the continuous aggregate. This will change the owner of the materialized hypertable, the user view, the partial view, and the direct view. Fixes #2414 Fixes #1277 --- src/process_utility.c | 60 ++++++++++++++++++----- tsl/test/expected/continuous_aggs_ddl.out | 53 ++++++++++++++++++++ tsl/test/sql/continuous_aggs_ddl.sql | 38 +++++++++++++- 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/src/process_utility.c b/src/process_utility.c index 0f64f15ec..3c95b90a0 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -2780,6 +2780,37 @@ process_altercontinuousagg_set_with(ContinuousAgg *cagg, Oid view_relid, const L } } +/* Run an alter table command on a relation */ +static void +alter_table_by_relation(RangeVar *relation, AlterTableCmd *cmd) +{ + const Oid relid = RangeVarGetRelid(relation, NoLock, true); + AlterTableInternal(relid, list_make1(cmd), false); +} + +/* Run an alter table command on a relation given by name */ +static void +alter_table_by_name(Name schema_name, Name table_name, AlterTableCmd *cmd) +{ + alter_table_by_relation(makeRangeVar(NameStr(*schema_name), NameStr(*table_name), -1), cmd); +} + +/* Alter a hypertable and do some extra processing */ +static void +alter_hypertable_by_id(int32 hypertable_id, AlterTableStmt *stmt, AlterTableCmd *cmd, + void (*extra)(Hypertable *, AlterTableCmd *)) +{ + Cache *hcache = ts_hypertable_cache_pin(); + Hypertable *ht = ts_hypertable_cache_get_entry_by_id(hcache, hypertable_id); + Assert(ht); /* Broken continuous aggregate */ + ts_hypertable_permissions_check_by_id(ht->fd.id); + check_alter_table_allowed_on_ht_with_compression(ht, stmt); + relation_not_only(stmt->relation); + AlterTableInternal(ht->main_table_relid, list_make1(cmd), false); + (*extra)(ht, cmd); + ts_cache_release(hcache); +} + static DDLResult process_altertable_start_matview(ProcessUtilityArgs *args) { @@ -2787,8 +2818,6 @@ process_altertable_start_matview(ProcessUtilityArgs *args) const Oid view_relid = RangeVarGetRelid(stmt->relation, NoLock, true); ContinuousAgg *cagg; ListCell *lc; - Hypertable *ht; - Cache *hcache; if (!OidIsValid(view_relid)) return DDL_CONTINUE; @@ -2814,16 +2843,25 @@ process_altertable_start_matview(ProcessUtilityArgs *args) process_altercontinuousagg_set_with(cagg, view_relid, (List *) cmd->def); break; + case AT_ChangeOwner: + alter_table_by_relation(stmt->relation, cmd); + alter_table_by_name(&cagg->data.partial_view_schema, + &cagg->data.partial_view_name, + cmd); + alter_table_by_name(&cagg->data.direct_view_schema, + &cagg->data.direct_view_name, + cmd); + alter_hypertable_by_id(cagg->data.mat_hypertable_id, + stmt, + cmd, + process_altertable_change_owner); + break; + case AT_SetTableSpace: - hcache = ts_hypertable_cache_pin(); - ht = ts_hypertable_cache_get_entry_by_id(hcache, cagg->data.mat_hypertable_id); - Assert(ht); /* Broken continuous aggregate */ - ts_hypertable_permissions_check_by_id(ht->fd.id); - check_alter_table_allowed_on_ht_with_compression(ht, stmt); - relation_not_only(stmt->relation); - process_altertable_set_tablespace_end(ht, cmd); - AlterTableInternal(ht->main_table_relid, list_make1(cmd), false); - ts_cache_release(hcache); + alter_hypertable_by_id(cagg->data.mat_hypertable_id, + stmt, + cmd, + process_altertable_set_tablespace_end); break; default: diff --git a/tsl/test/expected/continuous_aggs_ddl.out b/tsl/test/expected/continuous_aggs_ddl.out index 209c3f05f..acaff728e 100644 --- a/tsl/test/expected/continuous_aggs_ddl.out +++ b/tsl/test/expected/continuous_aggs_ddl.out @@ -869,15 +869,24 @@ CREATE VIEW cagg_info AS WITH caggs AS ( SELECT format('%s.%s', user_view_schema, user_view_name)::regclass AS user_view, + format('%s.%s', direct_view_schema, direct_view_name)::regclass AS direct_view, + format('%s.%s', partial_view_schema, partial_view_name)::regclass AS partial_view, format('%s.%s', ht.schema_name, ht.table_name)::regclass AS mat_relid FROM _timescaledb_catalog.hypertable ht, _timescaledb_catalog.continuous_agg cagg WHERE ht.id = cagg.mat_hypertable_id ) SELECT user_view, + pg_get_userbyid(relowner) AS user_view_owner, relname AS mat_table, + (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = mat_relid) AS mat_table_owner, + direct_view, + (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = direct_view) AS direct_view_owner, + partial_view, + (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = partial_view) AS partial_view_owner, (SELECT spcname FROM pg_tablespace WHERE oid = reltablespace) AS tablespace FROM pg_class JOIN caggs ON pg_class.oid = caggs.mat_relid; +GRANT SELECT ON cagg_info TO PUBLIC; CREATE VIEW chunk_info AS SELECT ht.schema_name, ht.table_name, relname AS chunk_name, (SELECT spcname FROM pg_tablespace WHERE oid = reltablespace) AS tablespace @@ -1061,5 +1070,49 @@ FROM metrics_int8 GROUP BY 1; ERROR: first argument to time_bucket function should be an immutable expression for continuous aggregate query \set ON_ERROR_STOP 1 +-- Test various ALTER MATERIALIZED VIEW statements. +SET ROLE :ROLE_DEFAULT_PERM_USER; +CREATE MATERIALIZED VIEW owner_check WITH (timescaledb.continuous) AS +SELECT time_bucket(1 + 2, time) +FROM metrics_int8 +GROUP BY 1 +WITH NO DATA; +\x on +SELECT * FROM cagg_info WHERE user_view::text = 'owner_check'; +-[ RECORD 1 ]------+--------------------------------------- +user_view | owner_check +user_view_owner | default_perm_user +mat_table | _materialized_hypertable_24 +mat_table_owner | default_perm_user +direct_view | _timescaledb_internal._direct_view_24 +direct_view_owner | default_perm_user +partial_view | _timescaledb_internal._partial_view_24 +partial_view_owner | default_perm_user +tablespace | + +\x off +-- This should not work since the target user has the wrong role, but +-- we test that the normal checks are done when changing the owner. +\set ON_ERROR_STOP 0 +ALTER MATERIALIZED VIEW owner_check OWNER TO :ROLE_1; +ERROR: must be member of role "test_role_1" +\set ON_ERROR_STOP 1 +-- Superuser can always change owner +SET ROLE :ROLE_SUPERUSER; +ALTER MATERIALIZED VIEW owner_check OWNER TO :ROLE_1; +\x on +SELECT * FROM cagg_info WHERE user_view::text = 'owner_check'; +-[ RECORD 1 ]------+--------------------------------------- +user_view | owner_check +user_view_owner | test_role_1 +mat_table | _materialized_hypertable_24 +mat_table_owner | test_role_1 +direct_view | _timescaledb_internal._direct_view_24 +direct_view_owner | test_role_1 +partial_view | _timescaledb_internal._partial_view_24 +partial_view_owner | test_role_1 +tablespace | + +\x off DROP TABLESPACE tablespace1; DROP TABLESPACE tablespace2; diff --git a/tsl/test/sql/continuous_aggs_ddl.sql b/tsl/test/sql/continuous_aggs_ddl.sql index 3a9b995fc..fa1bb879e 100644 --- a/tsl/test/sql/continuous_aggs_ddl.sql +++ b/tsl/test/sql/continuous_aggs_ddl.sql @@ -512,15 +512,24 @@ CREATE VIEW cagg_info AS WITH caggs AS ( SELECT format('%s.%s', user_view_schema, user_view_name)::regclass AS user_view, + format('%s.%s', direct_view_schema, direct_view_name)::regclass AS direct_view, + format('%s.%s', partial_view_schema, partial_view_name)::regclass AS partial_view, format('%s.%s', ht.schema_name, ht.table_name)::regclass AS mat_relid FROM _timescaledb_catalog.hypertable ht, _timescaledb_catalog.continuous_agg cagg WHERE ht.id = cagg.mat_hypertable_id ) SELECT user_view, + pg_get_userbyid(relowner) AS user_view_owner, relname AS mat_table, + (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = mat_relid) AS mat_table_owner, + direct_view, + (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = direct_view) AS direct_view_owner, + partial_view, + (SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = partial_view) AS partial_view_owner, (SELECT spcname FROM pg_tablespace WHERE oid = reltablespace) AS tablespace FROM pg_class JOIN caggs ON pg_class.oid = caggs.mat_relid; +GRANT SELECT ON cagg_info TO PUBLIC; CREATE VIEW chunk_info AS SELECT ht.schema_name, ht.table_name, relname AS chunk_name, @@ -695,8 +704,35 @@ CREATE MATERIALIZED VIEW width_expr WITH (timescaledb.continuous) AS SELECT time_bucket(extract(year FROM now())::int, time) FROM metrics_int8 GROUP BY 1; - \set ON_ERROR_STOP 1 +-- Test various ALTER MATERIALIZED VIEW statements. + +SET ROLE :ROLE_DEFAULT_PERM_USER; + +CREATE MATERIALIZED VIEW owner_check WITH (timescaledb.continuous) AS +SELECT time_bucket(1 + 2, time) +FROM metrics_int8 +GROUP BY 1 +WITH NO DATA; + +\x on +SELECT * FROM cagg_info WHERE user_view::text = 'owner_check'; +\x off + +-- This should not work since the target user has the wrong role, but +-- we test that the normal checks are done when changing the owner. +\set ON_ERROR_STOP 0 +ALTER MATERIALIZED VIEW owner_check OWNER TO :ROLE_1; +\set ON_ERROR_STOP 1 + +-- Superuser can always change owner +SET ROLE :ROLE_SUPERUSER; +ALTER MATERIALIZED VIEW owner_check OWNER TO :ROLE_1; + +\x on +SELECT * FROM cagg_info WHERE user_view::text = 'owner_check'; +\x off + DROP TABLESPACE tablespace1; DROP TABLESPACE tablespace2;