On 28.11.2010 23:44, Kevin Grittner wrote:
> (1) Should these be moved to ShmemVariableCache, or is it OK to
> leave them in this structure as long as I comment it adequately?
If they're only used by predicate.c, it seems better to leave them where
they are.
> (2) Would it be a good idea to create macros for referencing these
> fields? The current references are long and distracting to read, and
> would all need to change if the fields are moved to a different
> shared memory structure.
>
> The relevant commit diff is here:
>
>
http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=bc3aba45e192afcd7776c68e28438991a3406db6
Maybe invent a short name for PredLockTranList and/or the fields? I'm
not in favor of wrapping those references with macros, it can make it
harder to understand the code, and unless the macro name is much
shorter, it doesn't save all that much screen space anyway.
> There's another issue involving clarity of code, versus correct
> behavior. When I got to the last few false positives, I found that
> they could only be eliminated by tracking one more datum for a
> transaction committed with a conflict out to another transaction.
> There was a field which was unused in the particular situations where
> we needed this new datum, so I made it work with the dual use of the
> existing field.>
> (3) What would be the cleanest way to conditionally store values
> representing different things in one field of a structure, so that
> someone reading the code understands what's happening?
>
> The commit diff for this is here:
>
>
http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=9425d7fa551a1433bdbec1de5cfb1bfa9f43da22
I'd suggest a union field. We've done that e.g. in HeapTupleFields in
access/htup.h.
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com