On Tue, Aug 09, 2022 at 10:35:01AM -0400, Tom Lane wrote: > Agreed this is a bug, but I do not think we should add the proposed > regression test, regardless of where exactly. It looks like expending > a lot of cycles forevermore to watch for an extremely unlikely thing, > ie that we break this for ALTER MATERIALIZED VIEW and not anything > else.
Hmm. I see a second point in keeping a test in this area, because we have nothing that directly checks after AT_REWRITE_ACCESS_METHOD as introduced by b048326. It makes me wonder whether we should have a second test for a plain table with SET ACCESS METHOD, actually, but we have already cases for rewrites there, so.. > I think the real problem here is that we don't have any mechanism > for verifying that table_rewrite_ok is correct. The "cross-check" > in EventTriggerCommonSetup is utterly worthless, as this failure > shows. Does it give any confidence at all that there are no other > mislabelings? I sure have none now. What can we do to verify that > more rigorously? Or maybe we should find a way to get rid of the > table_rewrite_ok flag altogether? This comes down to the dependency between the event trigger paths in utility.c and tablecmds.c, which gets rather trickier with the ALTERs on various relkinds. I don't really know about if we could cut this specific flag, perhaps we could manage a list of command tags supported for it as that's rather short. I can also see that something could be done for the firing matrix in the docs, as well (the proposed patch has forgotten the docs). That's not something that should be done for v15 anyway, so I have fixed the issue at hand to take care of this open item. -- Michael
signature.asc
Description: PGP signature