Hello Andres,
I'm not sure this is the best way to go about this. I can see valid
arguments for *always* using _mdfd_openseg() in mdsync(); and I'm
wondering whether we shouldn't make EXTENSION_* into a bitmask
(extend,extend_recovery,return_null,open_deleted).
I thought about that when I looked at the previous fix, but it seemed that
not all combinations made sense.
Sure, but that's nothing unusual. Here's an attempt at doing so - not
fully polished, just as a discussion point. I think it looks better.
Fabien, Robert, what do you think?
My 0,02€.
Not tested, just a few comments on the patch from someone which does not
understand this API deep down... Nevertheless:
I agree that it is looks better than "EXTENSION_REALLY_RETURNS_NULL", that
I did not like much.
There are 3 possible behaviors on extension, but coding them as bits does
not make their exclusivity clear. Now mixing numbers & bits does not seem
advisable either.
Maybe consider checking for the exclusivity explicitely?
EXTENSION_BEHAVIORS = (EXTENSION_RETURN_NULL | ..._FAIL | ..._CREATE);
And then the Assert can check for the exclusivity:
int behavior = option & EXTENSION_BEHAVIORS;
Assert( (behavior == EXTENSION_RETURN_NULL) ||
(behavior == ..._FAIL) ||
(behavior == ..._CREATE));
I'm unsure about switching enum to #define, could be an enum still with
explicit values set, something like:
enum {
EXTENSION_RETURN_NULL = (1 << 0),
...
} extension_behavior;
I'm fuzzy about the _OPEN_DELETED part because it is an oxymoron. Is it
RECREATE really?
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
