From 6a62ec605b5b5cee72cf1697304032641a44f862 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <m.orlov@postgrespro.ru>
Date: Fri, 27 Nov 2020 13:39:54 +0300
Subject: [PATCH 1/4] Use more detailed message in record insert error.

In case of inserting a erroneous record in variable show clear messages, if it is
amount of fields or fields types are wrong.
---
 expected/pg_variables.out |  6 +++---
 pg_variables_record.c     | 11 +++++++----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/expected/pg_variables.out b/expected/pg_variables.out
index ba0b64a..2e67ab4 100644
--- a/expected/pg_variables.out
+++ b/expected/pg_variables.out
@@ -563,11 +563,11 @@ ERROR:  variable "j1" requires "jsonb" value
 SELECT pgv_insert('vars3', 'r1', tab) FROM tab;
 ERROR:  there is a record in the variable "r1" with same key
 SELECT pgv_insert('vars3', 'r1', row(1, 'str1', 'str2'));
-ERROR:  new record structure differs from variable "r1" structure
+ERROR:  new record structure have 3 attributes, but variable "r1" structure have 2.
 SELECT pgv_insert('vars3', 'r1', row(1, 1));
-ERROR:  new record structure differs from variable "r1" structure
+ERROR:  new record attribute type for attribute number 2 differs from variable "r1" structure. You may need explicit type casts.
 SELECT pgv_insert('vars3', 'r1', row('str1', 'str1'));
-ERROR:  new record structure differs from variable "r1" structure
+ERROR:  new record attribute type for attribute number 1 differs from variable "r1" structure. You may need explicit type casts.
 SELECT pgv_select('vars3', 'r1', ARRAY[[1,2]]); -- fail
 ERROR:  searching for elements in multidimensional arrays is not supported
 -- Test variables caching
diff --git a/pg_variables_record.c b/pg_variables_record.c
index b97ffca..d34d12c 100644
--- a/pg_variables_record.c
+++ b/pg_variables_record.c
@@ -188,8 +188,9 @@ check_attributes(Variable *variable, TupleDesc tupdesc)
 	if (record->tupdesc->natts != tupdesc->natts)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("new record structure differs from variable \"%s\" "
-						"structure", GetName(variable))));
+				 errmsg("new record structure have %d attributes, but variable "
+						"\"%s\" structure have %d.",
+						tupdesc->natts, GetName(variable), record->tupdesc->natts)));
 
 	/* Second, check columns type. */
 	for (i = 0; i < tupdesc->natts; i++)
@@ -202,8 +203,10 @@ check_attributes(Variable *variable, TupleDesc tupdesc)
 			|| (attr1->atttypmod != attr2->atttypmod))
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("new record structure differs from variable \"%s\" "
-							"structure", GetName(variable))));
+					 errmsg("new record attribute type for attribute number %d "
+							"differs from variable \"%s\" structure. You may "
+							"need explicit type casts.",
+							i + 1, GetName(variable))));
 	}
 }
 

From 4408d5adc7304666a1db051379cf71e9ff7677c2 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <m.orlov@postgrespro.ru>
Date: Fri, 27 Nov 2020 14:46:52 +0300
Subject: [PATCH 2/4] Minor refactoring for insert deleted variable.

---
 pg_variables.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/pg_variables.c b/pg_variables.c
index 4bd6bdd..0350a85 100644
--- a/pg_variables.c
+++ b/pg_variables.c
@@ -696,13 +696,15 @@ variable_insert(PG_FUNCTION_ARGS)
 	tupTypmod = HeapTupleHeaderGetTypMod(rec);
 
 	record = &(GetActualValue(variable).record);
-	if (!record->tupdesc)
+	tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
+
+	if (!record->tupdesc || variable->is_deleted)
 	{
 		/*
 		 * This is the first record for the var_name. Initialize record.
 		 */
-		tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
 		init_record(record, tupdesc, variable);
+		variable->is_deleted = false;
 	}
 	else if (LastTypeId == RECORDOID || !OidIsValid(LastTypeId) ||
 		LastTypeId != tupType)
