On Wed, Jan 11, 2023 at 4:13 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jan 11, 2023 at 10:48 AM Robert Treat <rob@xzilla.net> wrote:
> > > @Robert: I wonder why shouldn't CREATE..PARTITION OF *also* be patched
> > > to first create a table, and then attach the partition, transparently
> > > doing what everyone would want, without having to re-read the updated
> > > docs or know to issue two commands? I wrote a patch for this which
> > > "doesn't fail tests", but I still wonder if I'm missing something..
> > >
> >
> > I was thinking there might be either lock escalation issues or perhaps
> > issues around index attachment that don't surface using create
> > partition of, but I don't actually see any, in which case that does
> > seem like a better change all around. But like you, I feel I must be
> > overlooking something :-)
>
> To be honest, I'm not sure whether either of you are missing anything
> or not. I think a major reason why I didn't implement this was that
> it's a different code path. DefineRelation() has code to do a bunch of
> things that are also done by ATExecAttachPartition(), and I haven't
> gone through exhaustively and checked whether there are any relevant
> differences. I think that part of the reason that I did not research
> that at the time is that the patch was incredibly complicated to get
> working at all and I didn't want to take any risk of adding things to
> it that might create more problems. Now that it's been a few years, we
> might feel more confident.
>
> Another thing that probably deserves at least a bit of thought is the
> fact that ATTACH PARTITION just attaches a partition, whereas CREATE
> TABLE does a lot more things. Are any of those things potential
> hazards? Like what if the newly-created table references the parent
> via a foreign key, or uses the parent's row type as a column type or
> as part of a column default expression or in a CHECK constraint or
> something? Basically, try to think of weird scenarios where the new
> table would interact with the parent in some weird way where the
> weaker lock would be a problem. Maybe there's nothing to see here: not
> sure.
>
> Also, we need to separately analyze the cases where (1) the new
> partition is the default partition, (2) the new partition is not the
> default partition but a default partition exists, and (3) the new
> partition is not the default partition and no default partition
> exists.
>
> Sorry not to have more definite thoughts here. I know that when I
> developed the original patch, I thought about this case and decided my
> brain was full. However, I do not recall whether I knew about any
> specific problems that needed to be fixed, or just feared that there
> might be some.
>
I think all of that feedback is useful, I guess the immediate question
becomes if Justin wants to try to proceed with his patch implementing
the change, or if adjusting the documentation for the current
implementation is the right move for now.
Robert Treat
https://xzilla.net
