Re: Badly broken logic in plpython composite-type handling

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: Badly broken logic in plpython composite-type handling
Date: 2015-08-20 00:05:48
Message-ID: 28643.1440029148@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> I looked into the crash reported in bug #13579. The proximate cause
> of the crash is that PLyString_ToComposite does this:
> ...
> I'm inclined to think that maybe PLyString_ToComposite needs to be doing
> something more like what PLyObject_ToComposite does, ie doing its own
> lookup in a private descriptor; but I'm not sure if that's right, and
> anyway it would just be doubling down on a bad design. Not being able
> to cache these I/O function lookups is really horrid.

I wrote a draft patch that does it that way. It indeed stops the crash,
and even better, makes PLyString_ToComposite actually work for RECORD,
which AFAICT it's never done before. We'd conveniently omitted to test
the case in the plpython regression tests, thus masking the fact that
it would crash horribly if you invoked such a case more than once.

To make it work, I had to modify record_in to allow inputting a value
of type RECORD as long as it's given a correct typmod. While that would
not happen in ordinary SQL, it's possible in this and probably other
usages, and I can't really see any downside. The emitted record will
be properly marked with type RECORDOID and typmod whatever the internal
anonymous-type identifier is, and it's no more or less type-safe than
any other such value.

This version of PLyString_ToComposite is no better than
PLyObject_ToComposite as far as leaking memory goes. We could probably
fix that in a crude fashion by using plpython's "scratch context" for the
transient type-lookup data, but I'd rather see a proper fix wherein the
lookup results stay cached. In any case, that's a separate and less
critical matter.

Barring objections, I propose to commit and back-patch this. The crash
can be demonstrated back to 9.1.

regards, tom lane

Attachment Content-Type Size
plpython-returns-record-fix.patch text/x-diff 15.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joshua D. Drake 2015-08-20 00:09:48 Re: Badly broken logic in plpython composite-type handling
Previous Message Jim Nasby 2015-08-19 23:20:40 Re: proposal: function parse_ident