On Friday, March 08, 2013 11:21 PM Heikki Linnakangas wrote:
On 12.02.2013 11:03, Amit Kapila wrote:
>> + /*
>> + * equivalent_tlists
>> + * returns whether two traget lists are equivalent
>> + *
>> + * We consider two target lists equivalent if both have
>> + * only Var entries and resjunk of each target entry is same.
>> + *
>> + * This function is used to decide whether to create a result node.
>> + * We need to ensure that each entry is Var as below node will not be
>> + * projection capable, so it will not be able to compute expressions.
>> + */
>> + bool
>> + equivalent_tlists(List *tlist1, List *tlist2)
>> + {
>> + ListCell *lp,
>> + *lc;
>> +
>> + if (list_length(tlist1) != list_length(tlist2))
>> + return false; /* tlists not same length */
>> +
>> + forboth(lp, tlist1, lc, tlist2)
>> + {
>> + TargetEntry *tle1 = (TargetEntry *) lfirst(lp);
>> + TargetEntry *tle2 = (TargetEntry *) lfirst(lc);
>> +
>> + if (tle1->resjunk != tle1->resjunk)
>> + return false; /* tlist doesn't match junk status */
>> +
>> + if (tle1->expr && IsA(tle1->expr, Var) &&
>> + tle2->expr && IsA(tle2->expr, Var))
>> + {
>> + Var *var1 = (Var *) tle1->expr;
>> + Var *var2 = (Var *) tle2->expr;
>> +
>> + if (var1->varattno != var2->varattno)
>> + return false; /* different order */
>> + }
>> + else
>> + return false;
>> + }
>> + return true;
>> + }
> Hmm, shouldn't that also check Var.varno and Var.varlevelsup? I tried
> really hard to come up with a query that would fail because of that, but
> I wasn't able to find one. Nevertheless, seems like those fields should
> be checked.
I had reffered functions trivial_subqueryscan() and tlist_matches_tupdesc() which does
something similar. I had rechecked today and found that both of these functions have
Assert for Var.varno and Var.varlevelsup.
If you think that for current case we should have check rather than Assert, then I can
update the patch for same.
> On a more trivial note, equivalent_tlists() seems too generic for this.
> I'd expect two tlists that contain e.g an identical Const node to be
> "equivalent", but this returns false for it. I'd suggest calling it
> something like "tlist_needs_projection()" instead. Also, it probably
> belongs in tlist.c.
You are right. I shall update this in patch once above point for usage of vano and varlevelup is confirmed.
With Regards,
Amit Kapila.
