public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Partitioned tables and [un]loggedness
Date: Thu, 29 Aug 2024 15:44:45 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <Zs4-xnNDMEdiKQXo@nathan>
References: <[email protected]>
<20240424202640.GB861150@nathanxps13>
<CAKFQuwY3RTTrp7x4OmEruMmOghdorhq4=eC0ZfG0NM+HnBWFsw@mail.gmail.com>
<[email protected]>
<CAKFQuwZwMOkw-7112u1-gXEhRWuXiZe9otp801Z3dPVvarKdsA@mail.gmail.com>
<[email protected]>
<[email protected]>
<Zs4-xnNDMEdiKQXo@nathan>
On Tue, Aug 27, 2024 at 04:01:58PM -0500, Nathan Bossart wrote:
> 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.
Okay. Thanks for sharing an opinion.
> * 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.
The reason for temporary tables is different though: we expect
everything to be gone once the backend that created these relations is
gone. If persistence cocktails were allowed, the worse thing that
could happen would be to have a partitioned table that had temporary
partitions; its catalog state can easily get broken depending on the
DDLs issued on it. Valid partitioned index that should not be once
the partitions are gone, for example, which would require more exit
logic to flip states in pg_class, pg_index, etc.
> 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.
The addition of the new LOGGED keyword is not required if we limit
ourselves to an error when defining UNLOGGED, so if we drop this
proposal, let's also drop this part entirely and keep DefineRelation()
simpler. Actually, is really issuing an error the best thing we can
do after so many years allowing this grammar flavor to go through,
even if it is perhaps accidental? relpersistence is marked correctly
for partitioned tables, it's just useless. Expanding the
documentation sounds fine to me, one way or the other, to tell what
happens with partitioned tables.
By the way, I was looking at this patch series, and still got annoyed
with the code duplication with ALTER TABLE SET LOGGED/UNLOGGED, so
I've done something about that for now.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Partitioned tables and [un]loggedness
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox