From 5fde61722968f45e79d070e0e80a60921de379ea Mon Sep 17 00:00:00 2001 From: Hadi Moshayedi Date: Wed, 3 Feb 2021 12:10:00 -0800 Subject: [PATCH] Columnar: disallow CREATE INDEX CONCURRENTLY --- src/backend/columnar/cstore_tableam.c | 56 +++++++++++++++++++++ src/test/regress/columnar_am_schedule | 1 + src/test/regress/expected/am_indexes.out | 62 ++++++++++++++++++++++++ src/test/regress/sql/am_indexes.sql | 29 +++++++++++ 4 files changed, 148 insertions(+) create mode 100644 src/test/regress/expected/am_indexes.out create mode 100644 src/test/regress/sql/am_indexes.sql diff --git a/src/backend/columnar/cstore_tableam.c b/src/backend/columnar/cstore_tableam.c index 1178b94b4..c845f519f 100644 --- a/src/backend/columnar/cstore_tableam.c +++ b/src/backend/columnar/cstore_tableam.c @@ -93,6 +93,7 @@ typedef struct ColumnarScanDescData typedef struct ColumnarScanDescData *ColumnarScanDesc; static object_access_hook_type PrevObjectAccessHook = NULL; +static ProcessUtility_hook_type PrevProcessUtilityHook = NULL; /* forward declaration for static functions */ static void ColumnarTableDropHook(Oid tgid); @@ -100,6 +101,13 @@ static void ColumnarTriggerCreateHook(Oid tgid); static void ColumnarTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId, int subId, void *arg); +static void ColumnarProcessUtility(PlannedStmt *pstmt, + const char *queryString, + ProcessUtilityContext context, + ParamListInfo params, + struct QueryEnvironment *queryEnv, + DestReceiver *dest, + QueryCompletionCompat *completionTag); static bool ConditionalLockRelationWithTimeout(Relation rel, LOCKMODE lockMode, int timeout, int retryInterval); static void LogRelationStats(Relation rel, int elevel); @@ -1127,6 +1135,11 @@ columnar_tableam_init() PrevObjectAccessHook = object_access_hook; object_access_hook = ColumnarTableAMObjectAccessHook; + PrevProcessUtilityHook = ProcessUtility_hook ? + ProcessUtility_hook : + standard_ProcessUtility; + ProcessUtility_hook = ColumnarProcessUtility; + columnar_customscan_init(); TTSOpsColumnar = TTSOpsVirtual; @@ -1292,6 +1305,49 @@ ColumnarTableAMObjectAccessHook(ObjectAccessType access, Oid classId, Oid object } +/* + * Utility hook for columnar tables. + */ +static void +ColumnarProcessUtility(PlannedStmt *pstmt, + const char *queryString, + ProcessUtilityContext context, + ParamListInfo params, + struct QueryEnvironment *queryEnv, + DestReceiver *dest, + QueryCompletionCompat *completionTag) +{ + Node *parsetree = pstmt->utilityStmt; + + if (IsA(parsetree, IndexStmt)) + { + IndexStmt *indexStmt = (IndexStmt *) parsetree; + + /* + * We should reject CREATE INDEX CONCURRENTLY before DefineIndex() is + * called. Erroring in callbacks called from DefineIndex() will create + * the index and mark it as INVALID, which will cause segfault during + * inserts. + */ + if (indexStmt->concurrent) + { + Relation rel = relation_openrv(indexStmt->relation, + ShareUpdateExclusiveLock); + if (rel->rd_tableam == GetColumnarTableAmRoutine()) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("indexes not supported for columnar tables"))); + } + + RelationClose(rel); + } + } + + PrevProcessUtilityHook(pstmt, queryString, context, + params, queryEnv, dest, completionTag); +} + + /* * IsColumnarTableAmTable returns true if relation has columnar_tableam * access method. This can be called before extension creation. diff --git a/src/test/regress/columnar_am_schedule b/src/test/regress/columnar_am_schedule index a4afb3229..f1a2498e1 100644 --- a/src/test/regress/columnar_am_schedule +++ b/src/test/regress/columnar_am_schedule @@ -8,6 +8,7 @@ test: am_query test: am_analyze test: am_data_types test: am_drop +test: am_indexes test: columnar_fallback_scan test: columnar_partitioning test: am_empty diff --git a/src/test/regress/expected/am_indexes.out b/src/test/regress/expected/am_indexes.out new file mode 100644 index 000000000..bd1c41802 --- /dev/null +++ b/src/test/regress/expected/am_indexes.out @@ -0,0 +1,62 @@ +-- +-- Testing indexes on on columnar tables. +-- +CREATE SCHEMA columnar_indexes; +SET search_path tO columnar_indexes, public; +-- +-- create index with the concurrent option. We should +-- error out during index creation. +-- https://github.com/citusdata/citus/issues/4599 +-- +create table t(a int, b int) using columnar; +create index CONCURRENTLY t_idx on t(a, b); +ERROR: indexes not supported for columnar tables +\d t + Table "columnar_indexes.t" + Column | Type | Collation | Nullable | Default +--------------------------------------------------------------------- + a | integer | | | + b | integer | | | + +explain insert into t values (1, 2); + QUERY PLAN +--------------------------------------------------------------------- + Insert on t (cost=0.00..0.01 rows=1 width=8) + -> Result (cost=0.00..0.01 rows=1 width=8) +(2 rows) + +insert into t values (1, 2); +SELECT * FROM t; + a | b +--------------------------------------------------------------------- + 1 | 2 +(1 row) + +-- create index without the concurrent option. We should +-- error out during index creation. +create index t_idx on t(a, b); +ERROR: indexes not supported for columnar tables +\d t + Table "columnar_indexes.t" + Column | Type | Collation | Nullable | Default +--------------------------------------------------------------------- + a | integer | | | + b | integer | | | + +explain insert into t values (1, 2); + QUERY PLAN +--------------------------------------------------------------------- + Insert on t (cost=0.00..0.01 rows=1 width=8) + -> Result (cost=0.00..0.01 rows=1 width=8) +(2 rows) + +insert into t values (3, 4); +SELECT * FROM t; + a | b +--------------------------------------------------------------------- + 1 | 2 + 3 | 4 +(2 rows) + +SET client_min_messages TO WARNING; +DROP SCHEMA columnar_indexes CASCADE; diff --git a/src/test/regress/sql/am_indexes.sql b/src/test/regress/sql/am_indexes.sql new file mode 100644 index 000000000..831699dc4 --- /dev/null +++ b/src/test/regress/sql/am_indexes.sql @@ -0,0 +1,29 @@ +-- +-- Testing indexes on on columnar tables. +-- + +CREATE SCHEMA columnar_indexes; +SET search_path tO columnar_indexes, public; + +-- +-- create index with the concurrent option. We should +-- error out during index creation. +-- https://github.com/citusdata/citus/issues/4599 +-- +create table t(a int, b int) using columnar; +create index CONCURRENTLY t_idx on t(a, b); +\d t +explain insert into t values (1, 2); +insert into t values (1, 2); +SELECT * FROM t; + +-- create index without the concurrent option. We should +-- error out during index creation. +create index t_idx on t(a, b); +\d t +explain insert into t values (1, 2); +insert into t values (3, 4); +SELECT * FROM t; + +SET client_min_messages TO WARNING; +DROP SCHEMA columnar_indexes CASCADE;