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

Stronger check for triggers on columnar tables (#4493). (#4494) 

* Stronger check for triggers on columnar tables (#4493).

Previously, we used a simple ProcessUtility_hook. Change to use an
object_access_hook instead.

* Replace alter_table_set_access_method test on partition with foreign key

Co-authored-by: Jeff Davis <jefdavi@microsoft.com>
Co-authored-by: Marco Slot <marco.slot@gmail.com>
pull/4495/head
jeff-davis 2021-01-13 10:30:53 -08:00 committed by GitHub
parent b79af16dac
commit b49beda4c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 176 additions and 182 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

175
src/backend/columnar/cstore_tableam.c
View File

@ -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.

14
src/include/columnar/cstore_version_compat.h
View File

@ -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) \

71
src/test/regress/expected/alter_table_set_access_method.out
View File

@ -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);

42
src/test/regress/expected/am_trigger.out
View File

@ -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.

21
src/test/regress/sql/alter_table_set_access_method.sql
View File

@ -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;

35
src/test/regress/sql/am_trigger.sql
View File

@ -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