On Fri, Mar 17, 2017 at 1:50 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> While reviewing Ashutosh Bapat's partitionwise join code, I noticed
>> he'd run up against the problem that adjust_relid_set() is defined as
>> static in two different source files, and he wanted to call it from a
>> third file.  I didn't much like his solution to that problem, which
>> was to rename one of them and make that definition non-static; I think
>> it would be better to keep the existing name and stop defining it in
>> multiple places.  However, I discovered that there wasn't really an
>> obviously-good place to put the function; neither prepunion.c nor
>> rewriteManip.c, the two files that contain static versions as of now,
>> seem like an appropriate place from which to expose it, and I didn't
>> find anything else that I was wildly in love with, either.  The
>> attached patch puts it in var.c, because it didn't look horrible and I
>> thought it wasn't worth creating a new file just for this.
>> Objections, better ideas?
> I think it might be better to define it as a fundamental Bitmapset
> operation in nodes/bitmapset.c, along the lines of
> Bitmapset *bms_replace_member(const Bitmapset *bms, int member, int repl);
> This API would probably require giving up the property of not copying the
> set unless it changes, but I doubt that that's performance critical.

I thought of that, but I wasn't sure it was good in terms of clarity
to replace a function that works in terms of Relids with one that
works in terms of Bitmapset *.  I know they're the same, so it's just
a style thing.

But actually, I think my email was premature.  Actually, his patch
series first changes adjust_relid_set to take different arguments so
that it can do multiple translations at once, and then a later patch
in the series renames it.  So there's actually no need to merge the
definitions, because one of them ends up going away anyway.  So, uh,
never mind.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Reply via email to