Thread: Small improvement to get_base_rel_indexes()

Small improvement to get_base_rel_indexes()

From
David Rowley
Date:
Attached is a small patch which improves the way get_base_rel_indexes() works.

The current version creates a new bitmapset on each recursion level then bms_joins() to the one on the next level up each time. I understand that this will patch will have about a 0 net performance improvement, but I thought I'd post anyway as:

1. It removes 5 lines of code.
2. It's a better example to leave in the code.

Is it worth applying?

Regards

David Rowley
--
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
Attachment

Re: Small improvement to get_base_rel_indexes()

From
Tom Lane
Date:
David Rowley <david.rowley@2ndquadrant.com> writes:
> Attached is a small patch which improves the way get_base_rel_indexes()
> works.

> The current version creates a new bitmapset on each recursion level then
> bms_joins() to the one on the next level up each time. I understand that
> this will patch will have about a 0 net performance improvement, but I
> thought I'd post anyway as:

> 1. It removes 5 lines of code.
> 2. It's a better example to leave in the code.

I don't know that it's a better example: it seems substantially uglier.
(Notably, you complicated the API contract of get_base_rel_indexes()
without documenting the fact: it's now dependent on the original caller
to have initialized *result to NULL.)

We might have to use a method like this if the function were returning
multiple sets; but since it is not, the existing approach seems more
readable and safer to me.
        regards, tom lane