Re: Identity projection

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.

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.

- Heikki