Skip to content

Commit

Permalink
Fix rename and reindex bugs when objects are not relations
Browse files Browse the repository at this point in the history
When issuing rename or reindex commands that do not involve
relations, the relation rangevar in the utility statement
will be NULL.

This change adds extra checks to avoid calling, e.g.,
RangeVarGetRelid() when a relation's rangevar is NULL, as
that function does not accept NULL input values.
  • Loading branch information
erimatnor committed Oct 5, 2017
1 parent c3ebc67 commit 4c451e0
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 13 deletions.
63 changes: 50 additions & 13 deletions src/process_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,17 @@ process_truncate(Node *parsetree)

foreach(cell, truncatestmt->relations)
{
Oid relId = RangeVarGetRelid(lfirst(cell), NoLock, true);
RangeVar *relation = lfirst(cell);
Oid relid;

if (OidIsValid(relId))
if (NULL == relation)
continue;

relid = RangeVarGetRelid(relation, NoLock, true);

if (OidIsValid(relid))
{
Hypertable *ht = hypertable_cache_get_entry(hcache, relId);
Hypertable *ht = hypertable_cache_get_entry(hcache, relid);

if (ht != NULL)
{
Expand All @@ -172,7 +178,7 @@ process_alterobjectschema(Node *parsetree)
Cache *hcache;
Hypertable *ht;

if (alterstmt->objectType != OBJECT_TABLE)
if (alterstmt->objectType != OBJECT_TABLE || NULL == alterstmt->relation)
return;

relid = RangeVarGetRelid(alterstmt->relation, NoLock, true);
Expand Down Expand Up @@ -202,7 +208,7 @@ process_copy(Node *parsetree, const char *query_string, char *completion_tag)
Cache *hcache;
Oid relid;

if (!stmt->is_from)
if (!stmt->is_from || NULL == stmt->relation)
return false;

relid = RangeVarGetRelid(stmt->relation, NoLock, true);
Expand Down Expand Up @@ -326,7 +332,13 @@ process_drop_table(DropStmt *stmt)
foreach(lc, stmt->objects)
{
List *object = lfirst(lc);
Oid relid = RangeVarGetRelid(makeRangeVarFromNameList(object), NoLock, true);
RangeVar *relation = makeRangeVarFromNameList(object);
Oid relid;

if (NULL == relation)
continue;

relid = RangeVarGetRelid(relation, NoLock, true);

if (OidIsValid(relid))
{
Expand Down Expand Up @@ -428,8 +440,13 @@ process_drop_index(DropStmt *stmt)
foreach(lc, stmt->objects)
{
List *object = lfirst(lc);
RangeVar *rv = makeRangeVarFromNameList(object);
Oid idxrelid = RangeVarGetRelid(rv, NoLock, true);
RangeVar *relation = makeRangeVarFromNameList(object);
Oid idxrelid;

if (NULL == relation)
continue;

idxrelid = RangeVarGetRelid(relation, NoLock, true);

if (OidIsValid(idxrelid))
{
Expand Down Expand Up @@ -511,7 +528,16 @@ static bool
process_reindex(Node *parsetree)
{
ReindexStmt *stmt = (ReindexStmt *) parsetree;
Oid relid = RangeVarGetRelid(stmt->relation, NoLock, true);
Oid relid;

if (NULL == stmt->relation)
/* Not a case we are interested in */
return false;

relid = RangeVarGetRelid(stmt->relation, NoLock, true);

if (!OidIsValid(relid))
return false;

switch (stmt->kind)
{
Expand Down Expand Up @@ -571,8 +597,13 @@ process_rename_column(Cache *hcache, Oid relid, RenameStmt *stmt)
static void
process_rename_index(Cache *hcache, Oid relid, RenameStmt *stmt)
{
Oid tablerelid = IndexGetRelation(relid, false);
Hypertable *ht = hypertable_cache_get_entry(hcache, tablerelid);
Oid tablerelid = IndexGetRelation(relid, true);
Hypertable *ht;

if (!OidIsValid(tablerelid))
return;

ht = hypertable_cache_get_entry(hcache, tablerelid);

if (NULL != ht)
chunk_index_rename_parent(ht, relid, stmt->newname);
Expand All @@ -589,14 +620,20 @@ static void
process_rename(Node *parsetree)
{
RenameStmt *stmt = (RenameStmt *) parsetree;
Oid relid = RangeVarGetRelid(stmt->relation, NoLock, true);
Oid relid;
Cache *hcache;

/* TODO: forbid all rename op on chunk table */
if (NULL == stmt->relation)
/* Not an object we are interested in */
return;

relid = RangeVarGetRelid(stmt->relation, NoLock, true);

if (!OidIsValid(relid))
return;

/* TODO: forbid all rename op on chunk table */

hcache = hypertable_cache_pin();

switch (stmt->renameType)
Expand Down
44 changes: 44 additions & 0 deletions test/expected/plain.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
\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;
-- Tests for plain PostgreSQL commands to ensure that they work while
-- the TimescaleDB extension is loaded. This is a mix of statements
-- added mostly as regression checks when bugs are discovered and
-- fixed.
CREATE TABLE regular_table(time timestamp, temp float8, tag text, color integer);
-- Renaming indexes should work
CREATE INDEX time_color_idx ON regular_table(time, color);
ALTER INDEX time_color_idx RENAME TO time_color_idx2;
\d+ regular_table
Table "public.regular_table"
Column | Type | Modifiers | Storage | Stats target | Description
--------+-----------------------------+-----------+----------+--------------+-------------
time | timestamp without time zone | | plain | |
temp | double precision | | plain | |
tag | text | | extended | |
color | integer | | plain | |
Indexes:
"time_color_idx2" btree ("time", color)

-- Renaming types should work
CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple');
ALTER TYPE rainbow RENAME TO colors;
\dT+
List of data types
Schema | Name | Internal name | Size | Elements | Owner | Access privileges | Description
--------+--------+---------------+------+----------+----------+-------------------+-------------
public | colors | colors | 4 | red +| postgres | |
| | | | orange +| | |
| | | | yellow +| | |
| | | | green +| | |
| | | | blue +| | |
| | | | purple | | |
(1 row)

REINDEX TABLE regular_table;
REINDEX SCHEMA public;
23 changes: 23 additions & 0 deletions test/sql/plain.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
\ir include/create_single_db.sql

-- Tests for plain PostgreSQL commands to ensure that they work while
-- the TimescaleDB extension is loaded. This is a mix of statements
-- added mostly as regression checks when bugs are discovered and
-- fixed.

CREATE TABLE regular_table(time timestamp, temp float8, tag text, color integer);

-- Renaming indexes should work
CREATE INDEX time_color_idx ON regular_table(time, color);
ALTER INDEX time_color_idx RENAME TO time_color_idx2;

\d+ regular_table

-- Renaming types should work
CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple');
ALTER TYPE rainbow RENAME TO colors;

\dT+

REINDEX TABLE regular_table;
REINDEX SCHEMA public;

0 comments on commit 4c451e0

Please sign in to comment.