Re: [BUGS] Breakage with VACUUM ANALYSE + partitions

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.