Re: Small improvement to get_base_rel_indexes()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Small improvement to get_base_rel_indexes()
Date: 2015-08-15 19:05:59
Message-ID: 22140.1439665559@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Rowley <david(dot)rowley(at)2ndquadrant(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2015-08-15 21:58:16 Potential GIN vacuum bug
Previous Message Tom Lane 2015-08-15 17:48:17 Re: Raising our compiler requirements for 9.6