Error out when using different users in the same transaction (#3869)

Fixes #3867

As described in the issue above we return incorrect results when
changing user within a transaction. This causes us to error out instead.
pull/3896/head
Jelte Fennema 2020-06-10 14:07:40 +02:00 committed by GitHub
parent 02a70df656
commit b87bae71bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 201 additions and 2 deletions

View File

@ -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;

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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;