On Fri, Dec 27, 2019 at 1:59 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Ashutosh Sharma <ashu.coe...@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
