From 4152a391c2f4e216fe69f576d895638a7626b975 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Fri, 14 Oct 2022 17:31:25 +0300 Subject: [PATCH] Properly set col names for shard rels that citus_extradata_container points to (#6428) Deparser function set_relation_column_names() knows that it needs to re-evaluate column names based on relation's tuple descriptor when the rte belongs to a relation (RTE_RELATION). However before this commit, it didn't know about the fact that citus might wrap such an rte with an rte that points to citus_extradata_container() placeholder. And because of this, it was simply taking the column aliases (e.g., "bar" in "foo AS bar") into the account and this might result in an incorrectly deparsed query as in below case: * Say, if we had view based on following query: ```sql SELECT a FROM table; ``` * And if we rename column "a" to "b", the view query normally becomes: ```sql SELECT b AS a FROM table; ``` * So before this commit, deparsing a query based on that view was resulting in such a query due to deparsing based on the column aliases, which is not correct: ```sql SELECT a FROM table; ``` Fixes #5932. DESCRIPTION: Fixes a bug that might cause failing to query the views based on tables that have renamed columns --- .../distributed/deparser/ruleutils_13.c | 3 +- .../distributed/deparser/ruleutils_14.c | 3 +- .../distributed/deparser/ruleutils_15.c | 3 +- src/test/regress/expected/multi_view.out | 51 +++++++++++++++++++ src/test/regress/sql/multi_view.sql | 41 +++++++++++++++ 5 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/backend/distributed/deparser/ruleutils_13.c b/src/backend/distributed/deparser/ruleutils_13.c index d7e222d53..86eeb19a1 100644 --- a/src/backend/distributed/deparser/ruleutils_13.c +++ b/src/backend/distributed/deparser/ruleutils_13.c @@ -1003,7 +1003,8 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte, * get_rte_attribute_name, except that it's important to disregard dropped * columns. We put NULL into the array for a dropped column. */ - if (rte->rtekind == RTE_RELATION) + if (rte->rtekind == RTE_RELATION || + GetRangeTblKind(rte) == CITUS_RTE_SHARD) { /* Relation --- look to the system catalogs for up-to-date info */ Relation rel; diff --git a/src/backend/distributed/deparser/ruleutils_14.c b/src/backend/distributed/deparser/ruleutils_14.c index 9ae1f0a92..08615e61c 100644 --- a/src/backend/distributed/deparser/ruleutils_14.c +++ b/src/backend/distributed/deparser/ruleutils_14.c @@ -1125,7 +1125,8 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte, * get_rte_attribute_name, except that it's important to disregard dropped * columns. We put NULL into the array for a dropped column. */ - if (rte->rtekind == RTE_RELATION) + if (rte->rtekind == RTE_RELATION || + GetRangeTblKind(rte) == CITUS_RTE_SHARD) { /* Relation --- look to the system catalogs for up-to-date info */ Relation rel; diff --git a/src/backend/distributed/deparser/ruleutils_15.c b/src/backend/distributed/deparser/ruleutils_15.c index 128442a69..103c515bd 100644 --- a/src/backend/distributed/deparser/ruleutils_15.c +++ b/src/backend/distributed/deparser/ruleutils_15.c @@ -1133,7 +1133,8 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte, * real_colnames[] will be indexed by physical column number, with NULL * entries for dropped columns. */ - if (rte->rtekind == RTE_RELATION) + if (rte->rtekind == RTE_RELATION || + GetRangeTblKind(rte) == CITUS_RTE_SHARD) { /* Relation --- look to the system catalogs for up-to-date info */ Relation rel; diff --git a/src/test/regress/expected/multi_view.out b/src/test/regress/expected/multi_view.out index 70fa10874..fafa242df 100644 --- a/src/test/regress/expected/multi_view.out +++ b/src/test/regress/expected/multi_view.out @@ -1330,6 +1330,57 @@ SELECT * FROM ref_1 ORDER BY key, value; --------------------------------------------------------------------- (0 rows) +-- show that we correctly handle renamed columns when expanding view query +CREATE TABLE column_alias_test_1(a int, b int); +SELECT create_distributed_table('column_alias_test_1', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO column_alias_test_1 VALUES (1, 2); +CREATE VIEW column_alias_test_1_view AS +SELECT * FROM column_alias_test_1; +ALTER TABLE column_alias_test_1 RENAME COLUMN a TO tmp; +ALTER TABLE column_alias_test_1 RENAME COLUMN b TO a; +ALTER TABLE column_alias_test_1 RENAME COLUMN tmp TO b; +-- A tricky implication of #5932: +-- +-- Even if we renamed column names, this should normally print: +-- a | b +-- 1 | 2 +-- +-- This is because, views preserve original column names by +-- aliasing renamed columns. +-- +-- But before fixing #5932, this was printing: +-- a | b +-- 2 | 1 +-- +-- which was not correct. +SELECT * FROM column_alias_test_1_view; + a | b +--------------------------------------------------------------------- + 1 | 2 +(1 row) + +CREATE TABLE column_alias_test_2(a int, b int); +SELECT create_distributed_table('column_alias_test_2', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO column_alias_test_2 VALUES (1, 2); +CREATE VIEW column_alias_test_2_view AS +SELECT * FROM column_alias_test_2; +ALTER TABLE column_alias_test_2 RENAME COLUMN a TO a_renamed; +SELECT * FROM column_alias_test_2_view; + a | b +--------------------------------------------------------------------- + 1 | 2 +(1 row) + DROP TABLE large, small, ref_1 CASCADE; NOTICE: drop cascades to 4 other objects DETAIL: drop cascades to view v1 diff --git a/src/test/regress/sql/multi_view.sql b/src/test/regress/sql/multi_view.sql index 193d669f6..3165f50c4 100644 --- a/src/test/regress/sql/multi_view.sql +++ b/src/test/regress/sql/multi_view.sql @@ -673,5 +673,46 @@ CREATE MATERIALIZED VIEW v3 AS SELECT id AS col1 FROM small; DELETE FROM ref_1 WHERE value in (SELECT col1 FROM v3); SELECT * FROM ref_1 ORDER BY key, value; +-- show that we correctly handle renamed columns when expanding view query + +CREATE TABLE column_alias_test_1(a int, b int); +SELECT create_distributed_table('column_alias_test_1', 'a'); + +INSERT INTO column_alias_test_1 VALUES (1, 2); + +CREATE VIEW column_alias_test_1_view AS +SELECT * FROM column_alias_test_1; + +ALTER TABLE column_alias_test_1 RENAME COLUMN a TO tmp; +ALTER TABLE column_alias_test_1 RENAME COLUMN b TO a; +ALTER TABLE column_alias_test_1 RENAME COLUMN tmp TO b; + +-- A tricky implication of #5932: +-- +-- Even if we renamed column names, this should normally print: +-- a | b +-- 1 | 2 +-- +-- This is because, views preserve original column names by +-- aliasing renamed columns. +-- +-- But before fixing #5932, this was printing: +-- a | b +-- 2 | 1 +-- +-- which was not correct. +SELECT * FROM column_alias_test_1_view; + +CREATE TABLE column_alias_test_2(a int, b int); +SELECT create_distributed_table('column_alias_test_2', 'a'); + +INSERT INTO column_alias_test_2 VALUES (1, 2); + +CREATE VIEW column_alias_test_2_view AS +SELECT * FROM column_alias_test_2; + +ALTER TABLE column_alias_test_2 RENAME COLUMN a TO a_renamed; + +SELECT * FROM column_alias_test_2_view; DROP TABLE large, small, ref_1 CASCADE;