On Tue, Jul 2, 2019 at 2:00 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > movead li <movead...@highgo.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
