Skip to content

Commit

Permalink
Add GUC gp_enable_statement_trigger
Browse files Browse the repository at this point in the history
Even though we claim that triggers are not supported in Greenplum, users
were still allowed to create them in GP6. In GP7, we've disabled
creating triggers, as such, restoring from GP6 that has triggers will
cause issues. Add a new GUC `gp_enable_statement_trigger`
to let `pg_restore` and users by pass this issue and create the trigger
anyway when the GUC `gp_enable_statement_trigger` is on.

The default value of `gp_enable_statement_trigger` is off.

Creating Triggers in GP7 were disabled
in commit https://github.com/greenplum-db/gpdb/commit/2325cffa86fa663567693cf54d5a98e7d5ed665f.
  • Loading branch information
Chibin authored and avamingli committed Jan 23, 2025
1 parent 6c48698 commit ed52a1b
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 3 deletions.
16 changes: 13 additions & 3 deletions src/backend/parser/gram.y
Original file line number Diff line number Diff line change
Expand Up @@ -8968,9 +8968,14 @@ TriggerForSpec:
}
| /* EMPTY */
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("Triggers for statements are not yet supported")));
/* let creation of triggers go through for pg_restore when upgrading from GP6 to GP7 */
if (!gp_enable_statement_trigger)
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("Triggers for statements are not yet supported")));
}
$$ = false;
}
;

Expand All @@ -8983,9 +8988,14 @@ TriggerForType:
ROW { $$ = true; }
| STATEMENT
{
/* let creation of triggers go through for pg_restore when upgrading from GP6 to GP7 */
if (!gp_enable_statement_trigger)
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("Triggers for statements are not yet supported")));
}
$$ = false;
}
;

Expand Down
13 changes: 13 additions & 0 deletions src/backend/utils/misc/guc_gp.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ int gp_max_partition_level;
bool gp_maintenance_mode;
bool gp_maintenance_conn;
bool allow_segment_DML;
bool gp_enable_statement_trigger;

/* Time based authentication GUC */
char *gp_auth_time_override_str = NULL;
Expand Down Expand Up @@ -620,6 +621,18 @@ struct config_bool ConfigureNamesBool_gp[] =
false,
NULL, NULL, NULL
},

{
{"gp_enable_statement_trigger", PGC_USERSET, CUSTOM_OPTIONS,
gettext_noop("Enables statement triggers to be created instead of erroring out."),
NULL,
GUC_NOT_IN_SAMPLE
},
&gp_enable_statement_trigger,
false,
NULL, NULL, NULL
},

{
{"enable_groupagg", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of grouping aggregation plans."),
Expand Down
1 change: 1 addition & 0 deletions src/include/utils/guc.h
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ extern int rep_lag_avoidance_threshold;
extern bool gp_maintenance_mode;
extern bool gp_maintenance_conn;
extern bool allow_segment_DML;
extern bool gp_enable_statement_trigger;

extern bool gp_ignore_error_table;

Expand Down
1 change: 1 addition & 0 deletions src/include/utils/unsync_guc_name.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
"gp_enable_relsize_collection",
"gp_enable_slow_writer_testmode",
"gp_enable_sort_limit",
"gp_enable_statement_trigger",
"gp_encoding_check_locale_compatibility",
"gp_external_enable_exec",
"gp_external_max_segs",
Expand Down
22 changes: 22 additions & 0 deletions src/test/regress/expected/triggers_gp.out
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
-- This file aims to cover the things that behave sanely, even though we don't
-- officially support anything to do with triggers.
--
-- Even though we claim that triggers are not supported in Greenplum, users
-- were still allowed to create them. As such, restoring from GP6 that has
-- triggers will cause issues; we now have a new GUC gp_enable_statement_trigger
-- to let pg_restore by pass this issue and create the trigger anyway.
--
create or replace function insert_notice_trig() returns trigger as $$
begin
raise notice 'insert trigger fired on % for %', TG_TABLE_NAME, TG_OP;
Expand Down Expand Up @@ -135,3 +140,20 @@ ERROR: UPDATE on distributed key column not allowed on relation with update tri
-- Should fire the DELETE trigger.
delete from parted_trig where nonkey = 2;
NOTICE: delete trigger fired on parted_trig2 for DELETE (seg0 127.0.0.1:40000 pid=10559)
--
-- Add GUC test to enable statement trigger
-- default GUC value is off
--
SET gp_enable_statement_trigger = on;
CREATE TABLE main_table_gp (a int, b int);
NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
CREATE FUNCTION trigger_func_gp() RETURNS trigger LANGUAGE plpgsql AS '
BEGIN
RAISE NOTICE ''trigger_func(%) called: action = %, when = %, level = %'', TG_ARGV[0], TG_OP, TG_WHEN, TG_LEVEL;
RETURN NULL;
END;';
-- We do not drop the trigger since this is used as part of the dump and restore testing of ICW
CREATE TRIGGER before_ins_stmt_trig_gp BEFORE INSERT ON main_table_gp
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func_gp('before_ins_stmt');
SET gp_enable_statement_trigger = off;
23 changes: 23 additions & 0 deletions src/test/regress/sql/triggers_gp.sql
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
-- This file aims to cover the things that behave sanely, even though we don't
-- officially support anything to do with triggers.
--
-- Even though we claim that triggers are not supported in Greenplum, users
-- were still allowed to create them. As such, restoring from GP6 that has
-- triggers will cause issues; we now have a new GUC gp_enable_statement_trigger
-- to let pg_restore by pass this issue and create the trigger anyway.
--

create or replace function insert_notice_trig() returns trigger as $$
begin
Expand Down Expand Up @@ -133,3 +138,21 @@ update parted_trig set partkey = partkey + 1, distkey = distkey + 1;

-- Should fire the DELETE trigger.
delete from parted_trig where nonkey = 2;

--
-- Add GUC test to enable statement trigger
-- default GUC value is off
--
SET gp_enable_statement_trigger = on;

CREATE TABLE main_table_gp (a int, b int);
CREATE FUNCTION trigger_func_gp() RETURNS trigger LANGUAGE plpgsql AS '
BEGIN
RAISE NOTICE ''trigger_func(%) called: action = %, when = %, level = %'', TG_ARGV[0], TG_OP, TG_WHEN, TG_LEVEL;
RETURN NULL;
END;';
-- We do not drop the trigger since this is used as part of the dump and restore testing of ICW
CREATE TRIGGER before_ins_stmt_trig_gp BEFORE INSERT ON main_table_gp
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func_gp('before_ins_stmt');

SET gp_enable_statement_trigger = off;

0 comments on commit ed52a1b

Please sign in to comment.