Andres Freund <andres@2ndquadrant.com> writes:
> On 2012-12-20 22:50:54 +0100, Andres Freund wrote:
>> Ok, here are two patches:
>> * Add a cooked_expr member to IndexElem and use it for transformed
>> expressions, including filling it directly in generateClonedIndexStmt.
>>
>> * Follow the pattern set by other routines in parse_expr.c and don't
>> transformRowExpr the same expression twice.
>>
>> While the first one fixes the above bug - and I think its the right
>> approach not to analyze the expression twice, the second one seems like
>> a good idea anyway because as transformExpr says:
>> * 1. At least one construct (BETWEEN/AND) puts the same nodes
>> * into two branches of the parse tree; hence, some nodes
>> * are transformed twice.
>> * 2. Another way it can happen is that coercion of an operator or
>> * function argument to the required type (via coerce_type())
>> * can apply transformExpr to an already-transformed subexpression.
>> * An example here is "SELECT count(*) + 1.0 FROM table".
>>
>> There unfortunately is not sufficient padding in IndexElem to do that
>> without changing its size. Not sure whether we consider that to be a big
>> problem for the back branches, its nothing user code should do, but
>> ...
> So nobody has an idea that would avoid changing the sizeof(IndexElem)?
Yeah: forget the first patch and just do the second. There are already
sufficient reasons why transformExpr has to be idempotent; this is just
another one. I don't really see a need to kluge up IndexElem for this.
We might at some point try to clean all this up. But in the meantime
I see no good reason to make LIKE INCLUDING INDEXES adhere to a higher
standard than the rest of the code does, and even less reason to
back-patch such a change.
BTW, it sure looks to me like transformXmlExpr will get an Assert
failure on an already-transformed expression ...
regards, tom lane