On Tue, 23 Jan 2018, Tignor, Tom wrote:

>    compatibility_v2.2.patch
>
>    My understanding of this patch is so that you slon for any 2.2.x version to
>    be able to connect and replicate against a backend database with any other
>    2.2.y version (where x and y can be, but are not required to be equal).
>
> ttignor – Yes, that’s the intention and I believe the exact effect. 
> Specifically, we’re upgrading our slony1-dependent services from 
> slony1-2.2.4 to slony1-2.2.6. The upgrade process we’ve verified requires 
> a mixed mode environment for a limited period of time (a few days). I 
> chose “2.2.x” arbitrarily. We could achieve what we need with more 
> specific criteria, or a config option, or a combination of the two. 
> --------

How do you feel about instead having an option to disable the slony version 
check.

My concern with saying all 2.2.x versions are compatible or even that 
certain minor/patch versions are compatible is the cross version testing 
load it puts on slony maintainers and how it might restrict things in the 
future.

If instead we just provide a option to disable the version check users can 
use this option as they see fit.



>
> A few things on the specific patch
>
>    +++ slony1-2.2.6/src/slon/dbutils.c     2017-12-22 08:08:33.027322631 -0500
>    @@ -5,6 +5,7 @@
>       *
>       *     Copyright (c) 2003-2009, PostgreSQL Global Development Group
>       *     Author: Jan Wieck, Afilias USA INC.
>    + *      Copyright (C) 2017 - Akamai Technologies, Inc
>       *
>       *
>       * ----------------------------------------------------------------------
>    @@ -418,7 +419,8 @@
>
>    We don't normally list contributors in the copyright section. The copyright
>    to PostgreSQL global development group is intended to cover all
>    contributors.
>
> ttignor – I’ve gone a few rounds on this with our opensource group. As I understand the issues, there is a choice to either maintain or assign away the Akamai copyright for the code I wrote.  Per our previous discussion, we’re maintaining the Akamai copyright. This is accomplished by header comments and/or some meta-data in the patch. If the header comment is a problem, then the patch meta-data becomes essential. If slony1-hackers can agree on exactly what copyright changes are needed, I can take them back to Akamai opensource.
> --------
>
> So for the pg_home patch.
> …
>  #else
> +       char *pgHome = getenv("PG_HOME");
> +       if (pgHome) {
> +         strncpy(share_path, pgHome, MAXPGPATH-1);
> +         share_path[MAXPGPATH-1] = '\0';
> +         strncat(share_path, "/share", MAXPGPATH-1-strlen(pgHome));
> +       } else {
>         strcpy(share_path, PGSHARE);
> +       }
>
>
> The above code only gets compiled in if PGPORT is not defined/present at
> build time.  Is that your intention,  or do you want PG_HOME to take
> precendence even if pgport is present?
>
> snprintf(share_path,"%s/share",pgHome,MAXPGPATH-1);
>
> Would the above code be clearer? (I haven't tested/tried to compile above)
> but trying to do this in one line?
>
> ttignor – Reviewing the changes, the HAVE_PGPORT clause is marked with a comment “We need to find a share directory like PostgreSQL.”. That seemed like something I shouldn’t override with a customization. Re: snprintf, that seems like a good improvement. I see snprintf can also truncate output, so I’ll need to account for that as well with a rewrite.
> --------
>
> 	More thoughts from Steve or others? Keep them coming.
>
> 	Tom    (
>
>
>
>
>
>
>