Badly broken logic in plpython composite-type handling - Mailing list pgsql-hackers

From Tom Lane
Subject Badly broken logic in plpython composite-type handling
Date
Msg-id 12727.1440002795@sss.pgh.pa.us
Whole thread Raw
Responses Re: Badly broken logic in plpython composite-type handling  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I looked into the crash reported in bug #13579.  The proximate cause
of the crash is that PLyString_ToComposite does this:
  PLy_output_datum_func2(&info->out.d, typeTup,                         exec_ctx->curr_proc->langid,
    exec_ctx->curr_proc->trftypes);
 

without any thought for the possibility that info->is_rowtype is 1,
in which case it's stomping on the info->out.r data that will be
needed later.  I have serious doubts that making PLyTypeOutput a
union was a wise idea, as it means that C offers you exactly zero
protection against this type of data clobber.

I tried changing that to
  PLy_output_datum_func(info, typeTup,                        exec_ctx->curr_proc->langid,
exec_ctx->curr_proc->trftypes);

which unsurprisingly results in "ERROR:  PLyTypeInfo struct is initialized
for a Tuple" in the case reported in the bug ... and also results in quite
a few of the plpython regression tests failing that way, which makes it a
bit astonishing that we've not tripped over this bug already.

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 wonder if that could be fixed by changing PLyTypeInput and PLyTypeOutput
from unions to structs and getting rid of is_rowtype in favor of allowing
the d and r fields to be valid or not independently; then you could treat
the object as either scalar or rowtype at need.

Does whoever designed this code in the first place want to fix it?
        regards, tom lane



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: allowing wal_level change at run time
Next
From: David Fetter
Date:
Subject: Re: how to write/setup a C trigger function in a background worker