@@ -711,16 +713,7 @@ variable_insert(PG_FUNCTION_ARGS)
 		 * We need to check attributes of the new row if this is a transient
 		 * record type or if last record has different id.
 		 */
-		tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-		if (variable->is_deleted)
-		{
-			init_record(record, tupdesc, variable);
-			variable->is_deleted = false;
-		}
-		else
-		{
-			check_attributes(variable, tupdesc);
-		}
+		check_attributes(variable, tupdesc);
 	}
 
 	LastTypeId = tupType;

From 1b37d6629e89d0b3105eb56f1e86ccfd0c794e62 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <m.orlov@postgrespro.ru>
Date: Wed, 2 Dec 2020 16:22:11 +0300
Subject: [PATCH 3/4] Fix error on insert from table #32

---
 expected/pg_variables.out       | 56 +++++++++++++++++++++++++++++++++
 expected/pg_variables_trans.out | 56 +++++++++++++++++++++++++++++++++
 pg_variables.c                  | 23 +++-----------
 sql/pg_variables.sql            | 16 ++++++++++
 sql/pg_variables_trans.sql      | 18 +++++++++++
 5 files changed, 151 insertions(+), 18 deletions(-)

diff --git a/expected/pg_variables.out b/expected/pg_variables.out
index 2e67ab4..e21ec67 100644
--- a/expected/pg_variables.out
+++ b/expected/pg_variables.out
@@ -929,3 +929,59 @@ SELECT * FROM pgv_list() order by package, name;
 ---------+------+------------------
 (0 rows)
 
+-- Check insert of record with various amount of fields
+CREATE TEMP TABLE foo(id int, t text);
+INSERT INTO foo VALUES (0, 'str00');
+SELECT pgv_insert('vars', 'r1', row(1, 'str1'::text, 'str2'::text));
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_select('vars', 'r1');
+  pgv_select   
+---------------
+ (1,str1,str2)
+(1 row)
+
+SELECT pgv_insert('vars', 'r1', foo) FROM foo;
+ERROR:  new record structure have 2 attributes, but variable "r1" structure have 3.
+SELECT pgv_select('vars', 'r1');
+  pgv_select   
+---------------
+ (1,str1,str2)
+(1 row)
+
+SELECT pgv_insert('vars', 'r2', row(1, 'str1'));
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_insert('vars', 'r2', foo) FROM foo;
+ERROR:  new record attribute type for attribute number 2 differs from variable "r2" structure. You may need explicit type casts.
+SELECT pgv_select('vars', 'r2');
+ pgv_select 
+------------
+ (1,str1)
+(1 row)
+
+SELECT pgv_insert('vars', 'r3', row(1, 'str1'::text));
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_insert('vars', 'r3', foo) FROM foo;
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_select('vars', 'r3');
+ pgv_select 
+------------
+ (1,str1)
+ (0,str00)
+(2 rows)
+
diff --git a/expected/pg_variables_trans.out b/expected/pg_variables_trans.out
index 2389efa..2f46e04 100644
--- a/expected/pg_variables_trans.out
+++ b/expected/pg_variables_trans.out
@@ -3773,3 +3773,59 @@ SELECT pgv_free();
  
 (1 row)
 
+---
+--- Test case for issue #32 [PGPRO-4456]
+---
+CREATE TEMP TABLE tab (id int, t varchar);
+INSERT INTO tab VALUES (0, 'str00');
+SELECT pgv_insert('vars', 'r1', row(1, 'str1', 'str2'));
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_insert('vars', 'a', tab) FROM tab;
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_insert('vars', 'r1', tab) FROM tab;
+ERROR:  new record structure have 2 attributes, but variable "r1" structure have 3.
+SELECT pgv_select('vars', 'r1');
+  pgv_select   
+---------------
+ (1,str1,str2)
+(1 row)
+
+SELECT pgv_insert('vars', 'r2', row(1, 'str1'::varchar));
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_insert('vars', 'b', tab) FROM tab;
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_insert('vars', 'r2', tab) FROM tab;
+ pgv_insert 
+------------
+ 
+(1 row)
+
+SELECT pgv_select('vars', 'r2');
+ pgv_select 
+------------
+ (1,str1)
+ (0,str00)
+(2 rows)
+
+SELECT pgv_free();
+ pgv_free 
+----------
+ 
+(1 row)
+
diff --git a/pg_variables.c b/pg_variables.c
index 0350a85..b75701a 100644
--- a/pg_variables.c
+++ b/pg_variables.c
@@ -121,8 +121,6 @@ static MemoryContext ModuleContext = NULL;
 static Package *LastPackage = NULL;
 /* Recent variable */
 static Variable *LastVariable = NULL;
-/* Recent row type id */
-static Oid LastTypeId = InvalidOid;
 
 /* Saved hook values for recall */
 static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
@@ -706,8 +704,7 @@ variable_insert(PG_FUNCTION_ARGS)
 		init_record(record, tupdesc, variable);
 		variable->is_deleted = false;
 	}
-	else if (LastTypeId == RECORDOID || !OidIsValid(LastTypeId) ||
-		LastTypeId != tupType)
+	else
 	{
 		/*
 		 * We need to check attributes of the new row if this is a transient
@@ -716,8 +713,6 @@ variable_insert(PG_FUNCTION_ARGS)
 		check_attributes(variable, tupdesc);
 	}
 
-	LastTypeId = tupType;
-
 	insert_record(variable, rec);
 
 	/* Release resources */
@@ -742,6 +737,7 @@ variable_update(PG_FUNCTION_ARGS)
 	bool		res;
 	Oid			tupType;
 	int32		tupTypmod;
+	TupleDesc	tupdesc = NULL;
 
 	/* Checks */
 	CHECK_ARGS_FOR_NULL();
@@ -794,17 +790,9 @@ variable_update(PG_FUNCTION_ARGS)
 	tupType = HeapTupleHeaderGetTypeId(rec);
 	tupTypmod = HeapTupleHeaderGetTypMod(rec);
 
-	if (LastTypeId == RECORDOID || !OidIsValid(LastTypeId) ||
-		LastTypeId != tupType)
-	{
-		TupleDesc	tupdesc = NULL;
-
-		tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
-		check_attributes(variable, tupdesc);
-		ReleaseTupleDesc(tupdesc);
-	}
-
-	LastTypeId = tupType;
+	tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
+	check_attributes(variable, tupdesc);
+	ReleaseTupleDesc(tupdesc);
 
 	res = update_record(variable, rec);
 
@@ -1330,7 +1318,6 @@ resetVariablesCache(void)
 	/* Remove package and variable from cache */
 	LastPackage = NULL;
 	LastVariable = NULL;
-	LastTypeId = InvalidOid;
 }
 
 /*
diff --git a/sql/pg_variables.sql b/sql/pg_variables.sql
index a9fdbbd..bad7976 100644
--- a/sql/pg_variables.sql
+++ b/sql/pg_variables.sql
@@ -259,3 +259,19 @@ SELECT pgv_free();
 SELECT pgv_exists('vars');
 
 SELECT * FROM pgv_list() order by package, name;
+-- Check insert of record with various amount of fields
+CREATE TEMP TABLE foo(id int, t text);
+INSERT INTO foo VALUES (0, 'str00');
+
+SELECT pgv_insert('vars', 'r1', row(1, 'str1'::text, 'str2'::text));
+SELECT pgv_select('vars', 'r1');
+SELECT pgv_insert('vars', 'r1', foo) FROM foo;
+SELECT pgv_select('vars', 'r1');
+
+SELECT pgv_insert('vars', 'r2', row(1, 'str1'));
+SELECT pgv_insert('vars', 'r2', foo) FROM foo;
+SELECT pgv_select('vars', 'r2');
+
+SELECT pgv_insert('vars', 'r3', row(1, 'str1'::text));
+SELECT pgv_insert('vars', 'r3', foo) FROM foo;
+SELECT pgv_select('vars', 'r3');
diff --git a/sql/pg_variables_trans.sql b/sql/pg_variables_trans.sql
index 0d67ae1..72d9559 100644
--- a/sql/pg_variables_trans.sql
+++ b/sql/pg_variables_trans.sql
@@ -1145,3 +1145,21 @@ COMMIT;
 
 DROP VIEW pgv_stats_view;
 SELECT pgv_free();
+
+---
+--- Test case for issue #32 [PGPRO-4456]
+---
+CREATE TEMP TABLE tab (id int, t varchar);
+INSERT INTO tab VALUES (0, 'str00');
+
+SELECT pgv_insert('vars', 'r1', row(1, 'str1', 'str2'));
+SELECT pgv_insert('vars', 'a', tab) FROM tab;
+SELECT pgv_insert('vars', 'r1', tab) FROM tab;
+SELECT pgv_select('vars', 'r1');
+
+SELECT pgv_insert('vars', 'r2', row(1, 'str1'::varchar));
+SELECT pgv_insert('vars', 'b', tab) FROM tab;
+SELECT pgv_insert('vars', 'r2', tab) FROM tab;
+SELECT pgv_select('vars', 'r2');
+
+SELECT pgv_free();

From 2e3f14f194c9ff2f420cb6fb94628f6671b0361f Mon Sep 17 00:00:00 2001
From: Maxim Orlov <m.orlov@postgrespro.ru>
Date: Wed, 9 Dec 2020 14:09:16 +0300
Subject: [PATCH 4/4] Use hint instead of long message #32

---
 expected/pg_variables.out | 9 ++++++---
 pg_variables_record.c     | 6 +++---
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/expected/pg_variables.out b/expected/pg_variables.out
index e21ec67..180ebd1 100644
--- a/expected/pg_variables.out
+++ b/expected/pg_variables.out
@@ -565,9 +565,11 @@ ERROR:  there is a record in the variable "r1" with same key
 SELECT pgv_insert('vars3', 'r1', row(1, 'str1', 'str2'));
 ERROR:  new record structure have 3 attributes, but variable "r1" structure have 2.
 SELECT pgv_insert('vars3', 'r1', row(1, 1));
-ERROR:  new record attribute type for attribute number 2 differs from variable "r1" structure. You may need explicit type casts.
+ERROR:  new record attribute type for attribute number 2 differs from variable "r1" structure.
+HINT:  You may need explicit type casts.
 SELECT pgv_insert('vars3', 'r1', row('str1', 'str1'));
-ERROR:  new record attribute type for attribute number 1 differs from variable "r1" structure. You may need explicit type casts.
+ERROR:  new record attribute type for attribute number 1 differs from variable "r1" structure.
+HINT:  You may need explicit type casts.
 SELECT pgv_select('vars3', 'r1', ARRAY[[1,2]]); -- fail
 ERROR:  searching for elements in multidimensional arrays is not supported
 -- Test variables caching
@@ -959,7 +961,8 @@ SELECT pgv_insert('vars', 'r2', row(1, 'str1'));
 (1 row)
 
 SELECT pgv_insert('vars', 'r2', foo) FROM foo;
-ERROR:  new record attribute type for attribute number 2 differs from variable "r2" structure. You may need explicit type casts.
+ERROR:  new record attribute type for attribute number 2 differs from variable "r2" structure.
+HINT:  You may need explicit type casts.
 SELECT pgv_select('vars', 'r2');
  pgv_select 
 ------------
diff --git a/pg_variables_record.c b/pg_variables_record.c
index d34d12c..3d7ca18 100644
--- a/pg_variables_record.c
+++ b/pg_variables_record.c
@@ -204,9 +204,9 @@ check_attributes(Variable *variable, TupleDesc tupdesc)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 					 errmsg("new record attribute type for attribute number %d "
-							"differs from variable \"%s\" structure. You may "
-							"need explicit type casts.",
-							i + 1, GetName(variable))));
+							"differs from variable \"%s\" structure.",
+							i + 1, GetName(variable)),
+					 errhint("You may need explicit type casts.")));
 	}
 }
 
