diff --git a/src/backend/columnar/cstore_tableam.c b/src/backend/columnar/cstore_tableam.c index 6c52660ef..d87ba612b 100644 --- a/src/backend/columnar/cstore_tableam.c +++ b/src/backend/columnar/cstore_tableam.c @@ -42,6 +42,7 @@ #include "storage/smgr.h" #include "tcop/utility.h" #include "utils/builtins.h" +#include "utils/fmgroids.h" #include "utils/memutils.h" #include "utils/pg_rusage.h" #include "utils/rel.h" @@ -94,30 +95,13 @@ typedef struct CStoreScanDescData typedef struct CStoreScanDescData *CStoreScanDesc; static object_access_hook_type PrevObjectAccessHook = NULL; -static ProcessUtility_hook_type PreviousProcessUtilityHook = NULL; /* forward declaration for static functions */ -static void CStoreTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid - objectId, int subId, +static void CStoreTableDropHook(Oid tgid); +static void CStoreTriggerCreateHook(Oid tgid); +static void CStoreTableAMObjectAccessHook(ObjectAccessType access, Oid classId, + Oid objectId, int subId, void *arg); -#if PG_VERSION_NUM >= 130000 -static void CStoreTableAMProcessUtility(PlannedStmt *plannedStatement, - const char *queryString, - ProcessUtilityContext context, - ParamListInfo paramListInfo, - QueryEnvironment *queryEnvironment, - DestReceiver *destReceiver, - QueryCompletion *qc); -#else -static void CStoreTableAMProcessUtility(PlannedStmt *plannedStatement, - const char *queryString, - ProcessUtilityContext context, - ParamListInfo paramListInfo, - QueryEnvironment *queryEnvironment, - DestReceiver *destReceiver, - char *completionTag); -#endif - static bool ConditionalLockRelationWithTimeout(Relation rel, LOCKMODE lockMode, int timeout, int retryInterval); static void LogRelationStats(Relation rel, int elevel); @@ -1143,60 +1127,12 @@ CStoreSubXactCallback(SubXactEvent event, SubTransactionId mySubid, } -#if PG_VERSION_NUM >= 130000 -static void -CStoreTableAMProcessUtility(PlannedStmt *plannedStatement, - const char *queryString, - ProcessUtilityContext context, - ParamListInfo paramListInfo, - QueryEnvironment *queryEnvironment, - DestReceiver *destReceiver, - QueryCompletion *queryCompletion) -#else -static void -CStoreTableAMProcessUtility(PlannedStmt * plannedStatement, - const char * queryString, - ProcessUtilityContext context, - ParamListInfo paramListInfo, - QueryEnvironment * queryEnvironment, - DestReceiver * destReceiver, - char * completionTag) -#endif -{ - Node *parseTree = plannedStatement->utilityStmt; - - if (nodeTag(parseTree) == T_CreateTrigStmt) - { - CreateTrigStmt *createTrigStmt = (CreateTrigStmt *) parseTree; - - Relation rel = relation_openrv(createTrigStmt->relation, AccessShareLock); - bool isCStore = rel->rd_tableam == GetColumnarTableAmRoutine(); - relation_close(rel, AccessShareLock); - - if (isCStore && - createTrigStmt->row && - createTrigStmt->timing == TRIGGER_TYPE_AFTER) - { - ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg( - "AFTER ROW triggers are not supported for columnstore access method"), - errhint("Consider an AFTER STATEMENT trigger instead."))); - } - } - - CALL_PREVIOUS_UTILITY(); -} - - void cstore_tableam_init() { RegisterXactCallback(CStoreXactCallback, NULL); RegisterSubXactCallback(CStoreSubXactCallback, NULL); - PreviousProcessUtilityHook = (ProcessUtility_hook != NULL) ? - ProcessUtility_hook : standard_ProcessUtility; - ProcessUtility_hook = CStoreTableAMProcessUtility; PrevObjectAccessHook = object_access_hook; object_access_hook = CStoreTableAMObjectAccessHook; @@ -1240,44 +1176,28 @@ ColumnarSlotCopyHeapTuple(TupleTableSlot *slot) /* - * Implements object_access_hook. One of the places this is called is just - * before dropping an object, which allows us to clean-up resources for - * cstore tables. + * CStoreTableDropHook * - * See the comments for CStoreFdwObjectAccessHook for more details. + * Clean-up resources for columnar tables. */ static void -CStoreTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId, int - subId, - void *arg) +CStoreTableDropHook(Oid relid) { - if (PrevObjectAccessHook) - { - PrevObjectAccessHook(access, classId, objectId, subId, arg); - } - - /* - * Do nothing if this is not a DROP relation command. - */ - if (access != OAT_DROP || classId != RelationRelationId || OidIsValid(subId)) - { - return; - } - /* * Lock relation to prevent it from being dropped and to avoid * race conditions in the next if block. */ - LockRelationOid(objectId, AccessShareLock); + LockRelationOid(relid, AccessShareLock); - if (IsCStoreTableAmTable(objectId)) + if (IsCStoreTableAmTable(relid)) { /* * Drop metadata. No need to drop storage here since for * tableam tables storage is managed by postgres. */ - Relation rel = table_open(objectId, AccessExclusiveLock); + Relation rel = table_open(relid, AccessExclusiveLock); RelFileNode relfilenode = rel->rd_node; + DeleteMetadataRows(relfilenode); DeleteColumnarTableOptions(rel->rd_id, true); @@ -1289,6 +1209,77 @@ CStoreTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId } +/* + * Reject AFTER ... FOR EACH ROW triggers on columnar tables. + */ +static void +CStoreTriggerCreateHook(Oid tgid) +{ + /* + * Fetch the pg_trigger tuple by the Oid of the trigger + */ + ScanKeyData skey[1]; + Relation tgrel = table_open(TriggerRelationId, AccessShareLock); + + ScanKeyInit(&skey[0], + Anum_pg_trigger_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(tgid)); + + SysScanDesc tgscan = systable_beginscan(tgrel, TriggerOidIndexId, true, + SnapshotSelf, 1, skey); + + HeapTuple tgtup = systable_getnext(tgscan); + + if (!HeapTupleIsValid(tgtup)) + { + table_close(tgrel, AccessShareLock); + return; + } + + Form_pg_trigger tgrec = (Form_pg_trigger) GETSTRUCT(tgtup); + + Oid tgrelid = tgrec->tgrelid; + int16 tgtype = tgrec->tgtype; + + systable_endscan(tgscan); + table_close(tgrel, AccessShareLock); + + if (TRIGGER_FOR_ROW(tgtype) && TRIGGER_FOR_AFTER(tgtype) && + IsCStoreTableAmTable(tgrelid)) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg( + "Foreign keys and AFTER ROW triggers are not supported for columnar tables"), + errhint("Consider an AFTER STATEMENT trigger instead."))); + } +} + + +/* + * Capture create/drop events and dispatch to the proper action. + */ +static void +CStoreTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId, + int subId, void *arg) +{ + if (PrevObjectAccessHook) + { + PrevObjectAccessHook(access, classId, objectId, subId, arg); + } + + /* dispatch to the proper action */ + if (access == OAT_DROP && classId == RelationRelationId && !OidIsValid(subId)) + { + CStoreTableDropHook(objectId); + } + else if (access == OAT_POST_CREATE && classId == TriggerRelationId) + { + CStoreTriggerCreateHook(objectId); + } +} + + /* * IsCStoreTableAmTable returns true if relation has cstore_tableam * access method. This can be called before extension creation. diff --git a/src/include/columnar/cstore_version_compat.h b/src/include/columnar/cstore_version_compat.h index 69eb9c9f3..ee4b86f2c 100644 --- a/src/include/columnar/cstore_version_compat.h +++ b/src/include/columnar/cstore_version_compat.h @@ -32,20 +32,6 @@ ExplainPropertyInteger(qlabel, NULL, value, es) #endif -#if PG_VERSION_NUM >= 130000 -#define CALL_PREVIOUS_UTILITY() \ - PreviousProcessUtilityHook(plannedStatement, queryString, context, paramListInfo, \ - queryEnvironment, destReceiver, queryCompletion) -#elif PG_VERSION_NUM >= 100000 -#define CALL_PREVIOUS_UTILITY() \ - PreviousProcessUtilityHook(plannedStatement, queryString, context, paramListInfo, \ - queryEnvironment, destReceiver, completionTag) -#else -#define CALL_PREVIOUS_UTILITY() \ - PreviousProcessUtilityHook(parseTree, queryString, context, paramListInfo, \ - destReceiver, completionTag) -#endif - #if PG_VERSION_NUM < 120000 #define TTS_EMPTY(slot) ((slot)->tts_isempty) #define ExecForceStoreHeapTuple(tuple, slot, shouldFree) \ diff --git a/src/test/regress/expected/alter_table_set_access_method.out b/src/test/regress/expected/alter_table_set_access_method.out index 990c94cc4..e5cab6790 100644 --- a/src/test/regress/expected/alter_table_set_access_method.out +++ b/src/test/regress/expected/alter_table_set_access_method.out @@ -324,66 +324,19 @@ SELECT c.relname, a.amname FROM pg_class c, pg_am a where c.relname SIMILAR TO ' (4 rows) -- test when the parent of a partition has foreign key to a reference table -CREATE TABLE ref_table (a INT UNIQUE); -SELECT create_reference_table('ref_table'); - create_reference_table ---------------------------------------------------------------------- - -(1 row) - -INSERT INTO ref_table VALUES (2), (12); -ALTER TABLE partitioned_table ADD CONSTRAINT fkey_to_ref FOREIGN KEY (a) REFERENCES ref_table(a); -SELECT inhrelid::regclass::text FROM pg_catalog.pg_inherits WHERE inhparent = 'partitioned_table'::regclass ORDER BY 1; - inhrelid ---------------------------------------------------------------------- - partitioned_table_1_5 - partitioned_table_6_10 -(2 rows) - -SELECT "Name", "Access Method" FROM public.citus_tables WHERE "Name"::text = 'partitioned_table_6_10'; - Name | Access Method ---------------------------------------------------------------------- - partitioned_table_6_10 | heap -(1 row) - -SELECT conrelid::regclass::text AS "Referencing Table", pg_get_constraintdef(oid, true) AS "Definition" FROM pg_constraint - WHERE (conrelid::regclass::text = 'partitioned_table_6_10') ORDER BY 1, 2; - Referencing Table | Definition ---------------------------------------------------------------------- - partitioned_table_6_10 | FOREIGN KEY (a) REFERENCES ref_table(a) -(1 row) - -SELECT alter_table_set_access_method('partitioned_table_6_10', 'columnar'); +create table test_pk(n int primary key); +create table test_fk_p(i int references test_pk(n)) partition by range(i); +create table test_fk_p0 partition of test_fk_p for values from (0) to (10); +create table test_fk_p1 partition of test_fk_p for values from (10) to (20); +select alter_table_set_access_method('test_fk_p1', 'columnar'); NOTICE: any index will be dropped, because columnar tables cannot have indexes -NOTICE: creating a new table for alter_table_set_access_method.partitioned_table_6_10 -NOTICE: Moving the data of alter_table_set_access_method.partitioned_table_6_10 -NOTICE: Dropping the old alter_table_set_access_method.partitioned_table_6_10 -NOTICE: Renaming the new table to alter_table_set_access_method.partitioned_table_6_10 - alter_table_set_access_method ---------------------------------------------------------------------- - -(1 row) - -SELECT inhrelid::regclass::text FROM pg_catalog.pg_inherits WHERE inhparent = 'partitioned_table'::regclass ORDER BY 1; - inhrelid ---------------------------------------------------------------------- - partitioned_table_1_5 - partitioned_table_6_10 -(2 rows) - -SELECT "Name", "Access Method" FROM public.citus_tables WHERE "Name"::text = 'partitioned_table_6_10'; - Name | Access Method ---------------------------------------------------------------------- - partitioned_table_6_10 | columnar -(1 row) - -SELECT conrelid::regclass::text AS "Referencing Table", pg_get_constraintdef(oid, true) AS "Definition" FROM pg_constraint - WHERE (conrelid::regclass::text = 'partitioned_table_6_10') ORDER BY 1, 2; - Referencing Table | Definition ---------------------------------------------------------------------- - partitioned_table_6_10 | FOREIGN KEY (a) REFERENCES ref_table(a) -(1 row) - +NOTICE: creating a new table for alter_table_set_access_method.test_fk_p1 +NOTICE: Moving the data of alter_table_set_access_method.test_fk_p1 +NOTICE: Dropping the old alter_table_set_access_method.test_fk_p1 +NOTICE: Renaming the new table to alter_table_set_access_method.test_fk_p1 +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables +HINT: Consider an AFTER STATEMENT trigger instead. +CONTEXT: SQL statement "ALTER TABLE alter_table_set_access_method.test_fk_p ATTACH PARTITION alter_table_set_access_method.test_fk_p1 FOR VALUES FROM (10) TO (20);" SET client_min_messages TO WARNING; DROP SCHEMA alter_table_set_access_method CASCADE; SELECT 1 FROM master_remove_node('localhost', :master_port); diff --git a/src/test/regress/expected/am_trigger.out b/src/test/regress/expected/am_trigger.out index 595650591..048084d8e 100644 --- a/src/test/regress/expected/am_trigger.out +++ b/src/test/regress/expected/am_trigger.out @@ -46,7 +46,7 @@ create trigger tr_before_row before insert on test_tr -- after triggers require TIDs, which are not supported yet create trigger tr_after_row after insert on test_tr for each row execute procedure trr_after(); -ERROR: AFTER ROW triggers are not supported for columnstore access method +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables HINT: Consider an AFTER STATEMENT trigger instead. insert into test_tr values(1); NOTICE: BEFORE STATEMENT INSERT @@ -84,6 +84,46 @@ SELECT * FROM test_tr ORDER BY i; (4 rows) drop table test_tr; +-- +-- test implicit triggers created by foreign keys or partitions of a +-- table with a trigger +-- +create table test_tr_referenced(i int primary key); +-- should fail when creating FK trigger +create table test_tr_referencing(j int references test_tr_referenced(i)) using columnar; +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables +HINT: Consider an AFTER STATEMENT trigger instead. +drop table test_tr_referenced; +create table test_tr_p(i int) partition by range(i); +create trigger test_tr_p_tr after update on test_tr_p + for each row execute procedure trr_after(); +create table test_tr_p0 partition of test_tr_p + for values from (0) to (10); +-- create columnar partition should fail +create table test_tr_p1 partition of test_tr_p + for values from (10) to (20) using columnar; +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables +HINT: Consider an AFTER STATEMENT trigger instead. +create table test_tr_p2(i int) using columnar; +-- attach columnar partition should fail +alter table test_tr_p attach partition test_tr_p2 for values from (20) to (30); +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables +HINT: Consider an AFTER STATEMENT trigger instead. +drop table test_tr_p; +create table test_pk(n int primary key); +create table test_fk_p(i int references test_pk(n)) partition by range(i); +create table test_fk_p0 partition of test_fk_p for values from (0) to (10); +-- create columnar partition should fail +create table test_fk_p1 partition of test_fk_p for values from (10) to (20) using columnar; +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables +HINT: Consider an AFTER STATEMENT trigger instead. +create table test_fk_p2(i int) using columnar; +-- attach columnar partition should fail +alter table test_fk_p attach partition test_fk_p2 for values from (20) to (30); +ERROR: Foreign keys and AFTER ROW triggers are not supported for columnar tables +HINT: Consider an AFTER STATEMENT trigger instead. +drop table test_fk_p; +drop table test_pk; create table test_tr(i int) using columnar; -- we should be able to clean-up and continue gracefully if we -- error out in AFTER STATEMENT triggers. diff --git a/src/test/regress/sql/alter_table_set_access_method.sql b/src/test/regress/sql/alter_table_set_access_method.sql index 72f529fc5..487139593 100644 --- a/src/test/regress/sql/alter_table_set_access_method.sql +++ b/src/test/regress/sql/alter_table_set_access_method.sql @@ -93,22 +93,11 @@ SELECT "Name", "Citus Table Type", "Distribution Column", "Shard Count", "Access SELECT c.relname, a.amname FROM pg_class c, pg_am a where c.relname SIMILAR TO 'table_type\D*' AND c.relnamespace = 'alter_table_set_access_method'::regnamespace AND c.relam = a.oid; -- test when the parent of a partition has foreign key to a reference table -CREATE TABLE ref_table (a INT UNIQUE); -SELECT create_reference_table('ref_table'); -INSERT INTO ref_table VALUES (2), (12); -ALTER TABLE partitioned_table ADD CONSTRAINT fkey_to_ref FOREIGN KEY (a) REFERENCES ref_table(a); - -SELECT inhrelid::regclass::text FROM pg_catalog.pg_inherits WHERE inhparent = 'partitioned_table'::regclass ORDER BY 1; -SELECT "Name", "Access Method" FROM public.citus_tables WHERE "Name"::text = 'partitioned_table_6_10'; -SELECT conrelid::regclass::text AS "Referencing Table", pg_get_constraintdef(oid, true) AS "Definition" FROM pg_constraint - WHERE (conrelid::regclass::text = 'partitioned_table_6_10') ORDER BY 1, 2; - -SELECT alter_table_set_access_method('partitioned_table_6_10', 'columnar'); - -SELECT inhrelid::regclass::text FROM pg_catalog.pg_inherits WHERE inhparent = 'partitioned_table'::regclass ORDER BY 1; -SELECT "Name", "Access Method" FROM public.citus_tables WHERE "Name"::text = 'partitioned_table_6_10'; -SELECT conrelid::regclass::text AS "Referencing Table", pg_get_constraintdef(oid, true) AS "Definition" FROM pg_constraint - WHERE (conrelid::regclass::text = 'partitioned_table_6_10') ORDER BY 1, 2; +create table test_pk(n int primary key); +create table test_fk_p(i int references test_pk(n)) partition by range(i); +create table test_fk_p0 partition of test_fk_p for values from (0) to (10); +create table test_fk_p1 partition of test_fk_p for values from (10) to (20); +select alter_table_set_access_method('test_fk_p1', 'columnar'); SET client_min_messages TO WARNING; DROP SCHEMA alter_table_set_access_method CASCADE; diff --git a/src/test/regress/sql/am_trigger.sql b/src/test/regress/sql/am_trigger.sql index 5abb3a4ea..d8489fe4c 100644 --- a/src/test/regress/sql/am_trigger.sql +++ b/src/test/regress/sql/am_trigger.sql @@ -61,6 +61,41 @@ insert into test_tr values(2),(3),(4); SELECT * FROM test_tr ORDER BY i; drop table test_tr; + +-- +-- test implicit triggers created by foreign keys or partitions of a +-- table with a trigger +-- + +create table test_tr_referenced(i int primary key); +-- should fail when creating FK trigger +create table test_tr_referencing(j int references test_tr_referenced(i)) using columnar; +drop table test_tr_referenced; + +create table test_tr_p(i int) partition by range(i); +create trigger test_tr_p_tr after update on test_tr_p + for each row execute procedure trr_after(); +create table test_tr_p0 partition of test_tr_p + for values from (0) to (10); +-- create columnar partition should fail +create table test_tr_p1 partition of test_tr_p + for values from (10) to (20) using columnar; +create table test_tr_p2(i int) using columnar; +-- attach columnar partition should fail +alter table test_tr_p attach partition test_tr_p2 for values from (20) to (30); +drop table test_tr_p; + +create table test_pk(n int primary key); +create table test_fk_p(i int references test_pk(n)) partition by range(i); +create table test_fk_p0 partition of test_fk_p for values from (0) to (10); +-- create columnar partition should fail +create table test_fk_p1 partition of test_fk_p for values from (10) to (20) using columnar; +create table test_fk_p2(i int) using columnar; +-- attach columnar partition should fail +alter table test_fk_p attach partition test_fk_p2 for values from (20) to (30); +drop table test_fk_p; +drop table test_pk; + create table test_tr(i int) using columnar; -- we should be able to clean-up and continue gracefully if we