Re: Partitioned tables and [un]loggedness

On Thu, May 02, 2024 at 03:06:51PM +0900, Michael Paquier wrote:
> The case of a temporary persistence is actually *very* tricky. The
> namespace, where the relation is created, is guessed and locked with
> permission checks done based on the RangeVar when the CreateStmt is
> transformed, which is before we try to look at its inheritance tree to
> find its partitioned parent. So we would somewhat need to shortcut
> the existing RangeVar lookup and include the parents in the loop to
> find out the correct namespace. And this is much earlier than now.
> The code complexity is not trivial, so I am getting cold feet when
> trying to support this case in a robust fashion. For now, I have
> discarded this case and focused on the main problem with SET LOGGED
> and UNLOGGED.
>
> Switching between logged <-> unlogged does not have such
> complications, because the namespace where the relation is created is
> going to be the same. So we won't lock or perform permission checks
> on an incorrect namespace.

I've been thinking about this thread some more, and I'm finding myself -0.5
for adding relpersistence inheritance for UNLOGGED. There are a few
reasons:

* Existing partitioned tables may be marked UNLOGGED, and after upgrade,
new partitions would be UNLOGGED unless the user discovers that they need
to begin specifying LOGGED or change the persistence of the partitioned
table. I've seen many problems with UNLOGGED over the years, so I am
wary about anything that might increase the probability of someone using
it accidentally.

* I don't think partitions inheriting persistence is necessarily intuitive.
IIUC there's nothing stopping you from having a mix of LOGGED and
UNLOGGED partitions, so it's not clear to me why we should assume that
users want them to be the same by default. IMHO UNLOGGED is dangerous
enough that we really want users to unambiguously indicate that's what
they want.

* Inheriting certain persistences (e.g., UNLOGGED) and not others (e.g.,
TEMPORARY) seems confusing. Furthermore, if a partitioned table is
marked TEMPORARY, its partitions must also be marked TEMPORARY. There is
no such restriction when a partitioned table is marked UNLOGGED.

My current thinking is that it would be better to disallow marking
partitioned tables as LOGGED/UNLOGGED and continue to have users explicitly
specify what they want for each partition. It'd still probably be good to
expand the documentation, but a clear ERROR when trying to set a
partitioned table as UNLOGGED would hopefully clue folks in.

--
nathan