On Sat, Jul 30, 2022 at 5:25 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvhe...@alvh.no-ip.org> writes:
> > Yeah, I don't know about adding tons of values to that enum just so that
> > we can use that to hide a boolean inside. Why not add a boolean to the
> > containing struct? Something like the attached.
>
> I do not think it's a great idea to have ALTER TABLE scribbling on
> the source parsetree.
Hmm, I think we already do scribble on the source parse tree even
before this patch, for example, as ATPrepCmd() does for DROP
CONSTRAINT:
if (recurse)
cmd->subtype = AT_DropConstraintRecurse;
> That tree could be in plancache and subject
> to reuse later.
I see that 7c337b6b527b added 'readOnlyTree' to
standard_ProcessUtility()'s API, I guess, to make any changes that
AlterTable() and underlings make to the input AlterTableStmt be local
to a given execution. Though, maybe that's not really a permission to
add more code that makes such changes?
In this case of needing to remember the inh/recurse flag mentioned in
the original AT command, we could avoid scribbling over the input
AlterTableStmt by setting a new flag in AlteredTableInfo, instead of
AlterTableCmd. AlteredTableInfo has other runtime info about the
relation being altered and perhaps it wouldn't be too bad if it also
stores the inh/recurse flag.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
