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.