Hi Tomas,
On 03.09.2016 14:37, Tomas Vondra wrote:
> Hi Ildar,
>
> I've looked at this patch again today to do a bit more thorough review,
> and I think it's fine. There are a few comments (particularly in the new
> code in check_index_only) that need improving, and also a few small
> tweaks in the walkers.
>
> Attached is a modified v5 patch with the proposed changes - it's
> probably easier than explaining what the changes should/might be.
>
> I've added an XXX comment in check_index_only_expr_walker - ISTM we're
> first explicitly matching Vars to index attributes, and then dealing
> with expressions. But we loop over all index columns (including those
> that can only really match Vars). Not sure if it makes any difference,
> but is it worth differentiating between Var and non-Var expressions? Why
> not to simply call match_index_to_operand() in both cases?
>
Thank you! Yes, you're right and probably it doesn't worth it. I
intended to optimize just a little bit since we already have attribute
numbers that can be returned by index. But as you have already noted in
comment it is a cheap check and it would barely be noticeable.
> I've also tweaked a few comments to match project code style, and moved
> a few variables into the block where they are used. But the latter is
> probably matter of personal taste, I guess.
>
>
> regards
>
Thanks for that, I have some difficulties in expressing myself in
english, so it was very helpful.
Best regards,
--
Ildar Musin
i.musin@postgrespro.ru