Re: Moving snapshot code around

Neil Conway <neilc(at)samurai(dot)com> writes:
> On Tue, 2008-03-18 at 16:19 -0300, Alvaro Herrera wrote:
>> The other approach, of course, is to just keep all the code in tqual.c
>> and not create a separate module at all. Opinions? I prefer to keep
>> them separate, but I'm not wedded to it if there's any strong reason not
>> to do it. Also, the line is currently blurred because some users of
>> snapshots mess with the internals directly (setting snapshot->curcid),
>> but we could clean that up and make it so that those become external
>> interface users too.

> Sounds like a good idea to me -- +1 on keeping the code in two separate
> files, and moving snapshot users toward using the external interface.

I think thinking of snapshot.h as an "external" interface is
wrongheaded. In the proposed refactoring, snapshot.h is concerned with
snapshot *management* (creating, copying, deleting) while tqual.h is
concerned with tuple visibility testing (which requires a snapshot as an
input, but doesn't do any "management"). They're really entirely
orthogonal concerns. Some callers will need one, some will need the
other, a few might need both. But you could very much argue that
tqual.c depends on snapshot.c not the other way around, which makes
tqual the "external" party if you ask me.

With the exception of the outputs in SnapshotDirty testing, tqual.h's
operations would be read-only as far as snapshots are concerned. Not
sure if the read-only vs read-write distinction is helpful here,
but that's pretty much how it seems to be breaking down.

I don't have any particular objection to the factoring as proposed,
only to the way it's described. I am wondering a bit if the Snapshot
struct should be defined in a third file that's included by both of
these, rather than making either .h file depend on the other.

regards, tom lane