Hello Steve and slony1-hackers,
	Thanks in advance for your time and effort. I’m writing now to answer Steve’s comments so far. See inline with “ttignor”. Please feel free to follow up as needed.

On 1/16/18, 10:37 PM, "Steve Singer" <steve at ssinger.info> wrote:

    On Wed, 10 Jan 2018, Tignor, Tom wrote:
    
    Thank you for your submissions.
    
    I will try to look at the patches individually.
    
    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.
--------
    
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    (