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

Search Discussions

  • Jaime Casanova at Aug 6, 2009 at 6:15 am

    On Tue, Aug 4, 2009 at 4:30 AM, Pavel Stehulewrote:
    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
  • Tom Lane at Aug 6, 2009 at 8:47 pm

    Pavel Stehule 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
  • Pavel Stehule at Aug 7, 2009 at 4:26 am

    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedAug 4, '09 at 9:30a
activeAug 7, '09 at 4:26a
posts4
users3
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase