Обсуждение: 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
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