external
/
citus
mirror of https://github.com/citusdata/citus.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Retain trigger settings when re-creating the triggers (on shards) (#6398) 

Fixes https://github.com/citusdata/citus/issues/6394.

DESCRIPTION: Fixes a bug that causes creating disabled-triggers on
shards as enabled

Since CREATE TRIGGER doesn't have syntax support to specify
whether the trigger should be enabled/disabled, the underlying
PG function (`pg_get_triggerdef()`) that we use to generate the
command to create the trigger is not enough. For this reason, we
append a second command to enable/disable trigger, right after
creating it.

We don't retain explicit extension dependencies set by using
`ALTER trigger DEPENDS ON EXTENSION` commands too, but apparently
right fix for that is to throw an error as in
`PreprocessAlterTriggerDependsStmt()`; so, opened a separate PR
to fix that #6399.
pull/6361/head^2
Onur Tirtir 2022-10-06 14:51:07 +03:00 committed by GitHub
parent 27e867afbc
commit 86e186f671
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 333 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/backend/distributed/commands/trigger.c
View File

@ -43,6 +43,7 @@
/* local function forward declarations */
static char * GetAlterTriggerStateCommand(Oid triggerId);
static bool IsCreateCitusTruncateTriggerStmt(CreateTrigStmt *createTriggerStmt);
static String * GetAlterTriggerDependsTriggerNameValue(AlterObjectDependsStmt *
alterTriggerDependsStmt);
@ -99,6 +100,18 @@ GetExplicitTriggerCommandList(Oid relationId)
createTriggerCommandList = lappend(
createTriggerCommandList,
makeTableDDLCommandString(createTriggerCommand));
/*
* Appends the commands for the trigger settings that are not covered
* by CREATE TRIGGER command, such as ALTER TABLE ENABLE/DISABLE <trigger>.
*/
char *alterTriggerStateCommand =
GetAlterTriggerStateCommand(triggerId);
createTriggerCommandList = lappend(
createTriggerCommandList,
makeTableDDLCommandString(alterTriggerStateCommand));
}
/* revert back to original search_path */
@ -108,6 +121,67 @@ GetExplicitTriggerCommandList(Oid relationId)
}
/*
* GetAlterTriggerStateCommand returns the DDL command to set enable/disable
* state for given trigger. Throws an error if no such trigger exists.
*/
static char *
GetAlterTriggerStateCommand(Oid triggerId)
{
StringInfo alterTriggerStateCommand = makeStringInfo();
bool missingOk = false;
HeapTuple triggerTuple = GetTriggerTupleById(triggerId, missingOk);
Form_pg_trigger triggerForm = (Form_pg_trigger) GETSTRUCT(triggerTuple);
char *qualifiedRelName = generate_qualified_relation_name(triggerForm->tgrelid);
const char *quotedTrigName = quote_identifier(NameStr(triggerForm->tgname));
char enableDisableState = triggerForm->tgenabled;
heap_freetuple(triggerTuple);
const char *alterTriggerStateStr = NULL;
switch (enableDisableState)
{
case TRIGGER_FIRES_ON_ORIGIN:
{
/* default mode */
alterTriggerStateStr = "ENABLE";
break;
}
case TRIGGER_FIRES_ALWAYS:
{
alterTriggerStateStr = "ENABLE ALWAYS";
break;
}
case TRIGGER_FIRES_ON_REPLICA:
{
alterTriggerStateStr = "ENABLE REPLICA";
break;
}
case TRIGGER_DISABLED:
{
alterTriggerStateStr = "DISABLE";
break;
}
default:
{
elog(ERROR, "unexpected trigger state");
}
}
appendStringInfo(alterTriggerStateCommand, "ALTER TABLE %s %s TRIGGER %s;",
qualifiedRelName, alterTriggerStateStr, quotedTrigName);
return alterTriggerStateCommand->data;
}
/*
* GetTriggerTupleById returns copy of the heap tuple from pg_trigger for
* the trigger with triggerId. If no such trigger exists, this function returns

24
src/test/regress/expected/citus_local_table_triggers.out
View File

@ -232,6 +232,30 @@ SELECT * FROM citus_local_table_triggers
truncate_trigger_xxxxxxx | "interesting!schema"."citus_local!_table" | O
(3 rows)
-- ALTER TABLE ENABLE REPLICA trigger
ALTER TABLE "interesting!schema"."citus_local!_table" ENABLE REPLICA TRIGGER "trigger\'name22";
NOTICE: executing the command locally: SELECT worker_apply_shard_ddl_command (1507008, 'interesting!schema', E'ALTER TABLE "interesting!schema"."citus_local!_table" ENABLE REPLICA TRIGGER "trigger\\''name22";')
SELECT * FROM citus_local_table_triggers
WHERE tgname NOT LIKE 'RI_ConstraintTrigger%';
tgname | tgrelid | tgenabled
---------------------------------------------------------------------
trigger\'name22 | "interesting!schema"."citus_local!_table" | R
trigger\'name22_1507008 | "interesting!schema"."citus_local!_table_1507008" | R
truncate_trigger_xxxxxxx | "interesting!schema"."citus_local!_table" | O
(3 rows)
-- ALTER TABLE ENABLE ALWAYS trigger
ALTER TABLE "interesting!schema"."citus_local!_table" ENABLE ALWAYS TRIGGER "trigger\'name22";
NOTICE: executing the command locally: SELECT worker_apply_shard_ddl_command (1507008, 'interesting!schema', E'ALTER TABLE "interesting!schema"."citus_local!_table" ENABLE ALWAYS TRIGGER "trigger\\''name22";')
SELECT * FROM citus_local_table_triggers
WHERE tgname NOT LIKE 'RI_ConstraintTrigger%';
tgname | tgrelid | tgenabled
---------------------------------------------------------------------
trigger\'name22 | "interesting!schema"."citus_local!_table" | A
trigger\'name22_1507008 | "interesting!schema"."citus_local!_table_1507008" | A
truncate_trigger_xxxxxxx | "interesting!schema"."citus_local!_table" | O
(3 rows)
-- ALTER TABLE DISABLE trigger
ALTER TABLE "interesting!schema"."citus_local!_table" DISABLE TRIGGER "trigger\'name22";
NOTICE: executing the command locally: SELECT worker_apply_shard_ddl_command (1507008, 'interesting!schema', E'ALTER TABLE "interesting!schema"."citus_local!_table" DISABLE TRIGGER "trigger\\''name22";')

5
src/test/regress/expected/citus_table_triggers.out
View File

@ -146,9 +146,12 @@ SELECT master_get_table_ddl_events('test_table');
CREATE TABLE table_triggers_schema.test_table (id integer, text_number text, text_col text)
ALTER TABLE table_triggers_schema.test_table OWNER TO postgres
CREATE TRIGGER test_table_delete AFTER DELETE ON table_triggers_schema.test_table FOR EACH STATEMENT EXECUTE FUNCTION table_triggers_schema.test_table_trigger_function()
ALTER TABLE table_triggers_schema.test_table ENABLE TRIGGER test_table_delete;
CREATE CONSTRAINT TRIGGER test_table_insert AFTER INSERT ON table_triggers_schema.test_table DEFERRABLE INITIALLY IMMEDIATE FOR EACH ROW WHEN (((new.id > 5) OR ((new.text_col IS NOT NULL) AND ((new.id)::numeric < to_number(new.text_number, '9999'::text))))) EXECUTE FUNCTION table_triggers_schema.test_table_trigger_function()
ALTER TABLE table_triggers_schema.test_table ENABLE TRIGGER test_table_insert;
CREATE CONSTRAINT TRIGGER test_table_update AFTER UPDATE OF id ON table_triggers_schema.test_table NOT DEFERRABLE INITIALLY IMMEDIATE FOR EACH ROW WHEN (((NOT (old.* IS DISTINCT FROM new.*)) AND (old.text_number IS NOT NULL))) EXECUTE FUNCTION table_triggers_schema.test_table_trigger_function()
(5 rows)
ALTER TABLE table_triggers_schema.test_table ENABLE TRIGGER test_table_update;
(8 rows)
-- cleanup at exit
DROP SCHEMA table_triggers_schema CASCADE;

162
src/test/regress/expected/distributed_triggers.out
View File

@ -855,7 +855,145 @@ SELECT run_command_on_workers($$SELECT count(*) FROM pg_trigger WHERE tgname lik
(localhost,57638,t,1)
(2 rows)
RESET client_min_messages;
CREATE TABLE "dist_\'table"(a int);
CREATE FUNCTION trigger_func()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
BEGIN
RETURN NULL;
END;
$function$;
CREATE TRIGGER default_mode_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
CREATE TRIGGER "disabled_trigger\'"
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" DISABLE trigger "disabled_trigger\'";
CREATE TRIGGER replica_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" ENABLE REPLICA trigger replica_trigger;
CREATE TRIGGER always_enabled_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" ENABLE ALWAYS trigger always_enabled_trigger;
CREATE TRIGGER noop_enabled_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" ENABLE trigger noop_enabled_trigger;
SELECT create_distributed_table('dist_\''table', 'a');
create_distributed_table
---------------------------------------------------------------------
(1 row)
SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'default_mode_trigger%';
bool_and
---------------------------------------------------------------------
t
(1 row)
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'default_mode_trigger%'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,t)
(localhost,57638,t,t)
(2 rows)
SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'disabled_trigger%';
bool_and
---------------------------------------------------------------------
t
(1 row)
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'disabled_trigger%'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,t)
(localhost,57638,t,t)
(2 rows)
SELECT bool_and(tgenabled = 'R') FROM pg_trigger WHERE tgname LIKE 'replica_trigger%';
bool_and
---------------------------------------------------------------------
t
(1 row)
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'R') FROM pg_trigger WHERE tgname LIKE 'replica_trigger%'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,t)
(localhost,57638,t,t)
(2 rows)
SELECT bool_and(tgenabled = 'A') FROM pg_trigger WHERE tgname LIKE 'always_enabled_trigger%';
bool_and
---------------------------------------------------------------------
t
(1 row)
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'A') FROM pg_trigger WHERE tgname LIKE 'always_enabled_trigger%'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,t)
(localhost,57638,t,t)
(2 rows)
SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'noop_enabled_trigger%';
bool_and
---------------------------------------------------------------------
t
(1 row)
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'noop_enabled_trigger%'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,t)
(localhost,57638,t,t)
(2 rows)
CREATE TABLE citus_local(a int);
CREATE FUNCTION citus_local_trig_func()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
BEGIN
RETURN NULL;
END;
$function$;
CREATE TRIGGER citus_local_trig
AFTER UPDATE OR DELETE ON citus_local
FOR STATEMENT EXECUTE FUNCTION citus_local_trig_func();
-- make sure that trigger is initially not disabled
SELECT tgenabled = 'D' FROM pg_trigger WHERE tgname LIKE 'citus_local_trig%';
?column?
---------------------------------------------------------------------
f
(1 row)
ALTER TABLE citus_local DISABLE trigger citus_local_trig;
SELECT citus_add_local_table_to_metadata('citus_local');
citus_add_local_table_to_metadata
---------------------------------------------------------------------
(1 row)
SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'citus_local_trig%';
bool_and
---------------------------------------------------------------------
t
(1 row)
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'citus_local_trig%'$$);
run_command_on_workers
---------------------------------------------------------------------
(localhost,57637,t,t)
(localhost,57638,t,t)
(2 rows)
SET client_min_messages TO ERROR;
RESET citus.enable_unsafe_triggers;
SELECT run_command_on_workers('ALTER SYSTEM RESET citus.enable_unsafe_triggers;');
run_command_on_workers
@ -873,25 +1011,3 @@ SELECT run_command_on_workers('SELECT pg_reload_conf();');
SET citus.log_remote_commands TO off;
DROP SCHEMA distributed_triggers CASCADE;
NOTICE: drop cascades to 21 other objects
DETAIL: drop cascades to table data
drop cascades to function record_change()
drop cascades to function insert_delete_document(text,text)
drop cascades to function bad_shardkey_record_change()
drop cascades to function remote_shardkey_record_change()
drop cascades to function insert_document(text,text)
drop cascades to table emptest
drop cascades to table emptest_audit
drop cascades to function process_emp_audit()
drop cascades to view emp_triggers
drop cascades to table record_op
drop cascades to function record_emp()
drop cascades to table data_changes
drop cascades to table sale
drop cascades to table record_sale
drop cascades to function record_sale()
drop cascades to view sale_triggers
drop cascades to extension seg
drop cascades to table distributed_table
drop cascades to table distributed_table_change
drop cascades to function insert_99()

10
src/test/regress/sql/citus_local_table_triggers.sql
View File

@ -185,6 +185,16 @@ ALTER TRIGGER "trigger\'name" ON "interesting!schema"."citus_local!_table" RENAM
SELECT * FROM citus_local_table_triggers
WHERE tgname NOT LIKE 'RI_ConstraintTrigger%';
-- ALTER TABLE ENABLE REPLICA trigger
ALTER TABLE "interesting!schema"."citus_local!_table" ENABLE REPLICA TRIGGER "trigger\'name22";
SELECT * FROM citus_local_table_triggers
WHERE tgname NOT LIKE 'RI_ConstraintTrigger%';
-- ALTER TABLE ENABLE ALWAYS trigger
ALTER TABLE "interesting!schema"."citus_local!_table" ENABLE ALWAYS TRIGGER "trigger\'name22";
SELECT * FROM citus_local_table_triggers
WHERE tgname NOT LIKE 'RI_ConstraintTrigger%';
-- ALTER TABLE DISABLE trigger
ALTER TABLE "interesting!schema"."citus_local!_table" DISABLE TRIGGER "trigger\'name22";
SELECT * FROM citus_local_table_triggers

83
src/test/regress/sql/distributed_triggers.sql
View File

@ -473,7 +473,88 @@ SELECT * FROM distributed_table_change;
SELECT tgrelid::regclass::text, tgname FROM pg_trigger WHERE tgname like 'insert_99_trigger%' ORDER BY 1,2;
SELECT run_command_on_workers($$SELECT count(*) FROM pg_trigger WHERE tgname like 'insert_99_trigger%'$$);
RESET client_min_messages;
CREATE TABLE "dist_\'table"(a int);
CREATE FUNCTION trigger_func()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
BEGIN
RETURN NULL;
END;
$function$;
CREATE TRIGGER default_mode_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
CREATE TRIGGER "disabled_trigger\'"
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" DISABLE trigger "disabled_trigger\'";
CREATE TRIGGER replica_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" ENABLE REPLICA trigger replica_trigger;
CREATE TRIGGER always_enabled_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" ENABLE ALWAYS trigger always_enabled_trigger;
CREATE TRIGGER noop_enabled_trigger
AFTER UPDATE OR DELETE ON "dist_\'table"
FOR STATEMENT EXECUTE FUNCTION trigger_func();
ALTER TABLE "dist_\'table" ENABLE trigger noop_enabled_trigger;
SELECT create_distributed_table('dist_\''table', 'a');
SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'default_mode_trigger%';
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'default_mode_trigger%'$$);
SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'disabled_trigger%';
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'disabled_trigger%'$$);
SELECT bool_and(tgenabled = 'R') FROM pg_trigger WHERE tgname LIKE 'replica_trigger%';
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'R') FROM pg_trigger WHERE tgname LIKE 'replica_trigger%'$$);
SELECT bool_and(tgenabled = 'A') FROM pg_trigger WHERE tgname LIKE 'always_enabled_trigger%';
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'A') FROM pg_trigger WHERE tgname LIKE 'always_enabled_trigger%'$$);
SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'noop_enabled_trigger%';
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'O') FROM pg_trigger WHERE tgname LIKE 'noop_enabled_trigger%'$$);
CREATE TABLE citus_local(a int);
CREATE FUNCTION citus_local_trig_func()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
BEGIN
RETURN NULL;
END;
$function$;
CREATE TRIGGER citus_local_trig
AFTER UPDATE OR DELETE ON citus_local
FOR STATEMENT EXECUTE FUNCTION citus_local_trig_func();
-- make sure that trigger is initially not disabled
SELECT tgenabled = 'D' FROM pg_trigger WHERE tgname LIKE 'citus_local_trig%';
ALTER TABLE citus_local DISABLE trigger citus_local_trig;
SELECT citus_add_local_table_to_metadata('citus_local');
SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'citus_local_trig%';
SELECT run_command_on_workers($$SELECT bool_and(tgenabled = 'D') FROM pg_trigger WHERE tgname LIKE 'citus_local_trig%'$$);
SET client_min_messages TO ERROR;
RESET citus.enable_unsafe_triggers;
SELECT run_command_on_workers('ALTER SYSTEM RESET citus.enable_unsafe_triggers;');
SELECT run_command_on_workers('SELECT pg_reload_conf();');