On Sat, Oct 7, 2017 at 1:04 AM, Robert Haas <[email protected]> wrote:
>>
>> The fix is to copy the relevant partitioning information from relcache
>> into PartitionSchemeData and RelOptInfo. Here's a quick patch with
>> that fix.
>
> Committed. I hope that makes things less red rather than more,
> because I'm going to be AFK for a few hours anyway.
>
set_append_rel_size() crashes when it encounters a partitioned table
with a dropped column. Dropped columns do not have any translations
saved in AppendInfo::translated_vars; the corresponding entry is NULL
per make_inh_translation_list().
1802 att = TupleDescAttr(old_tupdesc, old_attno);
1803 if (att->attisdropped)
1804 {
1805 /* Just put NULL into this list entry */
1806 vars = lappend(vars, NULL);
1807 continue;
1808 }
In set_append_rel_size() we try to attr_needed for child tables. While
doing so we try to translate a user attribute number of parent to that
of a child and crash since the translated Var is NULL. Here's patch to
fix the crash. The patch also contains a testcase to test dropped
columns in partitioned table.
Sorry for not noticing this problem earlier.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From eca9e03410b049bf79d767657ad4d90fb974d19c Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Mon, 16 Oct 2017 13:23:49 +0530
Subject: [PATCH] Ignore dropped columns in set_append_rel_size().
A parent Var corresponding to column dropped from a parent table will
not have any translation for a child. It won't have any attr_needed
information since it can not be referenced from SQL. Hence ignore
dropped columns while constructing attr_needed information for a child
table.
Ashutosh Bapat.
---
src/backend/optimizer/path/allpaths.c | 12 ++++++++++++
src/test/regress/expected/alter_table.out | 7 +++++++
src/test/regress/sql/alter_table.sql | 4 ++++
3 files changed, 23 insertions(+)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 5535b63..1bc5e01 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -950,6 +950,18 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
attno - 1);
int child_index;
+ /*
+ * Ignore any column dropped from the parent.
+ * Corresponding Var won't have any translation. It won't
+ * have attr_needed information, since it can not be
+ * referenced in the query.
+ */
+ if (var == NULL)
+ {
+ Assert(attr_needed == NULL);
+ continue;
+ }
+
child_index = var->varattno - childrel->min_attr;
childrel->attr_needed[child_index] = attr_needed;
}
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index dbe438d..cc56651 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3604,6 +3604,13 @@ ALTER TABLE list_parted2 DROP COLUMN b;
ERROR: cannot drop column named in partition key
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
ERROR: cannot alter type of column named in partition key
+-- dropping non-partition key columns should be allowed on the parent table.
+ALTER TABLE list_parted DROP COLUMN b;
+SELECT * FROM list_parted;
+ a
+---
+(0 rows)
+
-- cleanup
DROP TABLE list_parted, list_parted2, range_parted;
DROP TABLE fail_def_part;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 0c8ae2a..fc7bd66 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2390,6 +2390,10 @@ ALTER TABLE part_2 INHERIT inh_test;
ALTER TABLE list_parted2 DROP COLUMN b;
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
+-- dropping non-partition key columns should be allowed on the parent table.
+ALTER TABLE list_parted DROP COLUMN b;
+SELECT * FROM list_parted;
+
-- cleanup
DROP TABLE list_parted, list_parted2, range_parted;
DROP TABLE fail_def_part;
--
1.7.9.5
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
