Andres Freund <andres@anarazel.de> writes:
> On 2017-03-24 11:26:27 -0400, Tom Lane wrote:
>> Another modest proposal:
>>
>> I'm not really sold on the approach of using EEOP_FETCHSOME opcodes to
>> trigger initial tupleslot de-forming. Certainly we want to have a single
>> slot_getsomeattrs call per source slot, but as-is, we need a separate
>> traversal over the expression tree just to precompute the max attribute
>> number needed. That can't be good for expression compile speed, and
>> it introduces nontrivial bug risks because the code that does that
>> is completely decoupled from the code that emits the EEOP_VAR opcodes
>> (which are what's really relying on the de-forming to have happened).
> Hm. We had the separate traversal for projections for a long while, and
> I don't think there've been a a lot of changes to the extraction of the
> last attribute number.
That's easily disproven just by looking at the code:
/* * Don't examine the arguments or filters of Aggrefs or WindowFuncs, * because those do not represent
expressionsto be evaluated within the * calling expression's econtext. GroupingFunc arguments are never *
evaluatedat all. */ if (IsA(node, Aggref)) return false; if (IsA(node, WindowFunc)) return false;
if(IsA(node, GroupingFunc)) return false; return expression_tree_walker(node, get_last_attnums_walker,
(void *) info);
The WindowFunc exception hasn't been there so long, and the GroupingFunc
one is very new. And who's to say whether e.g. the recent XMLTABLE patch
got this right at all? We could easily be extracting columns we don't
need to.
I'm willing to leave this as-is for the moment, but I really think we
should look into changing it (after the main patch is in).
regards, tom lane