On 2024-Mar-08, Michael Paquier wrote:
> I have spent more time reviewing the whole and the tests (I didn't see
> much value in testing the DEFAULT clause twice for the partitioned
> table case and there is a test in d61a6cad6418), tweaked a few
> comments and the documentation, did an indentation and a commit
> message draft.
>
> How does that look to you? The test coverage and the semantics do
> what we want them to do, so that looks rather reasonable here. A
> second or even third pair of eyes would not hurt.
I gave this a look. I found some of the comments a bit confusing or
overly long, so I propose to reword them. I also propose a small doc
change (during writing which I noticed that the docs for tablespace had
been neglected and one comment too many; patch to be committed
separately soon). I ended up also moving code in tablecmds.c so that
all the AT*SetAccessMethod* routines appear together rather than mixed
with the ones for tablespaces, and removing one CCI that seems
unnecessary, at the bottom of ATExecSetAccessMethodNoStorage.
0001 is Michaël's patch, 0002 are my proposed changes.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Hay que recordar que la existencia en el cosmos, y particularmente la
elaboración de civilizaciones dentro de él no son, por desgracia,
nada idílicas" (Ijon Tichy)