>> --- a/contrib/pgstattuple/pgstatapprox.c
>> +++ b/contrib/pgstattuple/pgstatapprox.c
>> @@ -70,7 +70,7 @@ statapprox_heap(Relation rel, output_type *stat)
>> TransactionId OldestXmin;
>> uint64 misc_count = 0;
>>
>> - OldestXmin = GetOldestXmin(rel, true);
>> + OldestXmin = GetOldestXmin(rel, true, false);
>> bstrategy = GetAccessStrategy(BAS_BULKREAD);
>>
>> nblocks = RelationGetNumberOfBlocks(rel);
>
> This does not seem correct, you are sending false as pointer parameter.
Thanks. That's an oversight from the GetOldestXmin interface change
per your prior feedback. C doesn't care since null is 0 and false is
0, and I missed it when transforming the patch.
> 0012:
>
> I think there should be parameter saying if snapshot should be exported
> or not and if user asks for it on standby it should fail.
Sounds reasonable. That also means clients can suppress standby export
on master, which as we recently learned can be desirable sometimes.
> 0014 makes 0011 even more pointless.
Yeah, as I said, it's a bit WIP still and needs some rebasing and rearrangement.
> This also replaces the previous timeline following and decoding
> threads/CF entries so maybe those should be closed in CF?
I wasn't sure what to do about that, since it's all a set of related
functionality. I think it's going to get more traction as a single
"logical decoding onstandby" feature though, since the other parts are
hard to test and use in isolation. So yeah, probably, I'll do so
unless someone objects.
-- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services