This is an automated email from the ASF dual-hosted git repository. avamingli pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit ccac620b0018fe4b9548378574c0c9156cf9185f Author: Marbin Tan <[email protected]> AuthorDate: Mon Mar 18 14:24:56 2024 -0700 Add GUC gp_enable_statement_trigger 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. --- src/backend/parser/gram.y | 16 +++++++++++++--- src/backend/utils/misc/guc_gp.c | 13 +++++++++++++ src/include/utils/guc.h | 1 + src/include/utils/unsync_guc_name.h | 1 + src/test/regress/expected/triggers_gp.out | 22 ++++++++++++++++++++++ src/test/regress/sql/triggers_gp.sql | 23 +++++++++++++++++++++++ 6 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index f300cdb828..fdbe8f7f9e 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -8966,9 +8966,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; } ; @@ -8981,9 +8986,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; } ; diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c index 34a84582b6..1995d91e75 100644 --- a/src/backend/utils/misc/guc_gp.c +++ b/src/backend/utils/misc/guc_gp.c @@ -249,6 +249,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; @@ -621,6 +622,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."), diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index b79498207c..f9045dc8a2 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -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; diff --git a/src/include/utils/unsync_guc_name.h b/src/include/utils/unsync_guc_name.h index 4c7226e33c..f6c3f11c48 100644 --- a/src/include/utils/unsync_guc_name.h +++ b/src/include/utils/unsync_guc_name.h @@ -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", diff --git a/src/test/regress/expected/triggers_gp.out b/src/test/regress/expected/triggers_gp.out index 493fe305f4..3191971aee 100644 --- a/src/test/regress/expected/triggers_gp.out +++ b/src/test/regress/expected/triggers_gp.out @@ -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; @@ -102,3 +107,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; diff --git a/src/test/regress/sql/triggers_gp.sql b/src/test/regress/sql/triggers_gp.sql index 46d3087239..72c50858ca 100644 --- a/src/test/regress/sql/triggers_gp.sql +++ b/src/test/regress/sql/triggers_gp.sql @@ -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 @@ -104,3 +109,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; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
