Re: Changeset Extraction v7.6.1

On Mon, Feb 24, 2014 at 9:48 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2014-02-15 17:29:04 -0500, Robert Haas wrote:
>> On Fri, Feb 14, 2014 at 4:55 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>
>> +       /*
>> +        * XXX: It's impolite to ignore our argument and keep decoding until the
>> +        * current position.
>> +        */
>>
>> Eh, what?
>
> So, the background here is that I was thinking of allowing to specify a
> limit for the number of returned rows. For the sql interface that sounds
> like a good idea. I am just not so sure anymore that allowing to specify
> a LSN as a limit is sufficient. Maybe simply allow to limit the number
> of changes and check everytime a transaction has been replayed?

The last idea there seems like pretty sound, but ...

> It's all trivial codewise, I am just wondering about the interface most
> users would want.

...I can't swear it meets this criterion.

>> +        * We misuse the original meaning of SnapshotData's xip and
>> subxip fields
>> +        * to make the more fitting for our needs.
>> [...]
>> +        * XXX: Do we want extra fields instead of misusing existing
>> ones instead?
>>
>> If we're going to do this, then it surely needs to be documented in
>> snapshot.h.  On the second question, you're not the first hacker to
>> want to abuse the meanings of the existing fields; SnapshotDirty
>> already does it.  It's tempting to think we need a more principled
>> approach to this, like what we've done with Node i.e. typedef enum ...
>> SnapshotType; and then a separate struct definition for each kind, all
>> beginning with SnapshotType type.
>
> Hm, essentially that's what the ->satisfies pointer already is, right?

Sorta, yeah.  But with nodes, you can change the whole struct
definition for each type.

> There's already documentation of the extra fields in snapbuild, but I
> understand you'd rather have them moved?

Yeah, I think it needs to be documented where SnapshotData is defined.There may be reason to mention it again, or in
moredetail,
 
elsewhere.  But there should be some mention of it there.

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