Subject: Re: BUG #16242: convert_tuple_* not handling missing values correctly
To: Andres Freund <andres@anarazel.de>
Cc: andrew@tao11.riddles.org.uk, pgsql-bugs@lists.postgresql.org
References: <16242-d1c9fca28445966b@postgresql.org>
 <31edb400-f065-73c4-7b1f-2953ad3413e6@postgresfriends.org>
 <20200205021933.c4wtkpcx2li3um2k@alap3.anarazel.de>
 <3c396c9e-960f-9c03-63b7-d58e0e044fba@postgresfriends.org>
 <20200205022634.77y7fg7vyhn2svh7@alap3.anarazel.de>
From: Vik Fearing <vik@postgresfriends.org>
Message-ID: <5e0747c0-68af-c937-b8f7-c11b12126d7a@postgresfriends.org>
Date: Wed, 5 Feb 2020 03:38:06 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
MIME-Version: 1.0
In-Reply-To: <20200205022634.77y7fg7vyhn2svh7@alap3.anarazel.de>
Content-Type: multipart/mixed;
 boundary="------------28E735DE413BB18AA8B5AAFF"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------28E735DE413BB18AA8B5AAFF
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

On 05/02/2020 03:26, Andres Freund wrote:
> On 2020-02-05 03:21:52 +0100, Vik Fearing wrote:
>> On 05/02/2020 03:19, Andres Freund wrote:
>>> Should include a regression test...
>>
>> That's what I thought, too.  Where should I put it?
> 
> alter.sql?

Okay, here you go.
-- 
Vik Fearing

--------------28E735DE413BB18AA8B5AAFF
Content-Type: text/x-patch; charset=UTF-8;
 name="bug16242_master.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="bug16242_master.diff"

diff --git a/src/backend/access/common/attmap.c b/src/backend/access/common/attmap.c
index a074852a01..2cd16d7eaf 100644
--- a/src/backend/access/common/attmap.c
+++ b/src/backend/access/common/attmap.c
@@ -294,8 +294,14 @@ check_attrmap_match(TupleDesc indesc,
 
 	for (i = 0; i < attrMap->maplen; i++)
 	{
-		Form_pg_attribute inatt;
-		Form_pg_attribute outatt;
+		Form_pg_attribute inatt = TupleDescAttr(indesc, i);
+		Form_pg_attribute outatt = TupleDescAttr(outdesc, i);
+
+		/*
+		 * If the input column has a missing attribute, we need a conversion.
+		 */
+		if (inatt->atthasmissing)
+			return false;
 
 		if (attrMap->attnums[i] == (i + 1))
 			continue;
@@ -305,8 +311,6 @@ check_attrmap_match(TupleDesc indesc,
 		 * dropped, we don't need a conversion.  However, attlen and attalign
 		 * must agree.
 		 */
-		inatt = TupleDescAttr(indesc, i);
-		outatt = TupleDescAttr(outdesc, i);
 		if (attrMap->attnums[i] == 0 &&
 			inatt->attisdropped &&
 			inatt->attlen == outatt->attlen &&
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e73ce4b6f3..a04f1c6756 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4258,3 +4258,41 @@ alter table at_test_sql_partop attach partition at_test_sql_partop_1 for values
 drop table at_test_sql_partop;
 drop operator class at_test_sql_partop using btree;
 drop function at_test_sql_partop;
+/* Test case for bug #16242 */
+-- partitioned table (parent)
+create table bar1 (a integer, b integer not null default 1)
+  partition by range (a);
+-- this will become a partition:
+create table bar2 (a integer);
+insert into bar2 values (1);
+alter table bar2 add column b integer not null default 1;
+-- (at this point bar2 contains tuple with natts=1)
+alter table bar1 attach partition bar2 default;
+-- this works:
+select * from bar1;
+ a | b 
+---+---
+ 1 | 1
+(1 row)
+
+-- this doesn't:
+create function xtrig()
+  returns trigger
+  language plpgsql
+as $$
+  declare
+    r record;
+  begin
+    for r in select * from old loop
+      raise info 'a=%, b=%', r.a, r.b;
+    end loop;
+    return NULL;
+  end;
+$$;
+create trigger xtrig
+  after update on bar1
+  referencing old table as old
+  for each statement execute procedure xtrig();
+update bar1 set a = a + 1;
+INFO:  a=1, b=1
+/* End test case for bug #16242 */
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index a16e4c9a29..1f6327296a 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2801,3 +2801,48 @@ alter table at_test_sql_partop attach partition at_test_sql_partop_1 for values
 drop table at_test_sql_partop;
 drop operator class at_test_sql_partop using btree;
 drop function at_test_sql_partop;
+
+
+/* Test case for bug #16242 */
+
+-- partitioned table (parent)
+create table bar1 (a integer, b integer not null default 1)
+  partition by range (a);
+
+-- this will become a partition:
+create table bar2 (a integer);
+insert into bar2 values (1);
+alter table bar2 add column b integer not null default 1;
+
+-- (at this point bar2 contains tuple with natts=1)
+
+alter table bar1 attach partition bar2 default;
+
+-- this works:
+
+select * from bar1;
+
+-- this doesn't:
+
+create function xtrig()
+  returns trigger
+  language plpgsql
+as $$
+  declare
+    r record;
+  begin
+    for r in select * from old loop
+      raise info 'a=%, b=%', r.a, r.b;
+    end loop;
+    return NULL;
+  end;
+$$;
+
+create trigger xtrig
+  after update on bar1
+  referencing old table as old
+  for each statement execute procedure xtrig();
+
+update bar1 set a = a + 1;
+
+/* End test case for bug #16242 */

--------------28E735DE413BB18AA8B5AAFF--