From a2bad2b5f76b9ec2b7a126216c1a518707575b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstro=CC=88m?= Date: Fri, 29 Sep 2017 14:49:27 +0200 Subject: [PATCH] Fix constraint validation on regular tables Constraints on regular PostgreSQL tables need to be validatated so that they don't have a foreign key constraint that references a hypertable, as this is not yet supported. This change fixes a bug in this validation caused by not checking for the proper node types when parsing a CreateStmt node. The CreateStmt is now also parsed in the DDL end hook, instead of the processUtility hook, which ensures that the CreateStmt node has been run through parse analysis. The parse analysis changes the contents of the node to be more consistent across different CREATE statements. For instance, a CREATE LIKE statement will look more similar to a regular CREATE statement after parse analysis. --- src/process_utility.c | 62 +++++++++++++++++++++++++++------- test/expected/create_table.out | 23 +++++++++++++ test/sql/create_table.sql | 13 +++++++ 3 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 test/expected/create_table.out create mode 100644 test/sql/create_table.sql diff --git a/src/process_utility.c b/src/process_utility.c index 4f27b63734a..0e1a5d8a56a 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -579,11 +579,13 @@ process_altertable_drop_constraint(Hypertable *ht, AlterTableCmd *cmd) /* foreign-key constraints to hypertables are not allowed */ static void -verify_constraint(Constraint *stmt) +verify_constraint(Constraint *constr) { - if (stmt->contype == CONSTR_FOREIGN) + Assert(IsA(constr, Constraint)); + + if (constr->contype == CONSTR_FOREIGN) { - RangeVar *primary_table = stmt->pktable; + RangeVar *primary_table = constr->pktable; Oid primary_oid = RangeVarGetRelid(primary_table, NoLock, true); if (OidIsValid(primary_oid)) @@ -609,25 +611,58 @@ verify_constraint_list(List *constraint_list) foreach(lc, constraint_list) { - Constraint *constraint = (Constraint *) lfirst(lc); + Constraint *constraint = lfirst(lc); verify_constraint(constraint); } } -/* process all create table commands to make sure their constraints are kosher */ +/* + * Process create table statements. + * + * For regular tables, we need to ensure that they don't have any foreign key + * constraints that point to hypertables. + * + * NOTE that this function should be called after parse analysis (in an end DDL + * trigger or by running parse analysis manually). + */ static void -process_create_table(Node *parsetree) +process_create_table_end(Node *parsetree) { CreateStmt *stmt = (CreateStmt *) parsetree; ListCell *lc; verify_constraint_list(stmt->constraints); + + /* + * Only after parse analyis does tableElts contain only ColumnDefs. So, if + * we capture this in processUtility, we should be prepared to have + * constraint nodes and TableLikeClauses intermixed + */ foreach(lc, stmt->tableElts) { - ColumnDef *column_def = (ColumnDef *) lfirst(lc); + ColumnDef *coldef; + + switch (nodeTag(lfirst(lc))) + { + case T_ColumnDef: + coldef = lfirst(lc); + verify_constraint_list(coldef->constraints); + break; + case T_Constraint: - verify_constraint_list(column_def->constraints); + /* + * There should be no Constraints in the list after parse + * analysis, but this case is included anyway for completeness + */ + verify_constraint(lfirst(lc)); + break; + case T_TableLikeClause: + /* Some as above case */ + break; + default: + break; + } } } @@ -833,7 +868,7 @@ create_trigger_chunk(Oid hypertable_relid, Oid chunk_relid, void *arg) } static void -process_create_trigger(Node *parsetree) +process_create_trigger_end(Node *parsetree) { CreateTrigStmt *stmt = (CreateTrigStmt *) parsetree; @@ -868,9 +903,6 @@ process_ddl_command_start(Node *parsetree, case T_RenameStmt: process_rename(parsetree); break; - case T_CreateStmt: - process_create_table(parsetree); - break; case T_DropStmt: /* @@ -912,11 +944,14 @@ process_ddl_command_end(CollectedCommand *cmd) { switch (nodeTag(cmd->parsetree)) { + case T_CreateStmt: + process_create_table_end(cmd->parsetree); + break; case T_AlterTableStmt: process_altertable_end(cmd->parsetree, cmd); break; case T_CreateTrigStmt: - process_create_trigger(cmd->parsetree); + process_create_trigger_end(cmd->parsetree); break; default: break; @@ -973,6 +1008,7 @@ timescaledb_ddl_command_end(PG_FUNCTION_ARGS) { case T_AlterTableStmt: case T_CreateTrigStmt: + case T_CreateStmt: foreach(lc, event_trigger_ddl_commands()) process_ddl_command_end(lfirst(lc)); break; diff --git a/test/expected/create_table.out b/test/expected/create_table.out new file mode 100644 index 00000000000..25eddac8bc7 --- /dev/null +++ b/test/expected/create_table.out @@ -0,0 +1,23 @@ +\ir include/create_single_db.sql +SET client_min_messages = WARNING; +DROP DATABASE IF EXISTS single; +SET client_min_messages = NOTICE; +CREATE DATABASE single; +\c single +CREATE EXTENSION IF NOT EXISTS timescaledb; +SET timescaledb.disable_optimizations = :DISABLE_OPTIMIZATIONS; +-- Test that we can verify constraints on regular tables +CREATE TABLE test_hyper_pk(time TIMESTAMPTZ PRIMARY KEY, temp FLOAT, device INT); +CREATE TABLE test_pk(device INT PRIMARY KEY); +CREATE TABLE test_like(LIKE test_pk); +SELECT create_hypertable('test_hyper_pk', 'time'); + create_hypertable +------------------- + +(1 row) + +\set ON_ERROR_STOP 0 +-- Foreign key constraints that reference hypertables are currently unsupported +CREATE TABLE test_fk(time TIMESTAMPTZ REFERENCES test_hyper_pk(time)); +ERROR: Foreign keys to hypertables are not supported. +\set ON_ERROR_STOP 1 diff --git a/test/sql/create_table.sql b/test/sql/create_table.sql new file mode 100644 index 00000000000..21dc729de41 --- /dev/null +++ b/test/sql/create_table.sql @@ -0,0 +1,13 @@ +\ir include/create_single_db.sql + +-- Test that we can verify constraints on regular tables +CREATE TABLE test_hyper_pk(time TIMESTAMPTZ PRIMARY KEY, temp FLOAT, device INT); +CREATE TABLE test_pk(device INT PRIMARY KEY); +CREATE TABLE test_like(LIKE test_pk); + +SELECT create_hypertable('test_hyper_pk', 'time'); + +\set ON_ERROR_STOP 0 +-- Foreign key constraints that reference hypertables are currently unsupported +CREATE TABLE test_fk(time TIMESTAMPTZ REFERENCES test_hyper_pk(time)); +\set ON_ERROR_STOP 1