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