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
