Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for DROP SCHEMA on multi-node #3949

Merged
merged 1 commit into from
Dec 23, 2021
Merged

Add support for DROP SCHEMA on multi-node #3949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions tsl/src/remote/dist_ddl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,19 +368,24 @@ dist_ddl_process_drop(const ProcessUtilityArgs *args)
{
DropStmt *stmt = castNode(DropStmt, args->parsetree);

/* For DROP TABLE and DROP SCHEMA operations hypertable_list will be empty */
if (list_length(args->hypertable_list) == 0)
{
Comment on lines +371 to 373
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the hypertable_list is not empty, and removeType == OBJECT_TABLE, what would that mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or what does the default branch in the following switch/case means... looks like this needs more assertions/comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe hypertable can't be added into the hypertable list, because drop operation is already happened by PostgreSQL standard hook (src/process_utility.c) at this point and it can't be resolved

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be an Assert then, with this explanation as a comment?

/*
* CASCADE operations of DROP TABLE and DROP SCHEMA are handled in
* sql_drop trigger.
*/

/* For DROP TABLE and DROP SCHEMA operations hypertable_list will be
* empty. Wait for sql_drop events.
*/
if (stmt->removeType == OBJECT_TABLE || stmt->removeType == OBJECT_SCHEMA)
dist_ddl_state_schedule(DIST_DDL_EXEC_ON_END, args);

switch (stmt->removeType)
{
case OBJECT_TABLE:
/* Wait for further sql_drop events */
dist_ddl_state_schedule(DIST_DDL_EXEC_ON_END, args);
break;
case OBJECT_SCHEMA:
/* Forward DROP SCHEMA command to all data nodes, following
* sql_drop events will be ignored */
Comment on lines +381 to +382
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A completely clueless question: why do we have to execute DROP SCHEMA immediately, in contrast to DROP TABLE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we did not support CREATE SCHEMA explicitly on data nodes and whenDROP SCHEMA CASCADE was executed, we only shipped it by analyzing distributed hypertables which we part of that schema. No it is much simpler, we assume schema exists on all data nodes and we can drop it right away.

dist_ddl_state_schedule(DIST_DDL_EXEC_ON_START, args);
dist_ddl_state_add_current_data_node_list();
break;
default:
break;
}
return;
}

Expand Down
288 changes: 287 additions & 1 deletion tsl/test/expected/dist_ddl.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,9 +731,57 @@ NOTICE: [data_node_3]:

DROP TABLE non_disttable1;
DROP TABLE non_disttable2;
-- CREATE and DROP SCHEMA on each node
-- CREATE SCHEMA tests
\c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
CREATE SCHEMA dist_schema AUTHORIZATION :ROLE_1;
-- make sure schema has been created on each data node
SELECT * FROM test.remote_exec(NULL, $$
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema';
$$);
NOTICE: [data_node_1]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema'
NOTICE: [data_node_1]:
nspname |usename
-----------+-----------
dist_schema|test_role_1
(1 row)


NOTICE: [data_node_2]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema'
NOTICE: [data_node_2]:
nspname |usename
-----------+-----------
dist_schema|test_role_1
(1 row)


NOTICE: [data_node_3]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema'
NOTICE: [data_node_3]:
nspname |usename
-----------+-----------
dist_schema|test_role_1
(1 row)


remote_exec
-------------

(1 row)

CREATE TABLE dist_schema.some_dist_table(time timestamptz, device int, color int, temp float);
SELECT * FROM create_hypertable('dist_schema.some_dist_table', 'time', replication_factor => 3);
NOTICE: adding not-null constraint to column "time"
Expand Down Expand Up @@ -772,6 +820,7 @@ dist_schema|some_dist_table

(1 row)

-- DROP SCHEMA
DROP SCHEMA dist_schema CASCADE;
NOTICE: drop cascades to table dist_schema.some_dist_table
SELECT * FROM test.remote_exec(NULL, $$ SELECT schemaname, tablename FROM pg_tables WHERE tablename = 'some_dist_table' $$);
Expand Down Expand Up @@ -801,6 +850,243 @@ schemaname|tablename

(1 row)

-- make sure schema has been dropped on each data node
SELECT * FROM test.remote_exec(NULL, $$
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema';
$$);
NOTICE: [data_node_1]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema'
NOTICE: [data_node_1]:
nspname|usename
-------+-------
(0 rows)


NOTICE: [data_node_2]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema'
NOTICE: [data_node_2]:
nspname|usename
-------+-------
(0 rows)


NOTICE: [data_node_3]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema'
NOTICE: [data_node_3]:
nspname|usename
-------+-------
(0 rows)


remote_exec
-------------

(1 row)

-- make sure empty schema schema has been created and then dropped on each data node
CREATE SCHEMA dist_schema_2;
SELECT * FROM test.remote_exec(NULL, $$
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2';
$$);
NOTICE: [data_node_1]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2'
NOTICE: [data_node_1]:
nspname |usename
-------------+------------------
dist_schema_2|cluster_super_user
(1 row)


NOTICE: [data_node_2]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2'
NOTICE: [data_node_2]:
nspname |usename
-------------+------------------
dist_schema_2|cluster_super_user
(1 row)


NOTICE: [data_node_3]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2'
NOTICE: [data_node_3]:
nspname |usename
-------------+------------------
dist_schema_2|cluster_super_user
(1 row)


remote_exec
-------------

(1 row)

DROP SCHEMA dist_schema_2;
SELECT * FROM test.remote_exec(NULL, $$
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2';
$$);
NOTICE: [data_node_1]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2'
NOTICE: [data_node_1]:
nspname|usename
-------+-------
(0 rows)


NOTICE: [data_node_2]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2'
NOTICE: [data_node_2]:
nspname|usename
-------+-------
(0 rows)


NOTICE: [data_node_3]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_2'
NOTICE: [data_node_3]:
nspname|usename
-------+-------
(0 rows)


remote_exec
-------------

(1 row)

-- transactional schema create/drop with local table
BEGIN;
CREATE SCHEMA dist_schema_3;
CREATE TABLE dist_schema_3.some_dist_table(time timestamptz, device int);
SELECT * FROM test.remote_exec(NULL, $$
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3';
$$);
NOTICE: [data_node_1]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3'
NOTICE: [data_node_1]:
nspname |usename
-------------+------------------
dist_schema_3|cluster_super_user
(1 row)


NOTICE: [data_node_2]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3'
NOTICE: [data_node_2]:
nspname |usename
-------------+------------------
dist_schema_3|cluster_super_user
(1 row)


NOTICE: [data_node_3]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3'
NOTICE: [data_node_3]:
nspname |usename
-------------+------------------
dist_schema_3|cluster_super_user
(1 row)


remote_exec
-------------

(1 row)

DROP SCHEMA dist_schema_3 CASCADE;
NOTICE: drop cascades to table dist_schema_3.some_dist_table
ROLLBACK;
SELECT * FROM test.remote_exec(NULL, $$
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3';
$$);
NOTICE: [data_node_1]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3'
NOTICE: [data_node_1]:
nspname|usename
-------+-------
(0 rows)


NOTICE: [data_node_2]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3'
NOTICE: [data_node_2]:
nspname|usename
-------+-------
(0 rows)


NOTICE: [data_node_3]:
SELECT s.nspname, u.usename
FROM pg_catalog.pg_namespace s
JOIN pg_catalog.pg_user u ON u.usesysid = s.nspowner
WHERE s.nspname = 'dist_schema_3'
NOTICE: [data_node_3]:
nspname|usename
-------+-------
(0 rows)


remote_exec
-------------

(1 row)

-- DROP column cascades to index drop
CREATE TABLE some_dist_table(time timestamptz, device int, color int, temp float);
SELECT * FROM create_hypertable('some_dist_table', 'time', replication_factor => 3);
Expand Down
Loading