From 6a62ec605b5b5cee72cf1697304032641a44f862 Mon Sep 17 00:00:00 2001 From: Maxim Orlov 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 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 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 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."))); } }