Re: Rearranging ALTER TABLE to avoid multi-operations bugs

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> From my point of view, the DDL code doesn't do a great job separating
> parsing/parse analysis from optimization/execution. The ALTER TABLE
> stuff is actually pretty good in this regard.

Meh. I think a pretty fair characterization of the bug(s) I'm trying to
fix is "we separated parse analysis from execution when we should not
have, because it leads to parse analysis being done against the wrong
database state". So I'm *very* suspicious of any argument that we should
try to separate them more, let alone that doing so will somehow fix this
set of bugs.

>> Also, recursive ProcessUtility cases exist independently of this issue,
>> in particular in CreateSchemaCommand. My worry about my patch upthread
>> is not really that it introduces another one, but that it doesn't do
>> anything towards providing a uniform framework/notation for all these
>> cases.

> I'm not really sure I understand this.

Well, I tried to wrap what are currently a random set of ProcessUtility
arguments into one struct to reduce the notational burden. But as things
are set up, that's specific to the ALTER TABLE case. I'm feeling like it
should not be, but I'm not very sure where to draw the line between
arguments that should be folded into the struct and ones that shouldn't.

Note that I think there are live bugs in here that are directly traceable
to not having tried to fold those arguments before. Of the four existing
recursive ProcessUtility calls with context = PROCESS_UTILITY_SUBCOMMAND,
two pass down the outer call's "ParamListInfo params", and two don't ---
how is it not a bug that they don't all behave alike? And none of the
four pass down the outer call's QueryEnvironment, which seems like even
more of a bug. So it feels like we ought to have a uniform approach
to what gets passed down during recursion, and enforce it by passing
all such values in a struct rather than as independent arguments.

regards, tom lane