Skip to content

Commit

Permalink
Fix constraint validation on regular tables
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
erimatnor committed Oct 3, 2017
1 parent bf6fd31 commit a2bad2b
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 13 deletions.
62 changes: 49 additions & 13 deletions src/process_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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;
}
}
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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:

/*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 23 additions & 0 deletions test/expected/create_table.out
Original file line number Diff line number Diff line change
@@ -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
13 changes: 13 additions & 0 deletions test/sql/create_table.sql
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a2bad2b

Please sign in to comment.