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.


Thanks for that, I have some difficulties in expressing myself in english, so it was very helpful.

Best regards,
Ildar Musin

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to