Re: [BUGS] Breakage with VACUUM ANALYSE + partitions

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Thom Brown <thom(at)linux(dot)com>, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [BUGS] Breakage with VACUUM ANALYSE + partitions
Date: 2016-04-25 18:53:26
Message-ID: alpine.DEB.2.10.1604252026040.32250@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers


Hello Andres,

> Fixes it for me too.

Yep, for me too.

I'm not at ease with the part of the API the code is dealing with, so I do
not feel like a competent reviewer. I agree with the "more comments"
suggested by Robert.

I have just a small naming point:

/* ereport if segment not present, create in recovery */
EXTENSION_FAIL,
/* return NULL if not present, create in recovery */
EXTENSION_RETURN_NULL,
/* return NULL if not present */
EXTENSION_REALLY_RETURN_NULL,
/* create new segments as needed */
EXTENSION_CREATE

The comments seem pretty clear, but the naming of these options are more
behavioral than functional somehow (or the reverse?), especially the
RETURN_NULL and REALLY_RETURN_NULL names seemed pretty contrived to me.

There is one context: whether it is in recovery. There are 3 possible
behaviors: whether to error or ignore or create if segment does not
exist.

In recovery it is always create if asked for it must be made available,
maybe it does not exists because of the crash...

If I understand correctly, with flushes kept a long time before being
processed, there is a new state which is "ignore whatever", we do not want
to create a segment for flushing non existing data.

So I would suggest it would be better to name the options closer to the
comments above, something like:

normal / in recovery:
EXTENSION_ERROR_OR_IR_CREATE
EXTENSION_IGNORE_OR_IR_CREATE
EXTENSION_IGNORE
EXTENSION_CREATE

Now, maybe that is too late to try to find a better name for these
options now:-)

--
Fabien.

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Yaroslav 2016-04-25 18:54:53 Re: BUG #14107: Major query planner bug regarding subqueries and indices
Previous Message Andres Freund 2016-04-25 18:05:21 Re: [BUGS] Breakage with VACUUM ANALYSE + partitions

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2016-04-25 18:58:04 Re: Rename max_parallel_degree?
Previous Message Andres Freund 2016-04-25 18:52:04 Re: xlc atomics