Thread: Fwd: [BUGS] fix: plpgsql: return query and dropped columns problem

Fwd: [BUGS] fix: plpgsql: return query and dropped columns problem

From
Pavel Stehule
Date:
forward patch to pg_hackers

There is fixed patch. Please, Jaime, can you look on it?

Thank You
Pavel

2009/7/30 Tom Lane <tgl@sss.pgh.pa.us>:
> Jaime Casanova <jcasanov@systemguards.com.ec> writes:
>>> On Mon, Jul 20, 2009 at 10:09 AM, Alvaro
>>>> Getting rid of the check on natts was "ungood" ... it needs to compare
>>>> the number of undropped columns of both tupdescs.
>
>> patch attached
>
> This patch is *still* introducing more bugs than it fixes.  The reason
> is that it has modified validate_tupdesc_compat to allow the tupdescs to
> be somewhat different, but has fixed only one of the several call sites
> to deal with the consequences of that.  The others will now become crash
> risks if we apply it as-is.
>
> What I would suggest as a suitable plan for a fix is to modify
> validate_tupdesc_compat so that it returns a flag indicating whether the
> tupdesc compatibility is exact or requires translation.  If translation
> is required, provide another routine that does that --- probably using a
> mapping data structure set up by validate_tupdesc_compat, since in some
> of these cases we'll be processing many tuples.  Then the callers just
> have to know enough to call the tuple-translation function when
> validate_tupdesc_compat tells them to.
>
> There are a number of other places in the system with similar
> requirements, although none of them seem to be exact matches.
> In particular ExecEvalConvertRowtype() provides a good template for
> efficient translation logic, but it's using column name matching
> rather than positional matching so you couldn't share the setup logic.
> I'm not sure if it's worth moving all this code into the core so that
> it can be shared with other future uses (maybe in other PLs), but it's
> worth considering that.  access/common/heaptuple.c or tupdesc.c might
> be a good place for it if we decide to do that.
>
> The executor's junkfilter code is pretty nearly related as well, and
> perhaps the Right Thing is to make all of this stuff into applications
> of junkfilters with different setup routines for the different
> requirements.  However the junkfilter code is designed to work with
> tuples that are in TupleTableSlots, which isn't particularly helpful for
> plpgsql's uses, so maybe trying to unify with that code is more trouble
> than it's worth.
>
> I'm marking this patch as Waiting on Author again, although perhaps
> Returned with Feedback would be better since my suggestions amount
> to wholesale rewrites.
>
>                        regards, tom lane
>

Attachment

Re: Fwd: [BUGS] fix: plpgsql: return query and dropped columns problem

From
Jaime Casanova
Date:
On Tue, Aug 4, 2009 at 4:30 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote:
> forward patch to pg_hackers
>
> There is fixed patch. Please, Jaime, can you look on it?
>

this one passed regression tests and my personal test script (of
course it passed the script the last time too)... i'm doing a lot of
alter table [add|drop] column...

it seems it's good enough and is implementing tom's suggestions... can
this be reviewed by a commiter?

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157


Re: Fwd: [BUGS] fix: plpgsql: return query and dropped columns problem

From
Tom Lane
Date:
Pavel Stehule <pavel.stehule@gmail.com> writes:
> There is fixed patch. Please, Jaime, can you look on it?

Applied with significant revisions.  I really wanted this code factored
out, because we'd otherwise end up duplicating it in other PLs (and it
was already duplicative of execQual.c).  So I pushed the support code
into a new file tupconvert.c.
        regards, tom lane


Re: Fwd: [BUGS] fix: plpgsql: return query and dropped columns problem

From
Pavel Stehule
Date:
2009/8/6 Tom Lane <tgl@sss.pgh.pa.us>:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
>> There is fixed patch. Please, Jaime, can you look on it?
>
> Applied with significant revisions.  I really wanted this code factored
> out, because we'd otherwise end up duplicating it in other PLs (and it
> was already duplicative of execQual.c).  So I pushed the support code
> into a new file tupconvert.c.
>
>                        regards, tom lane
>

thank you

Pavel