many copies of atooid() and oid_cmp()
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:t35848psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 08:39 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 t35848_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 t35848_1 && git checkout t35848_1Patchset v1 (message #1) is on t35848_1
There are approximately 11 copies of atooid() and 3 of oid_cmp() or
equivalent, and pending patches are proposing to add more. I propose
these two patches to collect them in central places.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
There are approximately 11 copies of atooid() and 3 of oid_cmp() or
equivalent, and pending patches are proposing to add more. I propose
these two patches to collect them in central places.
+1 for the concept, but I'm a bit worried about putting atooid() in
postgres_ext.h. That's going to impose on the namespace of libpq-using
applications, for instance. A more conservative answer would be to
add it to c.h. OTOH, postgres_ext.h is where the Oid typedef lives,
so I do see the consistency of adding this there. Hard choice.
The oid_cmp() move looks fine if we only need it on the server side.
But doesn't pg_dump have one too?
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 Wed, Jan 11, 2017 at 9:42 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
There are approximately 11 copies of atooid() and 3 of oid_cmp() or
equivalent, and pending patches are proposing to add more. I propose
these two patches to collect them in central places.
I've verified that the patch covers all the copies of atooid() and
oid_cmp() that should be replaced. However, as Tom suggested, I've
looked into pg_dump and found that there is a oidcmp() as well. It may
have been missed because it has a different name.
I was wondering whether it is worth to replace the following as well:
(TransactionId)strtoul(str, NULL, 16) to something like #define atotranid().
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 1/11/17 11:25 PM, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
There are approximately 11 copies of atooid() and 3 of oid_cmp() or
equivalent, and pending patches are proposing to add more. I propose
these two patches to collect them in central places.+1 for the concept, but I'm a bit worried about putting atooid() in
postgres_ext.h. That's going to impose on the namespace of libpq-using
applications, for instance. A more conservative answer would be to
add it to c.h. OTOH, postgres_ext.h is where the Oid typedef lives,
so I do see the consistency of adding this there. Hard choice.
How about two copies: one in postgres_fe.h and one in postgres.h?
The oid_cmp() move looks fine if we only need it on the server side.
But doesn't pg_dump have one too?
The pg_dump one isn't a qsort comparator, though.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 1/12/17 12:25 AM, Kuntal Ghosh wrote:
On Wed, Jan 11, 2017 at 9:42 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:There are approximately 11 copies of atooid() and 3 of oid_cmp() or
equivalent, and pending patches are proposing to add more. I propose
these two patches to collect them in central places.I've verified that the patch covers all the copies of atooid() and
oid_cmp() that should be replaced. However, as Tom suggested, I've
looked into pg_dump and found that there is a oidcmp() as well. It may
have been missed because it has a different name.I was wondering whether it is worth to replace the following as well:
(TransactionId)strtoul(str, NULL, 16) to something like #define atotranid().
There is a atoxid(), which is actually not used and removed by my patch.
There are a few other calls of this pattern, but they are not frequent
or consistent enough to worry about (yet), I think.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 1/11/17 11:25 PM, Tom Lane wrote:
+1 for the concept, but I'm a bit worried about putting atooid() in
postgres_ext.h. That's going to impose on the namespace of libpq-using
applications, for instance. A more conservative answer would be to
add it to c.h. OTOH, postgres_ext.h is where the Oid typedef lives,
so I do see the consistency of adding this there. Hard choice.
How about two copies: one in postgres_fe.h and one in postgres.h?
That seems uglier than either of the other choices.
I don't personally have a huge problem with adding atooid in
postgres_ext.h, but I thought I'd better flag the potential issue
to see if anyone else thinks it's a big problem.
The oid_cmp() move looks fine if we only need it on the server side.
But doesn't pg_dump have one too?
The pg_dump one isn't a qsort comparator, though.
OK.
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 Thu, Jan 12, 2017 at 11:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 1/11/17 11:25 PM, Tom Lane wrote:
+1 for the concept, but I'm a bit worried about putting atooid() in
postgres_ext.h. That's going to impose on the namespace of libpq-using
applications, for instance. A more conservative answer would be to
add it to c.h. OTOH, postgres_ext.h is where the Oid typedef lives,
so I do see the consistency of adding this there. Hard choice.How about two copies: one in postgres_fe.h and one in postgres.h?
That seems uglier than either of the other choices.
I don't personally have a huge problem with adding atooid in
postgres_ext.h, but I thought I'd better flag the potential issue
to see if anyone else thinks it's a big problem.
FWIW, postgres_ext.h would make the most sense to me. Now, there is
one way to not impose that to frontends linked to libpq which would be
to locate it in some new header in src/include/common/, where both
backend and frontend could reference to it.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 1/12/17 09:36, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 1/11/17 11:25 PM, Tom Lane wrote:
+1 for the concept, but I'm a bit worried about putting atooid() in
postgres_ext.h. That's going to impose on the namespace of libpq-using
applications, for instance. A more conservative answer would be to
add it to c.h. OTOH, postgres_ext.h is where the Oid typedef lives,
so I do see the consistency of adding this there. Hard choice.How about two copies: one in postgres_fe.h and one in postgres.h?
That seems uglier than either of the other choices.
I don't personally have a huge problem with adding atooid in
postgres_ext.h, but I thought I'd better flag the potential issue
to see if anyone else thinks it's a big problem.
committed as is then
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers