Calling PLpgSQL function with composite type fails with an error: "ERROR: could not open relation with OID ..."
Hi All,
When the following test-case is executed on master, it fails with an
error: "ERROR: could not open relation with OID ..."
-- create a test table:
create table tab1(a int, b text);
-- create a test function:
create or replace function f1() returns void as
$$
declare
var1 tab1;
begin
select * into var1 from tab1;
end
$$ language plpgsql;
-- call the test function:
select f1();
-- drop the test table and re-create it:
drop table tab1;
create table tab1(a int, b text);
-- call the test function:
select f1();
-- call the test function once again:
select f1(); -- this fails with an error "ERROR: could not open
relation with OID .."
I'm trying to investigate this issue and will try to share my findings soon...
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
The issue here is that PLpgSQL_rec structure being updated by
revalidate_rectypeid() is actually a local/duplicate copy of the
PLpgSQL_rec structure available in plpgsql_HashTable (refer to
copy_plpgsql_datums() where you would notice that if datum type is
PLPGSQL_DTYPE_REC we actually mempcy() the PLpgSQL_rec structure
available in func >datums[] array). This basically means that the
rectypeid field updated post typcache entry validation in
revalidation_rectypeid() is actually a field in duplicate copy of
PLpgSQL_rec structure, not the original copy of it available in
func->datums[]. Hence, when the same function is executed for the
second time, the rectypeid field of PLpgSQL_rec structure being
reloaded from the func->datums[] actually contains the stale value
however the typcache entry in it is up-to-date which means
revalidation_rectypeid() returns immediately leaving a stale value in
rectypeid. This causes the function make_expanded_record_from_typeid()
to use the outdated value in rec->rectypeid resulting into the given
error.
To fix this, I think instead of using rec->rectypeid field we should
try using rec->datatype->typoid when calling
make_expanded_record_from_typeid(). Here is the change that I'm
suggesting:
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -6942,7 +6942,7 @@ make_expanded_record_for_rec(PLpgSQL_execstate *estate,
newerh = make_expanded_record_from_exprecord(srcerh,
mcontext);
else
- newerh = make_expanded_record_from_typeid(rec->rectypeid, -1,
+ newerh = make_expanded_record_from_typeid(rec->datatype->typoid, -1,Thoughts ?
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
Show quoted text
On Thu, Dec 26, 2019 at 3:57 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
Hi All,
When the following test-case is executed on master, it fails with an
error: "ERROR: could not open relation with OID ..."-- create a test table:
create table tab1(a int, b text);-- create a test function:
create or replace function f1() returns void as
$$
declare
var1 tab1;
begin
select * into var1 from tab1;
end
$$ language plpgsql;-- call the test function:
select f1();-- drop the test table and re-create it:
drop table tab1;
create table tab1(a int, b text);-- call the test function:
select f1();-- call the test function once again:
select f1(); -- this fails with an error "ERROR: could not open
relation with OID .."I'm trying to investigate this issue and will try to share my findings soon...
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
Ashutosh Sharma <ashu.coek88@gmail.com> writes:
The issue here is that PLpgSQL_rec structure being updated by
revalidate_rectypeid() is actually a local/duplicate copy of the
PLpgSQL_rec structure available in plpgsql_HashTable (refer to
copy_plpgsql_datums() where you would notice that if datum type is
PLPGSQL_DTYPE_REC we actually mempcy() the PLpgSQL_rec structure
available in func >datums[] array). This basically means that the
rectypeid field updated post typcache entry validation in
revalidation_rectypeid() is actually a field in duplicate copy of
PLpgSQL_rec structure, not the original copy of it available in
func->datums[]. Hence, when the same function is executed for the
second time, the rectypeid field of PLpgSQL_rec structure being
reloaded from the func->datums[] actually contains the stale value
however the typcache entry in it is up-to-date which means
revalidation_rectypeid() returns immediately leaving a stale value in
rectypeid. This causes the function make_expanded_record_from_typeid()
to use the outdated value in rec->rectypeid resulting into the given
error.
Good catch!
To fix this, I think instead of using rec->rectypeid field we should
try using rec->datatype->typoid when calling
make_expanded_record_from_typeid().
This is a crummy fix, though. In the first place, if we did it like this
we'd have to fix every other caller of revalidate_rectypeid() likewise.
Basically the issue here is that revalidate_rectypeid() is failing to do
what it says on the tin, and you're proposing to make the callers work
around that instead of fixing revalidate_rectypeid(). That seems like
an odd choice from here.
More generally, the reason for the separation between PLpgSQL_rec and
PLpgSQL_type in this part of the code is that PLpgSQL_rec.rectypeid is
supposed to record the actual type ID currently instantiated in that
variable (in the current function execution), whereas PLpgSQL_type is a
cache for the last type lookup we did; that's why it's okay to share the
latter but not the former across function executions. So failing to
update rec->rectypeid is almost certainly going to lead to problems
later on.
I pushed a fix that makes revalidate_rectypeid() deal with this case.
Thanks for the report and debugging!
regards, tom lane
On Fri, Dec 27, 2019 at 1:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Ashutosh Sharma <ashu.coek88@gmail.com> writes:
The issue here is that PLpgSQL_rec structure being updated by
revalidate_rectypeid() is actually a local/duplicate copy of the
PLpgSQL_rec structure available in plpgsql_HashTable (refer to
copy_plpgsql_datums() where you would notice that if datum type is
PLPGSQL_DTYPE_REC we actually mempcy() the PLpgSQL_rec structure
available in func >datums[] array). This basically means that the
rectypeid field updated post typcache entry validation in
revalidation_rectypeid() is actually a field in duplicate copy of
PLpgSQL_rec structure, not the original copy of it available in
func->datums[]. Hence, when the same function is executed for the
second time, the rectypeid field of PLpgSQL_rec structure being
reloaded from the func->datums[] actually contains the stale value
however the typcache entry in it is up-to-date which means
revalidation_rectypeid() returns immediately leaving a stale value in
rectypeid. This causes the function make_expanded_record_from_typeid()
to use the outdated value in rec->rectypeid resulting into the given
error.Good catch!
To fix this, I think instead of using rec->rectypeid field we should
try using rec->datatype->typoid when calling
make_expanded_record_from_typeid().This is a crummy fix, though. In the first place, if we did it like this
we'd have to fix every other caller of revalidate_rectypeid() likewise.
Basically the issue here is that revalidate_rectypeid() is failing to do
what it says on the tin, and you're proposing to make the callers work
around that instead of fixing revalidate_rectypeid(). That seems like
an odd choice from here.More generally, the reason for the separation between PLpgSQL_rec and
PLpgSQL_type in this part of the code is that PLpgSQL_rec.rectypeid is
supposed to record the actual type ID currently instantiated in that
variable (in the current function execution), whereas PLpgSQL_type is a
cache for the last type lookup we did; that's why it's okay to share the
latter but not the former across function executions. So failing to
update rec->rectypeid is almost certainly going to lead to problems
later on.I pushed a fix that makes revalidate_rectypeid() deal with this case.
Thanks for the report and debugging!
Okay. Thanks for that fix. You've basically forced
revalidate_rectypeid() to update the PLpgSQL_rec's rectypeid
irrespective of typcache entry requires re-validation or not.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
Ashutosh Sharma <ashu.coek88@gmail.com> writes:
Okay. Thanks for that fix. You've basically forced
revalidate_rectypeid() to update the PLpgSQL_rec's rectypeid
irrespective of typcache entry requires re-validation or not.
Right. The assignment is cheap enough that it hardly seems
worth avoiding.
regards, tom lane
On Fri, Dec 27, 2019 at 9:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Ashutosh Sharma <ashu.coek88@gmail.com> writes:
Okay. Thanks for that fix. You've basically forced
revalidate_rectypeid() to update the PLpgSQL_rec's rectypeid
irrespective of typcache entry requires re-validation or not.Right. The assignment is cheap enough that it hardly seems
worth avoiding.
Agreed.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com