diff --git a/src/backend/distributed/connection/placement_connection.c b/src/backend/distributed/connection/placement_connection.c index b9e2e4672..f7e2c70f7 100644 --- a/src/backend/distributed/connection/placement_connection.c +++ b/src/backend/distributed/connection/placement_connection.c @@ -627,6 +627,24 @@ FindPlacementListConnection(int flags, List *placementAccessList, const char *us foundModifyingConnection = true; } } + else if (placementConnection->hadDDL || placementConnection->hadDML) + { + if (strcmp(placementConnection->userName, userName) != 0) + { + ereport(ERROR, + (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION), + errmsg("cannot perform query on placements that were " + "modified in this transaction by a different " + "user"))); + } + ereport(ERROR, + (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION), + errmsg("cannot perform query, because modifications were " + "made over a connection that cannot be used at " + "this time. This is most likely a Citus bug so " + "please report it" + ))); + } } return chosenConnection; diff --git a/src/test/regress/expected/set_role_in_transaction.out b/src/test/regress/expected/set_role_in_transaction.out new file mode 100644 index 000000000..c37d5322c --- /dev/null +++ b/src/test/regress/expected/set_role_in_transaction.out @@ -0,0 +1,114 @@ +-- Regression test for this issue: +-- https://github.com/citusdata/citus/issues/3867 +SET citus.shard_count = 4; +SET citus.next_shard_id TO 1959300; +CREATE SCHEMA set_role_in_transaction; +SET search_path TO set_role_in_transaction; +CREATE TABLE t(a int); +SELECT create_distributed_table('t', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- hide messages only visible in Citus Opensource +SET client_min_messages TO WARNING; +CREATE USER user1; +CREATE USER user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('CREATE USER user1'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +SELECT count(*) FROM run_command_on_workers('CREATE USER user2'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +GRANT ALL ON SCHEMA set_role_in_transaction TO user1; +GRANT ALL ON SCHEMA set_role_in_transaction TO user2; +GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user1; +GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user1'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +SELECT count(*) FROM run_command_on_workers('GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user2'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +RESET client_min_messages; +-- Visibility bugs +SET ROLE user1; +SET search_path TO set_role_in_transaction; +BEGIN; +INSERT INTO t values (1); +SELECT * FROM t; -- 1 is visible as expected + a +--------------------------------------------------------------------- + 1 +(1 row) + +SET ROLE user2; +SET search_path TO set_role_in_transaction; +SELECT * FROM t; -- 1 is not visible +ERROR: cannot perform query on placements that were modified in this transaction by a different user +INSERT INTO t values (1); -- assert failure +ERROR: current transaction is aborted, commands ignored until end of transaction block +SELECT * from t; +ERROR: current transaction is aborted, commands ignored until end of transaction block +ROLLBACK; +-- Self deadlock +SET ROLE user1; +SET search_path TO set_role_in_transaction; +BEGIN; +TRUNCATE t; +SET ROLE user2; +SET search_path TO set_role_in_transaction; +-- assert failure or self deadlock (it is detected by deadlock detector) +INSERT INTO t values (2); +ERROR: cannot perform query on placements that were modified in this transaction by a different user +ROLLBACK; +RESET ROLE; +REVOKE ALL ON SCHEMA set_role_in_transaction FROM user1; +REVOKE ALL ON SCHEMA set_role_in_transaction FROM user2; +REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user1; +REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user1'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +SELECT count(*) FROM run_command_on_workers('REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user2'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +DROP USER user1; +DROP USER user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('DROP USER user1'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +SELECT count(*) FROM run_command_on_workers('DROP USER user2'); + count +--------------------------------------------------------------------- + 2 +(1 row) + +DROP SCHEMA set_role_in_transaction CASCADE; +NOTICE: drop cascades to table t diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 0b327b394..860afb65d 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -91,7 +91,7 @@ test: multi_subquery_complex_reference_clause multi_subquery_window_functions mu test: sql_procedure multi_function_in_join row_types materialized_view test: multi_subquery_in_where_reference_clause full_join adaptive_executor propagate_set_commands test: multi_subquery_union multi_subquery_in_where_clause multi_subquery_misc -test: multi_agg_distinct multi_agg_approximate_distinct multi_limit_clause_approximate multi_outer_join_reference multi_single_relation_subquery multi_prepare_plsql +test: multi_agg_distinct multi_agg_approximate_distinct multi_limit_clause_approximate multi_outer_join_reference multi_single_relation_subquery multi_prepare_plsql set_role_in_transaction test: multi_reference_table multi_select_for_update relation_access_tracking test: custom_aggregate_support aggregate_support test: multi_average_expression multi_working_columns multi_having_pushdown having_subquery diff --git a/src/test/regress/multi_schedule_hyperscale b/src/test/regress/multi_schedule_hyperscale index fb102f868..3898fbbbc 100644 --- a/src/test/regress/multi_schedule_hyperscale +++ b/src/test/regress/multi_schedule_hyperscale @@ -65,7 +65,7 @@ test: multi_subquery_complex_reference_clause multi_subquery_window_functions mu test: multi_function_in_join row_types test: multi_subquery_in_where_reference_clause full_join adaptive_executor propagate_set_commands test: multi_subquery_union multi_subquery_in_where_clause multi_subquery_misc -test: multi_limit_clause_approximate multi_single_relation_subquery +test: multi_limit_clause_approximate multi_single_relation_subquery set_role_in_transaction test: multi_select_for_update test: multi_average_expression multi_working_columns multi_having_pushdown test: multi_array_agg diff --git a/src/test/regress/sql/set_role_in_transaction.sql b/src/test/regress/sql/set_role_in_transaction.sql new file mode 100644 index 000000000..c4dc9cee5 --- /dev/null +++ b/src/test/regress/sql/set_role_in_transaction.sql @@ -0,0 +1,67 @@ +-- Regression test for this issue: +-- https://github.com/citusdata/citus/issues/3867 +SET citus.shard_count = 4; +SET citus.next_shard_id TO 1959300; +CREATE SCHEMA set_role_in_transaction; +SET search_path TO set_role_in_transaction; +CREATE TABLE t(a int); +SELECT create_distributed_table('t', 'a'); + +-- hide messages only visible in Citus Opensource +SET client_min_messages TO WARNING; +CREATE USER user1; +CREATE USER user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('CREATE USER user1'); +SELECT count(*) FROM run_command_on_workers('CREATE USER user2'); + +GRANT ALL ON SCHEMA set_role_in_transaction TO user1; +GRANT ALL ON SCHEMA set_role_in_transaction TO user2; +GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user1; +GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user1'); +SELECT count(*) FROM run_command_on_workers('GRANT ALL ON ALL TABLES IN SCHEMA set_role_in_transaction TO user2'); +RESET client_min_messages; + +-- Visibility bugs +SET ROLE user1; +SET search_path TO set_role_in_transaction; +BEGIN; +INSERT INTO t values (1); +SELECT * FROM t; -- 1 is visible as expected +SET ROLE user2; +SET search_path TO set_role_in_transaction; +SELECT * FROM t; -- 1 is not visible +INSERT INTO t values (1); -- assert failure +SELECT * from t; +ROLLBACK; + +-- Self deadlock +SET ROLE user1; +SET search_path TO set_role_in_transaction; +BEGIN; +TRUNCATE t; +SET ROLE user2; +SET search_path TO set_role_in_transaction; +-- assert failure or self deadlock (it is detected by deadlock detector) +INSERT INTO t values (2); +ROLLBACK; + +RESET ROLE; + +REVOKE ALL ON SCHEMA set_role_in_transaction FROM user1; +REVOKE ALL ON SCHEMA set_role_in_transaction FROM user2; +REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user1; +REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user1'); +SELECT count(*) FROM run_command_on_workers('REVOKE ALL ON ALL TABLES IN SCHEMA set_role_in_transaction FROM user2'); + +DROP USER user1; +DROP USER user2; +-- Not needed on Citus Enterprise, so using count(*) for same output +SELECT count(*) FROM run_command_on_workers('DROP USER user1'); +SELECT count(*) FROM run_command_on_workers('DROP USER user2'); + +DROP SCHEMA set_role_in_transaction CASCADE;