Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> That seems ok to me. Calling eval_const_expressions() in ALTER COLUMN
> and elsewhere is a good idea for performance reasons as well.
Yeah, probably so.
> I can only find one more call to fix_opfuncids, where we're not already
> calling eval_const_expressions(): GetDomainConstraints(). Adding a
> eval_const_expressions() call to ExecPrepareExpr() would take care of
> the ALTER COLUMN and many other cases where we have a problem now.
I'd prefer not to have ExecPrepareExpr do it, though; that's supposed
to be working from a read-only expression tree supplied by the caller.
(The fix_opfuncids call in it is already pushing the bounds of that
concept.)
From a structural point of view the right thing would be to introduce
a concept of "expression planning", along the lines of
expr = plan_expression(expr);
which callers would be required to invoke before ExecPrepareExpr.
Right now this would do the fix_opfuncids bit and
eval_const_expressions, but I could see someday allowing SubLinks
in standalone expressions because we'd have the ability to invoke
the full planner from inside here.
The trick is to get the attention of third-party code about the need
to make this change. Removing fix_opfuncids from ExecPrepareExpr
wouldn't really help much, because in very many common cases it's
a no-op anyway; so unless their testing is quite thorough they would
not see a failure before shipping.
The only idea I have at the moment is to rename ExecPrepareExpr to
something else, but it's not clear if that will persuade people to read
its header comment or not ...
regards, tom lane
