merging duplicate definitions of adjust_relid_set
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t36271psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 07:46 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t36271_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t36271_1 && git checkout t36271_1Patchset v1 (message #1) is on t36271_1
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?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@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.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Mar 17, 2017 at 1:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@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 ofBitmapset *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