From 1843ec62be5a36c573c4706da0aa2a3e9cd599e3 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 31 Mar 2017 15:58:36 +0300 Subject: [PATCH] Fix SQL escaping bug in internal query. The old comment was unsure if toast tables and indexes have their Oids in sync across the cluster. They do, so let's rely on that, which is simpler. --- src/backend/cdb/cdbrelsize.c | 18 +++--------------- src/test/regress/expected/alter_table_gp.out | 4 ++++ .../expected/alter_table_gp_optimizer.out | 4 ++++ src/test/regress/sql/alter_table_gp.sql | 6 ++++++ 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/backend/cdb/cdbrelsize.c b/src/backend/cdb/cdbrelsize.c index a02bc1be4c..033e09379b 100644 --- a/src/backend/cdb/cdbrelsize.c +++ b/src/backend/cdb/cdbrelsize.c @@ -8,8 +8,6 @@ */ #include "postgres.h" -#include - #include "utils/lsyscache.h" #include "utils/relcache.h" #include "utils/syscache.h" @@ -37,26 +35,16 @@ cdbRelMaxSegSize(Relation rel) CdbPgResults cdb_pgresults = {NULL, 0}; StringInfoData buffer; - char *schemaName; - char *relName; - /* * Let's ask the QEs for the size of the relation */ initStringInfo(&buffer); - schemaName = get_namespace_name(RelationGetNamespace(rel)); - if (schemaName == NULL) - elog(ERROR, "cache lookup failed for namespace %d", - RelationGetNamespace(rel)); - relName = RelationGetRelationName(rel); - /* - * Safer to pass names than oids, just in case they get out of sync between QD and QE, - * which might happen with a toast table or index, I think (but maybe not) + * Relation Oids are assumed to be in sync in all nodes. */ - appendStringInfo(&buffer, "select pg_relation_size('%s.%s')", - quote_identifier(schemaName), quote_identifier(relName)); + appendStringInfo(&buffer, "select pg_relation_size(%u)", + RelationGetRelid(rel)); CdbDispatchCommand(buffer.data, DF_WITH_SNAPSHOT, &cdb_pgresults); diff --git a/src/test/regress/expected/alter_table_gp.out b/src/test/regress/expected/alter_table_gp.out index 5476f18966..ebaf15a3b3 100644 --- a/src/test/regress/expected/alter_table_gp.out +++ b/src/test/regress/expected/alter_table_gp.out @@ -125,3 +125,7 @@ SELECT * FROM altable ORDER BY 1; 0 | 1 (1 row) +-- Verify that changing the datatype of a funnily-named column works. +-- (There used to be a quoting bug in the internal query this issues.) +create table "foo'bar" (id int4, t text); +alter table "foo'bar" alter column t type integer using length(t); diff --git a/src/test/regress/expected/alter_table_gp_optimizer.out b/src/test/regress/expected/alter_table_gp_optimizer.out index 1aa7cf276e..a9165bb11a 100644 --- a/src/test/regress/expected/alter_table_gp_optimizer.out +++ b/src/test/regress/expected/alter_table_gp_optimizer.out @@ -127,3 +127,7 @@ SELECT * FROM altable ORDER BY 1; 0 | 1 (1 row) +-- Verify that changing the datatype of a funnily-named column works. +-- (There used to be a quoting bug in the internal query this issues.) +create table "foo'bar" (id int4, t text); +alter table "foo'bar" alter column t type integer using length(t); diff --git a/src/test/regress/sql/alter_table_gp.sql b/src/test/regress/sql/alter_table_gp.sql index 8204f57d1b..3a23c937bf 100644 --- a/src/test/regress/sql/alter_table_gp.sql +++ b/src/test/regress/sql/alter_table_gp.sql @@ -82,3 +82,9 @@ UPDATE altable SET c = -10; SELECT * FROM altable ORDER BY 1; UPDATE altable SET c = 1; SELECT * FROM altable ORDER BY 1; + + +-- Verify that changing the datatype of a funnily-named column works. +-- (There used to be a quoting bug in the internal query this issues.) +create table "foo'bar" (id int4, t text); +alter table "foo'bar" alter column t type integer using length(t); -- GitLab