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: http://www.postgresql.org/mailpref/pgsql-hackers
