From 31858c8a2939cfa8defe2abc34ad6fa1332d1636 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Thu, 15 Oct 2020 12:00:44 +0200 Subject: [PATCH] Check table existence in EnsureRelationKindSupported --- .../commands/create_distributed_table.c | 19 +++++++++++++++++++ .../distributed/deparser/citus_ruleutils.c | 6 ++++++ .../distributed/metadata/metadata_cache.c | 18 ++++++++++++++++++ src/include/distributed/metadata_cache.h | 1 + src/test/regress/bin/normalize.sed | 3 +++ .../regress/expected/undistribute_table.out | 8 ++++++++ src/test/regress/sql/undistribute_table.sql | 4 ++++ 7 files changed, 59 insertions(+) diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index b7b190d18..2ec094f0d 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -112,6 +112,7 @@ static void EnsureLocalTableEmptyIfNecessary(Oid relationId, char distributionMe static bool ShouldLocalTableBeEmpty(Oid relationId, char distributionMethod, bool viaDeprecatedAPI); static void EnsureCitusTableCanBeCreated(Oid relationOid); +static void EnsureRelationExists(Oid relationId); static bool LocalTableEmpty(Oid tableId); static void CopyLocalDataIntoShards(Oid relationId); static List * TupleDescColumnNameList(TupleDesc tupleDescriptor); @@ -323,6 +324,7 @@ undistribute_table(PG_FUNCTION_ARGS) CheckCitusVersion(ERROR); EnsureCoordinator(); + EnsureRelationExists(relationId); EnsureTableOwner(relationId); UndistributeTable(relationId); @@ -341,6 +343,7 @@ static void EnsureCitusTableCanBeCreated(Oid relationOid) { EnsureCoordinator(); + EnsureRelationExists(relationOid); EnsureTableOwner(relationOid); /* @@ -352,6 +355,22 @@ EnsureCitusTableCanBeCreated(Oid relationOid) } +/* + * EnsureRelationExists does a basic check on whether the OID belongs to + * an existing relation. + */ +static void +EnsureRelationExists(Oid relationId) +{ + if (!RelationExists(relationId)) + { + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("relation with OID %d does not exist", + relationId))); + } +} + + /* * CreateDistributedTable creates distributed table in the given configuration. * This functions contains all necessary logic to create distributed tables. It diff --git a/src/backend/distributed/deparser/citus_ruleutils.c b/src/backend/distributed/deparser/citus_ruleutils.c index 2dd90a722..e053890cb 100644 --- a/src/backend/distributed/deparser/citus_ruleutils.c +++ b/src/backend/distributed/deparser/citus_ruleutils.c @@ -478,6 +478,12 @@ void EnsureRelationKindSupported(Oid relationId) { char relationKind = get_rel_relkind(relationId); + if (!relationKind) + { + ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("relation with OID %d does not exist", + relationId))); + } bool supportedRelationKind = RegularTable(relationId) || relationKind == RELKIND_FOREIGN_TABLE; diff --git a/src/backend/distributed/metadata/metadata_cache.c b/src/backend/distributed/metadata/metadata_cache.c index 62941966f..e9ade9d56 100644 --- a/src/backend/distributed/metadata/metadata_cache.c +++ b/src/backend/distributed/metadata/metadata_cache.c @@ -4262,6 +4262,24 @@ CachedRelationNamespaceLookup(const char *relationName, Oid relnamespace, } +/* + * RelationExists returns whether a relation with the given OID exists. + */ +bool +RelationExists(Oid relationId) +{ + HeapTuple relTuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relationId)); + + bool relationExists = HeapTupleIsValid(relTuple); + if (relationExists) + { + ReleaseSysCache(relTuple); + } + + return relationExists; +} + + /* * Register a relcache invalidation for a non-shared relation. * diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index 63e3296c1..30ecc2eb9 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -189,6 +189,7 @@ extern bool MajorVersionsCompatible(char *leftVersion, char *rightVersion); extern void ErrorIfInconsistentShardIntervals(CitusTableCacheEntry *cacheEntry); extern void EnsureModificationsCanRun(void); extern char LookupDistributionMethod(Oid distributionMethodOid); +extern bool RelationExists(Oid relationId); /* access WorkerNodeHash */ extern HTAB * GetWorkerNodeHash(void); diff --git a/src/test/regress/bin/normalize.sed b/src/test/regress/bin/normalize.sed index 16e63a21a..ca75eaf68 100644 --- a/src/test/regress/bin/normalize.sed +++ b/src/test/regress/bin/normalize.sed @@ -159,3 +159,6 @@ s/failed to roll back prepared transaction '.*'/failed to roll back prepared tra # Errors with binary decoding where OIDs should be normalized s/wrong data type: [0-9]+, expected [0-9]+/wrong data type: XXXX, expected XXXX/g + +# Errors with relation OID does not exist +s/relation with OID [0-9]+ does not exist/relation with OID XXXX does not exist/g diff --git a/src/test/regress/expected/undistribute_table.out b/src/test/regress/expected/undistribute_table.out index 0b61ac674..a15631efb 100644 --- a/src/test/regress/expected/undistribute_table.out +++ b/src/test/regress/expected/undistribute_table.out @@ -31,6 +31,14 @@ SELECT * FROM dist_table ORDER BY 1, 2, 3; 2 | 3 | abcd (3 rows) +-- we cannot immediately convert in the same statement, because +-- the name->OID conversion happens at parse time. +SELECT undistribute_table('dist_table'), create_distributed_table('dist_table', 'a'); +NOTICE: Creating a new local table for undistribute_table.dist_table +NOTICE: Moving the data of undistribute_table.dist_table +NOTICE: Dropping the old undistribute_table.dist_table +NOTICE: Renaming the new table to undistribute_table.dist_table +ERROR: relation with OID XXXX does not exist SELECT undistribute_table('dist_table'); NOTICE: Creating a new local table for undistribute_table.dist_table NOTICE: Moving the data of undistribute_table.dist_table diff --git a/src/test/regress/sql/undistribute_table.sql b/src/test/regress/sql/undistribute_table.sql index 5043b3444..ca393c5d9 100644 --- a/src/test/regress/sql/undistribute_table.sql +++ b/src/test/regress/sql/undistribute_table.sql @@ -11,6 +11,10 @@ SELECT logicalrelid FROM pg_dist_partition WHERE logicalrelid = 'dist_table'::re SELECT run_command_on_workers($$SELECT COUNT(*) FROM pg_catalog.pg_class WHERE relname LIKE 'dist\_table\_%'$$); SELECT * FROM dist_table ORDER BY 1, 2, 3; +-- we cannot immediately convert in the same statement, because +-- the name->OID conversion happens at parse time. +SELECT undistribute_table('dist_table'), create_distributed_table('dist_table', 'a'); + SELECT undistribute_table('dist_table'); SELECT logicalrelid FROM pg_dist_partition WHERE logicalrelid = 'dist_table'::regclass;