Christopher Browne cbbrowne at ca.afilias.info
Mon Nov 26 08:30:20 PST 2007
Jacques Caron <jc at oxado.com> writes:
> Hi,
>
> The following very quick patch (against 1.2.12) provides the following changes:
> - when scanning attributes looking for key columns (in UPDATEs and
> DELETEs), stop when we reach the end of the attkind string
> - when creating the triggers, final "v"s are stripped
>
> Result:
> - one can add columns (without defaults, constraints, etc.) to tables
> (destinations first, origin last) without changing the logtrigger and
> without using EXECUTE SCRIPT
> - as a bonus we save a few cycles in updates and deletes
>
> Comments welcome!
>
> Jacque.
>
> --- src/backend/slony1_funcs.c.orig Wed Nov 21 15:43:24 2007
> +++ src/backend/slony1_funcs.c      Wed Nov 21 15:45:13 2007
> @@ -821,6 +821,8 @@
>                                 continue;
>
>                         attkind_idx++;
> +                       if (!attkind[attkind_idx])
> +                               break;
>                         if (attkind[attkind_idx] != 'k')
>                                 continue;
>                         col_ident = (char
> *)slon_quote_identifier(SPI_fname(tupdesc, i + 1));
> @@ -888,6 +890,8 @@
>                                 continue;
>
>                         attkind_idx++;
> +                       if (!attkind[attkind_idx])
> +                               break;
>                         if (attkind[attkind_idx] != 'k')
>                                 continue;
>                         col_ident = (char
> *)slon_quote_identifier(SPI_fname(tupdesc, i + 1));
> --- src/backend/slony1_funcs.sql.orig       Wed Nov 21 15:45:35 2007
> +++ src/backend/slony1_funcs.sql    Wed Nov 21 15:53:11 2007
> @@ -5029,6 +5029,11 @@
>         end loop;
>
>         --
> +       -- Strip final 'v's as these are not needed by the logtrigger
> +       --
> +       v_attkind := regexp_replace(v_attkind,''v*$'','''');
> +
> +       --
>         -- Return the resulting attkind
>         --
>         return v_attkind;

I would be super-disinclined to put this into the 1.2 branch, because:

a) 1.2 is the "stable" branch, and adding new functionality isn't
   proper for that, anymore

b) Once the next version gets out, I want to be able to at least *think*
   about deprecating 1.2, as opposed to having new features in 1.2 that
   will continue to need support

Could you redirect this to CVS HEAD?  I'd be much more comfortable
discussing the change in that context.

I may still have some objections then, but I'd be happier discussing
it in the context of future releases...

As with the other discussion, I think it would be really useful to
have, already framed, some sort of test scenario where we know this
will fix problems that the "old way" was known to handle wrong.

Having that makes it a lot more clear whether:
  a) This is actually an improvement or not, and
  b) The problem is one that appears worth fixing.

At first glance, neither a) nor b) are presently *obviously* the case.
-- 
(format nil "~S@~S" "cbbrowne" "linuxdatabases.info")
http://linuxfinances.info/info/unix.html
"In man-machine symbiosis, it is man who must adjust: The machines
can't." -- Alan J. Perlis


More information about the Slony1-general mailing list