From cbc5e60abeb92b6b5f3dbf91bd298dffacda5ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstro=CC=88m?= Date: Fri, 29 Jun 2018 15:28:19 +0200 Subject: [PATCH] Block NO INHERIT constraints on hypertables Constraints with the NO INHERIT option does not make sense on a hypertable's root table since these will not be enforced. Previously, NO INHERIT constraints were blocked on chunks, and were thus not enforced until chunk creation time, allowing creation of NO INHERIT constraints on empty hypertables, but then causing failure at chunk-creation time. Instead, NO INHERIT constraints are now properly blocked at the hypertable level. --- src/chunk_constraint.c | 7 ------ src/hypertable.c | 46 ++++++++++++++++++++++++++++++++++++ src/process_utility.c | 7 ++++++ test/expected/constraint.out | 30 +++++++++++++++++++++++ test/sql/constraint.sql | 24 +++++++++++++++++++ 5 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/chunk_constraint.c b/src/chunk_constraint.c index e5e877532..01d7c82ba 100644 --- a/src/chunk_constraint.c +++ b/src/chunk_constraint.c @@ -581,13 +581,6 @@ chunk_constraint_need_on_chunk(Form_pg_constraint conform) * of constraints (unique, primary key, and foreign key constraints) * are not inherited." */ - if (conform->connoinherit) - { - ereport(ERROR, - (errcode(ERRCODE_IO_OPERATION_NOT_SUPPORTED), - errmsg("NO INHERIT option not supported on hypertables: %s", conform->conname.data) - )); - } return false; } return true; diff --git a/src/hypertable.c b/src/hypertable.c index 2ae7f1512..77809883d 100644 --- a/src/hypertable.c +++ b/src/hypertable.c @@ -11,6 +11,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -849,6 +852,46 @@ hypertable_create_schema(const char *schema_name) ); } +/* + * Check that existing table constraints are supported. + * + * Hypertables do not support some constraints. For instance, NO INHERIT + * constraints cannot be enforced on a hypertable since they only exist on the + * parent table, which will have no tuples. + */ +static void +hypertable_validate_constraints(Oid relid) +{ + Relation catalog; + SysScanDesc scan; + ScanKeyData scankey; + HeapTuple tuple; + + catalog = heap_open(ConstraintRelationId, AccessShareLock); + + ScanKeyInit(&scankey, Anum_pg_constraint_conrelid, BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(relid)); + + scan = systable_beginscan(catalog, ConstraintRelidIndexId, true, + NULL, 1, &scankey); + + while (HeapTupleIsValid(tuple = systable_getnext(scan))) + { + Form_pg_constraint form = (Form_pg_constraint) GETSTRUCT(tuple); + + if (form->contype == CONSTRAINT_CHECK && form->connoinherit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot have NO INHERIT constraints on hypertable \"%s\"", + get_rel_name(relid)), + errhint("Remove all NO INHERIT constraints from table \"%s\" before making it a hypertable.", + get_rel_name(relid)))); + } + + systable_endscan(scan); + heap_close(catalog, AccessShareLock); +} + TS_FUNCTION_INFO_V1(hypertable_create); /* @@ -947,6 +990,9 @@ hypertable_create(PG_FUNCTION_ARGS) errmsg("invalid relation type"))); } + /* Check that the table doesn't have any unsupported constraints */ + hypertable_validate_constraints(table_relid); + table_has_data = table_has_tuples(table_relid, GetActiveSnapshot(), NoLock); if (!migrate_data && table_has_data) diff --git a/src/process_utility.c b/src/process_utility.c index cfa6026f4..636f18e56 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -918,6 +918,13 @@ verify_constraint_hypertable(Hypertable *ht, Node *constr_node) contype = constr->contype; keys = (contype == CONSTR_EXCLUSION) ? constr->exclusions : constr->keys; indexname = constr->indexname; + + /* NO INHERIT constraints do not really make sense on a hypertable */ + if (constr->is_no_inherit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot have NO INHERIT constraints on hypertable \"%s\"", + get_rel_name(ht->main_table_relid)))); } else if (IsA(constr_node, IndexStmt)) { diff --git a/test/expected/constraint.out b/test/expected/constraint.out index acf79505b..3e163cf6f 100644 --- a/test/expected/constraint.out +++ b/test/expected/constraint.out @@ -545,3 +545,33 @@ SELECT * FROM create_hypertable('hyper_ex_invalid', 'time', chunk_time_interval= NOTICE: adding not-null constraint to column "time" ERROR: cannot create a unique index without the column "time" (used in partitioning) \set ON_ERROR_STOP 1 +--- NO INHERIT constraints (not allowed) ---- +CREATE TABLE hyper_noinherit ( + time BIGINT, + sensor_1 NUMERIC NULL DEFAULT 1 CHECK (sensor_1 > 0) NO INHERIT +); +SELECT * FROM test.show_constraints('hyper_noinherit'); + Constraint | Type | Columns | Index | Expr +--------------------------------+------+------------+-------+--------------------------- + hyper_noinherit_sensor_1_check | c | {sensor_1} | - | (sensor_1 > (0)::numeric) +(1 row) + +\set ON_ERROR_STOP 0 +SELECT * FROM create_hypertable('hyper_noinherit', 'time', chunk_time_interval=>_timescaledb_internal.interval_to_usec('1 month')); +ERROR: cannot have NO INHERIT constraints on hypertable "hyper_noinherit" +\set ON_ERROR_STOP 1 +CREATE TABLE hyper_noinherit_alter ( + time BIGINT, + sensor_1 NUMERIC NULL DEFAULT 1 +); +SELECT * FROM create_hypertable('hyper_noinherit_alter', 'time', chunk_time_interval=>_timescaledb_internal.interval_to_usec('1 month')); +NOTICE: adding not-null constraint to column "time" + create_hypertable +------------------- + +(1 row) + +\set ON_ERROR_STOP 0 +ALTER TABLE hyper_noinherit_alter ADD CONSTRAINT check_noinherit CHECK (sensor_1 > 0) NO INHERIT; +ERROR: cannot have NO INHERIT constraints on hypertable "hyper_noinherit_alter" +\set ON_ERROR_STOP 1 diff --git a/test/sql/constraint.sql b/test/sql/constraint.sql index eee0304eb..40e5a3af6 100644 --- a/test/sql/constraint.sql +++ b/test/sql/constraint.sql @@ -398,3 +398,27 @@ CREATE TABLE hyper_ex_invalid ( \set ON_ERROR_STOP 0 SELECT * FROM create_hypertable('hyper_ex_invalid', 'time', chunk_time_interval=>_timescaledb_internal.interval_to_usec('1 month')); \set ON_ERROR_STOP 1 + + +--- NO INHERIT constraints (not allowed) ---- +CREATE TABLE hyper_noinherit ( + time BIGINT, + sensor_1 NUMERIC NULL DEFAULT 1 CHECK (sensor_1 > 0) NO INHERIT +); + +SELECT * FROM test.show_constraints('hyper_noinherit'); + +\set ON_ERROR_STOP 0 +SELECT * FROM create_hypertable('hyper_noinherit', 'time', chunk_time_interval=>_timescaledb_internal.interval_to_usec('1 month')); +\set ON_ERROR_STOP 1 + +CREATE TABLE hyper_noinherit_alter ( + time BIGINT, + sensor_1 NUMERIC NULL DEFAULT 1 +); + +SELECT * FROM create_hypertable('hyper_noinherit_alter', 'time', chunk_time_interval=>_timescaledb_internal.interval_to_usec('1 month')); + +\set ON_ERROR_STOP 0 +ALTER TABLE hyper_noinherit_alter ADD CONSTRAINT check_noinherit CHECK (sensor_1 > 0) NO INHERIT; +\set ON_ERROR_STOP 1