Re: Rearranging ALTER TABLE to avoid multi-operations bugs

On Tue, Jul 2, 2019 at 2:00 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> movead li <movead(dot)li(at)highgo(dot)ca> writes:
> > I applied the 'alter-table-with-recursive-process-utility-calls-wip.patch'
> > on the master(e788e849addd56007a0e75f3b5514f294a0f3bca). And
> > when I test the cases, I find it works well on 'alter table t1 add column
> > f2 int not null, alter column f2 add generated always as identity' case,
> > but it doesn't work on #14827, #15180, #15670, #15710.
>
> This review seems not very on-point, because I made no claim to have fixed
> any of those bugs. The issue at the moment is how to structure the code
> to allow ALTER TABLE to call other utility statements --- or, if we aren't
> going to do that as Robert seems not to want to, what exactly we're going
> to do instead.
>
> The patch at hand does fix some ALTER TABLE ... IDENTITY bugs, because
> fixing those doesn't require any conditional execution of utility
> statements. But we'll need infrastructure for such conditional execution
> to fix the original bugs. I don't see much point in working on that part
> until we have some agreement about how to handle what this patch is
> already doing.

With my CF manager hat: I've moved this to the next CF so we can
close this one soon, but since it's really a bug report it might be
good to get more eyeballs on the problem sooner than September.

With my hacker hat: Hmm. I haven't looked at the patch, but not
passing down the QueryEnvironment when recursing is probably my fault,
and folding all such things into a new mechanism that would avoid such
bugs in the future sounds like a reasonable approach, if potentially
complicated to back-patch. I'm hoping to come back and look at this
properly in a while.

--
Thomas Munro
https://enterprisedb.com