Re: Identifying no-op length coercions

Noah Misch <noah(at)leadboat(dot)com> writes:
> We originally discussed having the transform function take and return Expr
> nodes. It turns out that simplify_function() does not have the Expr, probably
> because the particular choice of node type among the many that can convey a
> function call does not matter to it. The same should be true of a transform
> function -- it should do the same thing whether the subject call arrives from
> a FuncExpr or an OpExpr. Therefore, I changed the transform function
> signature to "Expr *protransform(List *args)".

That seems like the wrong thing. For one thing, it makes it quite
impossible to share one transform support function among multiple
functions/operators, since it won't know which one the argument list
is for. More generally, I foresee the transform needing to know
most of the other information that might be in the FuncExpr or OpExpr.
It certainly would need access to the collation, and it would probably
like to be able to copy the parse location to whatever it outputs,
and so forth and so on. So I really think passing the node to the
function is the most future-proof way to do it. If that means you
need to rejigger things a bit in clauses.c, so be it.

> The large pg_proc.h diff mostly just adds protransform = 0 to every function.
> Due to the resulting patch size, I've compressed it. There are new/otherwise
> changed DATA lines for OIDs 669 and 3097 only.

The chances that this part will still apply cleanly by the time anyone
gets around to committing it are nil. I'd suggest you break it into two
separate patches, one that modifies the existing lines to add the
protransform = 0 column and then one to apply the remaining deltas in
the file. I envision the eventual committer doing the first step
on-the-fly (perhaps with an emacs macro, at least that's the way I
usually do it) and then wanting a patchable diff for the rest. Or if
you wanted to be really helpful, you could provide a throwaway perl
script to do the modifications of the existing lines ...

I haven't read the actual patch, these are just some quick reactions
to your description.

regards, tom lane