Thread: Add PGDLLIMPORT lines to some variables

Hi,

Please quote properly on postgres mailing lists... We're old school [tm].

On 2017-11-20 11:58:44 -0800, Brian Cloutier wrote:
> > please, append session_timezone to your list
> 
> Here's a v2 patch which also includes session_timezone.
> 
> Separately, is this the kind of thing which is eligible for backporting
> into the next releases of 9.6 and 10?

I don't think we quite have an established protocol for this. I
personally, but I'm biased in this specific case, is that we should
adopt a position that PGDLLIMPORTs should basically backpatched whenever
a credible extension even halfway reasonably requires it. There's no
easy way to get this done by default, and we're so far unwilling to just
slap this onto every variable. So to not further disadvantage people
force dto live in the MS environment, that seems the sanest
solution. It's not like these are high risk.

- Andres

On Wed, Nov 29, 2017 at 5:00 AM, Brian Cloutier <brian@citusdata.com> wrote:
> Sorry, I'm new to pg-hackers, so I'm not sure what the next step is.
>
> Do I submit this to commitfest?
>
> When submitting, do I submit multiple changes, one per branch this should be
> packported to?

If you want a patch to get reviewed, please register it to the commit
fest? Here are more details:
https://wiki.postgresql.org/wiki/Submitting_a_Patch
Sending as a first step only one patch for HEAD is fine, as discussion
will likely occur while you work on a patch. If the patch is worth
back-patching, usually the committer who picks up the patch would
generate the versions for the back-branches. Help is always
appreciated with version-specific patches once it is clearly decided
that a certain patch merits a backpatch. Note that PGDLLIMPORT does
not have this treatment usually.
-- 
Michael

On Mon, Nov 20, 2017 at 12:02:30PM -0800, Andres Freund wrote:
> On 2017-11-20 11:58:44 -0800, Brian Cloutier wrote:
> > > please, append session_timezone to your list
> > 
> > Here's a v2 patch which also includes session_timezone.
> > 
> > Separately, is this the kind of thing which is eligible for backporting
> > into the next releases of 9.6 and 10?
> 
> I don't think we quite have an established protocol for this. I
> personally, but I'm biased in this specific case, is that we should
> adopt a position that PGDLLIMPORTs should basically backpatched whenever
> a credible extension even halfway reasonably requires it. There's no
> easy way to get this done by default, and we're so far unwilling to just
> slap this onto every variable. So to not further disadvantage people
> force dto live in the MS environment, that seems the sanest
> solution. It's not like these are high risk.

+1

.
On Mon, Dec 4, 2017 at 5:41 PM, Noah Misch <noah@leadboat.com> wrote:
>> I don't think we quite have an established protocol for this. I
>> personally, but I'm biased in this specific case, is that we should
>> adopt a position that PGDLLIMPORTs should basically backpatched whenever
>> a credible extension even halfway reasonably requires it. There's no
>> easy way to get this done by default, and we're so far unwilling to just
>> slap this onto every variable. So to not further disadvantage people
>> force dto live in the MS environment, that seems the sanest
>> solution. It's not like these are high risk.
>
> +1

If that's going to be the policy, then I have some requests of my own.
I would like to add some PGDLLIMPORTs to suit the external version of
amcheck (the version that targets earlier versions of Postgres). These
are:

RecentGlobalXmin -- This is only PGDLLIMPORT on Postgres 10+,
following commit 56018bf2. I'd like to get that back to 9.4, although
there is no reason to not include 9.3.

TransactionXmin -- This is needed for the newer heap-matches-index
verification check. Again, I would like this on 9.4+, but 9.3+ works
too.

Note that somebody asked about running it on Windows recently, and on
one other occasion in the past. It does come up.

-- 
Peter Geoghegan

On Mon, Dec 4, 2017 at 8:48 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> On Mon, Dec 4, 2017 at 5:41 PM, Noah Misch <noah@leadboat.com> wrote:
>>> I don't think we quite have an established protocol for this. I
>>> personally, but I'm biased in this specific case, is that we should
>>> adopt a position that PGDLLIMPORTs should basically backpatched whenever
>>> a credible extension even halfway reasonably requires it. There's no
>>> easy way to get this done by default, and we're so far unwilling to just
>>> slap this onto every variable. So to not further disadvantage people
>>> force dto live in the MS environment, that seems the sanest
>>> solution. It's not like these are high risk.
>>
>> +1
>
> If that's going to be the policy, then I have some requests of my own.
> I would like to add some PGDLLIMPORTs to suit the external version of
> amcheck (the version that targets earlier versions of Postgres). These
> are:
>
> RecentGlobalXmin -- This is only PGDLLIMPORT on Postgres 10+,
> following commit 56018bf2. I'd like to get that back to 9.4, although
> there is no reason to not include 9.3.
>
> TransactionXmin -- This is needed for the newer heap-matches-index
> verification check. Again, I would like this on 9.4+, but 9.3+ works
> too.
>
> Note that somebody asked about running it on Windows recently, and on
> one other occasion in the past. It does come up.

Committed with these additions.  Please check that I haven't messed anything up.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Dec 5, 2017 at 6:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> RecentGlobalXmin -- This is only PGDLLIMPORT on Postgres 10+,
>> following commit 56018bf2. I'd like to get that back to 9.4, although
>> there is no reason to not include 9.3.
>>
>> TransactionXmin -- This is needed for the newer heap-matches-index
>> verification check. Again, I would like this on 9.4+, but 9.3+ works
>> too.
>>
>> Note that somebody asked about running it on Windows recently, and on
>> one other occasion in the past. It does come up.
>
> Committed with these additions.  Please check that I haven't messed anything up.

Thanks, but you modified RecentGlobalDataXmin, not RecentGlobalXmin.
Can you fix it, please?

-- 
Peter Geoghegan

On Tue, Dec 5, 2017 at 9:09 AM, Peter Geoghegan <pg@bowt.ie> wrote:
>> Committed with these additions.  Please check that I haven't messed anything up.
>
> Thanks, but you modified RecentGlobalDataXmin, not RecentGlobalXmin.
> Can you fix it, please?

I was looking at the wrong branch. Forget it.

Thanks
-- 
Peter Geoghegan

[ blast-from-the-past department ]

Craig Ringer <craig@2ndquadrant.com> writes:
> On 5 December 2017 at 22:49, Robert Haas <robertmhaas@gmail.com> wrote:
>>     Mark assorted variables PGDLLIMPORT.

> I was going to pipe up here to add ReplicationSlotCtl to the list.
> Otherwise the only way to access slot information is via the SPI and
> pg_stat_replication_slots, which isn't super fun. And it's not like
> ReplicationSlotCtl is any more internal than MyReplicationSlot.

> I missed the boat on your commit, but ... please?

Not sure why this request was ignored, but now that we have another
request for the same thing [1], I've pushed this.

            regards, tom lane

[1] https://postgr.es/m/345138875.20190611151943@cybertec.at