language cleanups in code and docs

https://en.wikipedia.org/wiki/Postmaster_(computing)

thomas.munro@gmail.com

over 5 years ago

In reply to: Daniel Gustafsson (#2)

Re: language cleanups in code and docs

On Tue, Jun 16, 2020 at 7:04 AM Daniel Gustafsson <daniel@yesql.se> wrote:

On 15 Jun 2020, at 20:22, Andres Freund <andres@anarazel.de> wrote:

Thanks for picking this up!

1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.

FWIW, I've never really liked the name postmaster as I don't think it conveys
meaning. I support renaming to supervisor or a similar term.

+1. Postmaster has always sounded like a mailer daemon or something,
which we ain't.

Bruce Momjian

bruce@momjian.us

over 5 years ago

In reply to: Thomas Munro (#3)

Re: language cleanups in code and docs

On Tue, Jun 16, 2020 at 09:53:34AM +1200, Thomas Munro wrote:

On Tue, Jun 16, 2020 at 7:04 AM Daniel Gustafsson <daniel@yesql.se> wrote:

On 15 Jun 2020, at 20:22, Andres Freund <andres@anarazel.de> wrote:

Thanks for picking this up!

1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.

FWIW, I've never really liked the name postmaster as I don't think it conveys
meaning. I support renaming to supervisor or a similar term.

+1. Postmaster has always sounded like a mailer daemon or something,
which we ain't.

Postmaster is historically confused with the postmaster email account:

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Daniel Gustafsson (#2)

Re: language cleanups in code and docs

Daniel Gustafsson <daniel@yesql.se> writes:

On 15 Jun 2020, at 20:22, Andres Freund <andres@anarazel.de> wrote:

1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.

FWIW, I've never really liked the name postmaster as I don't think it conveys
meaning. I support renaming to supervisor or a similar term.

Meh. That's carrying PC naming foibles to the point where they
negatively affect our users (by breaking start scripts and such).
I think we should leave this alone.

regards, tom lane

andres@anarazel.de

over 5 years ago

In reply to: Tom Lane (#5)

Re: language cleanups in code and docs

Hi,

On 2020-06-15 19:54:25 -0400, Tom Lane wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 15 Jun 2020, at 20:22, Andres Freund <andres@anarazel.de> wrote:

1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.

FWIW, I've never really liked the name postmaster as I don't think it conveys
meaning. I support renaming to supervisor or a similar term.

Meh. That's carrying PC naming foibles to the point where they
negatively affect our users (by breaking start scripts and such).
I think we should leave this alone.

postmaster is just a symlink, which we very well could just leave in
place... I was really just thinking of the code level stuff. And I think
there's some clarity reasons to rename it as well (see comments by
others in the thread).

Anyway, for now my focus is on patches in the series...

- Andres

Peter Geoghegan

pg@bowt.ie

over 5 years ago

In reply to: Tom Lane (#5)

Re: language cleanups in code and docs

On Mon, Jun 15, 2020 at 4:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Meh. That's carrying PC naming foibles to the point where they
negatively affect our users (by breaking start scripts and such).
I think we should leave this alone.

+1. Apart from the practical considerations, I just don't see a
problem with the word postmaster. My mother is a postmistress.

I'm in favor of updating any individual instances of the word "master"
to the preferred equivalent in code and code comments, though.

--
Peter Geoghegan

magnus@hagander.net

over 5 years ago

In reply to: Andres Freund (#6)

Re: language cleanups in code and docs

On Tue, Jun 16, 2020 at 2:23 AM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2020-06-15 19:54:25 -0400, Tom Lane wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 15 Jun 2020, at 20:22, Andres Freund <andres@anarazel.de> wrote:

1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.

FWIW, I've never really liked the name postmaster as I don't think it

conveys

meaning. I support renaming to supervisor or a similar term.

Meh. That's carrying PC naming foibles to the point where they
negatively affect our users (by breaking start scripts and such).
I think we should leave this alone.

postmaster is just a symlink, which we very well could just leave in
place... I was really just thinking of the code level stuff. And I think
there's some clarity reasons to rename it as well (see comments by
others in the thread).

Is the symlink even used? If not we could just get rid of it.

I'd be more worried about for example postmaster.pid, which would break a
*lot* of scripts and integrations. postmaster is also exposed in the system
catalogs.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

Joe Conway

mail@joeconway.com

over 5 years ago

In reply to: Magnus Hagander (#8)

Re: language cleanups in code and docs

On 6/16/20 3:26 AM, Magnus Hagander wrote:

On Tue, Jun 16, 2020 at 2:23 AM Andres Freund wrote:
postmaster is just a symlink, which we very well could just leave in
place... I was really just thinking of the code level stuff. And I think
there's some clarity reasons to rename it as well (see comments by
others in the thread).

Is the symlink even used? If not we could just get rid of it.

I am pretty sure that last time I checked Devrim was still using it in his
systemd unit file bundled with the PGDG rpms, although that was probably a
couple of years ago.

Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

#10

andrew.dunstan@2ndquadrant.com

over 5 years ago

In reply to: Joe Conway (#9)

Re: language cleanups in code and docs

On 6/16/20 9:10 AM, Joe Conway wrote:

On 6/16/20 3:26 AM, Magnus Hagander wrote:

On Tue, Jun 16, 2020 at 2:23 AM Andres Freund wrote:
postmaster is just a symlink, which we very well could just leave in
place... I was really just thinking of the code level stuff. And I think
there's some clarity reasons to rename it as well (see comments by
others in the thread).

Is the symlink even used? If not we could just get rid of it.

I am pretty sure that last time I checked Devrim was still using it in his
systemd unit file bundled with the PGDG rpms, although that was probably a
couple of years ago.

Just checked a recent install and it's there.

Honestly, I think I'm with Tom, and we can just let this one alone.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#11

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Magnus Hagander (#8)

Re: language cleanups in code and docs

Magnus Hagander <magnus@hagander.net> writes:

I'd be more worried about for example postmaster.pid, which would break a
*lot* of scripts and integrations. postmaster is also exposed in the system
catalogs.

Oooh, that's an excellent point. A lot of random stuff knows that file
name.

To be clear, I'm not against removing incidental uses of the word
"master". But the specific case of "postmaster" seems a little too
far ingrained to be worth changing.

regards, tom lane

#12

Bruce Momjian

bruce@momjian.us

over 5 years ago

In reply to: Andres Freund (#1)

Re: language cleanups in code and docs

On Mon, Jun 15, 2020 at 11:22:35AM -0700, Andres Freund wrote:

0006: docs: s/master/root/
Here using root seems a lot better than master anyway (master seems
confusing in regard to inheritance scenarios). But perhaps parent
would be better? Went with root since it's about the topmost table.

Because we allow multiple levels of inheritance, I have always wanted a
clear term for the top-most parent.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

#13

david@pgmasters.net

over 5 years ago

In reply to: Andres Freund (#1)

Re: language cleanups in code and docs

Hi Andres,

Thanks for doing this!

On 6/15/20 2:22 PM, Andres Freund wrote:

We've removed the use of "slave" from most of the repo (one use
remained, included here), but we didn't do the same for master. In the
attached series I replaced most of the uses.

0001: tap tests: s/master/primary/
Pretty clear cut imo.

Nothing to argue with here as far as I can see. It's a lot of churn,
though, so the sooner it goes in the better so people can update for the
next CF.

0002: code: s/master/primary/
This also includes a few minor other changes (s/in master/on the
primary/, a few 'the's added). Perhaps it'd be better to do those
separately?

I would only commit the grammar/language separately if the commit as a
whole does not go in. Some of them really do make the comments much
clearer beyond just in/on.

I think the user-facing messages, e.g.:

-			 errhint("Make sure the configuration parameter \"%s\" is set on the 
master server.",
+			 errhint("Make sure the configuration parameter \"%s\" is set on the 
primary server.",

should go in no matter what so we are consistent with our documentation.
Same for the postgresql.conf updates.

0003: code: s/master/leader/
This feels pretty obvious. We've largely used the leader / worker
terminology, but there were a few uses of master left.

Yeah, leader already outnumbers master by a lot. Looks good.

0004: code: s/master/$other/
This is most of the remaining uses of master in code. A number of
references to 'master' in the context of toast, a few uses of 'master
copy'. I guess some of these are a bit less clear cut.

Not sure I love authoritative, e.g.

+ * fullPageWrites is the authoritative value used by all backends to

and

+ * grabbed a WAL insertion lock to read the authoritative value in

Possibly "shared"?

+ * Create the Tcl interpreter subsidiary to pltcl_hold_interp.

Maybe use "worker" here? Not much we can do about the Tcl function name,
though. It's pretty localized, though, so may not matter much.

0005: docs: s/master/primary/
These seem mostly pretty straightforward to me. The changes in
high-availability.sgml probably deserve the most attention.

+ on primary and the current time on the standby. Delays in transfer

on *the* primary

I saw a few places where readability could be improved, but this patch
did not make any of them worse, and did make a few better.

0006: docs: s/master/root/
Here using root seems a lot better than master anyway (master seems
confusing in regard to inheritance scenarios). But perhaps parent
would be better? Went with root since it's about the topmost table.

I looked through to see if there was an instance of parent that should
be changed to root but I didn't see any. Even if there are, it's no
worse than before.

0007: docs: s/master/supervisor/
I guess this could be a bit more contentious. Supervisor seems clearer
to me, but I can see why people would disagree. See also later point
about changes I have not done at this stage.

Supervisor seems OK to me, but the postmaster has a special place since
there is only one of them. Main supervisor, root supervisor?

0008: docs: WIP multi-master rephrasing.
I like neither the new nor the old language much. I'd welcome input.

Why not multi-primary?

After this series there are only two widespread use of 'master' in the
tree.
1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.

FWIW, I don't consider this to be a very big change from an end-user
perspective. I don't think the majority of users interact directly with
the postmaster, rather they are using systemd, pg_ctl, pg_ctlcluster, etc.

As for postmaster.pid and postmaster.opts, we have renamed plenty of
things in the past so this is just one more. They'd be better and
clearer as postgresql.pid and postgresql.opts, IMO.

Overall I'm +.5 because I may just be ignorant of the pain this will cause.

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

Happily this has no end-user impact. I think "main" is a good
alternative but I agree this feels like a separate topic.

One last thing -- are we considering back-patching any/all of this?

Regards,
--
-David
david@pgmasters.net

#14

andres@anarazel.de

over 5 years ago

In reply to: David Steele (#13)

Re: language cleanups in code and docs

Hi,

On 2020-06-16 17:14:57 -0400, David Steele wrote:

On 6/15/20 2:22 PM, Andres Freund wrote:

We've removed the use of "slave" from most of the repo (one use
remained, included here), but we didn't do the same for master. In the
attached series I replaced most of the uses.

0001: tap tests: s/master/primary/
Pretty clear cut imo.

Nothing to argue with here as far as I can see. It's a lot of churn, though,
so the sooner it goes in the better so people can update for the next CF.

Yea, unless somebody protests I'm planning to push this part soon.

0004: code: s/master/$other/
This is most of the remaining uses of master in code. A number of
references to 'master' in the context of toast, a few uses of 'master
copy'. I guess some of these are a bit less clear cut.

Not sure I love authoritative, e.g.

+ * fullPageWrites is the authoritative value used by all backends to

and

+ * grabbed a WAL insertion lock to read the authoritative value in

Possibly "shared"?

I don't think shared is necessarily correct for all of these. E.g. in
the GetRedoRecPtr() there's two shared values at play, but only one is
"authoritative".

+ * Create the Tcl interpreter subsidiary to pltcl_hold_interp.

Maybe use "worker" here? Not much we can do about the Tcl function name,
though. It's pretty localized, though, so may not matter much.

I don't think it matters much what we use here

0008: docs: WIP multi-master rephrasing.
I like neither the new nor the old language much. I'd welcome input.

Why not multi-primary?

My understanding of primary is that there really can't be two things
that are primary in relation to each other. active/active is probably
the most common term in use besides multi-master.

One last thing -- are we considering back-patching any/all of this?

I don't think there's a good reason to do so.

Thanks for the look!

Greetings,

Andres Freund

#15

david@pgmasters.net

over 5 years ago

In reply to: Andres Freund (#14)

Re: language cleanups in code and docs

On 6/16/20 6:27 PM, Andres Freund wrote:

On 2020-06-16 17:14:57 -0400, David Steele wrote:

On 6/15/20 2:22 PM, Andres Freund wrote:

0008: docs: WIP multi-master rephrasing.
I like neither the new nor the old language much. I'd welcome input.

Why not multi-primary?

My understanding of primary is that there really can't be two things
that are primary in relation to each other.

Well, I think the same is true for multi-master and that's pretty common.

active/active is probably
the most common term in use besides multi-master.

Works for me and can always be updated later if we come up with
something better. At least active-active will be easier to search for.

One last thing -- are we considering back-patching any/all of this?

I don't think there's a good reason to do so.

I was thinking of back-patching pain but if you don't think that's an
issue then I'm not worried about it.

Thanks for the look!

You are welcome!

--
-David
david@pgmasters.net

#16

andrew.dunstan@2ndquadrant.com

over 5 years ago

In reply to: Andres Freund (#1)

Re: language cleanups in code and docs

On 6/15/20 2:22 PM, Andres Freund wrote:

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

I'm OK with this, but I would need plenty of notice to get the buildfarm
adjusted.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#17

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Andrew Dunstan (#16)

Re: language cleanups in code and docs

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/15/20 2:22 PM, Andres Freund wrote:

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

I'm OK with this, but I would need plenty of notice to get the buildfarm
adjusted.

"master" is the default branch name established by git, is it not? Not
something we picked. I don't feel like we need to be out front of the
rest of the world in changing that. It'd be a cheaper change than some of
the other proposals in this thread, no doubt, but it would still create
confusion for new hackers who are used to the standard git convention.

regards, tom lane

#18

Dave Cramer

davecramer@postgres.rocks

over 5 years ago

In reply to: Tom Lane (#17)

Re: language cleanups in code and docs

On Tue, 16 Jun 2020 at 19:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/15/20 2:22 PM, Andres Freund wrote:

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

I'm OK with this, but I would need plenty of notice to get the buildfarm
adjusted.

"master" is the default branch name established by git, is it not? Not
something we picked. I don't feel like we need to be out front of the
rest of the world in changing that. It'd be a cheaper change than some of
the other proposals in this thread, no doubt, but it would still create
confusion for new hackers who are used to the standard git convention.

While it is the default I expect that will change soon. Github is planning
on making main the default.
https://www.zdnet.com/article/github-to-replace-master-with-alternative-term-to-avoid-slavery-references/

Many projects are moving from master to main.

I expect it will be less confusing than you think.

Dave Cramer
www.postgres.rocks

#19

Alvaro Herrera

alvherre@2ndquadrant.com

over 5 years ago

In reply to: Tom Lane (#17)

Re: language cleanups in code and docs

On 2020-Jun-16, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/15/20 2:22 PM, Andres Freund wrote:

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

I'm OK with this, but I would need plenty of notice to get the buildfarm
adjusted.

"master" is the default branch name established by git, is it not? Not
something we picked. I don't feel like we need to be out front of the
rest of the world in changing that. It'd be a cheaper change than some of
the other proposals in this thread, no doubt, but it would still create
confusion for new hackers who are used to the standard git convention.

Git itself is discussing this:
https://public-inbox.org/git/41438A0F-50E4-4E58-A3A7-3DAAECB5576B@jramsay.com.au/T/#t
and it seems that "main" is the winning choice.

(Personally) I would leave master to have root, would leave root to have
default, would leave default to have primary, would leave primary to
have base, would leave base to have main, would leave main to have
mainline.

--
ï¿½lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#20

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Alvaro Herrera (#19)

Re: language cleanups in code and docs

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On 2020-Jun-16, Tom Lane wrote:

"master" is the default branch name established by git, is it not? Not
something we picked.

Git itself is discussing this:
https://public-inbox.org/git/41438A0F-50E4-4E58-A3A7-3DAAECB5576B@jramsay.com.au/T/#t
and it seems that "main" is the winning choice.

Oh, interesting. If they do change I'd be happy to follow suit.
But let's wait and see what they do, rather than possibly ending
up with our own private convention.

regards, tom lane

#21

magnus@hagander.net

over 5 years ago

In reply to: Tom Lane (#20)

Re: language cleanups in code and docs

On Wed, Jun 17, 2020 at 3:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On 2020-Jun-16, Tom Lane wrote:

"master" is the default branch name established by git, is it not? Not
something we picked.

Git itself is discussing this:

https://public-inbox.org/git/41438A0F-50E4-4E58-A3A7-3DAAECB5576B@jramsay.com.au/T/#t

and it seems that "main" is the winning choice.

Oh, interesting. If they do change I'd be happy to follow suit.
But let's wait and see what they do, rather than possibly ending
up with our own private convention.

I'm +1 for changing it (with good warning time to handle the buildfarm
situation), but also very much +1 for waiting to see exactly what upstream
(git) decides on and make sure we change to the same. The worst possible
combination would be that we change it to something that's *different* than
upstream ends up with (even if upstream ends up being configurable).

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

#22

Laurenz Albe

laurenz.albe@cybertec.at

over 5 years ago

In reply to: Tom Lane (#17)

Re: language cleanups in code and docs

On Tue, 2020-06-16 at 19:55 -0400, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/15/20 2:22 PM, Andres Freund wrote:

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

I'm OK with this, but I would need plenty of notice to get the buildfarm
adjusted.

"master" is the default branch name established by git, is it not? Not
something we picked. I don't feel like we need to be out front of the
rest of the world in changing that. It'd be a cheaper change than some of
the other proposals in this thread, no doubt, but it would still create
confusion for new hackers who are used to the standard git convention.

I have the feeling that all this is going somewhat too far.

I feel fine with removing the term "slave", even though I have no qualms
about enslaving machines.

But the term "master" is not restricted to slavery. It can just as well
imply expert knowledge (think academic degree), and it can denote someone
with the authority to command (there is nothing wrong with "servant", right?
Or do we have to abolish the term "server" too?).

I appreciate that other people's sensitivities might be different, and I
don't want to start a fight over it. But renaming things makes the code
history harder to read, so it should be used with moderation.

Yours,
Laurenz Albe

#23

magnus@hagander.net

over 5 years ago

In reply to: Andres Freund (#1)

1 attachment(s)

Re: language cleanups in code and docs

On Mon, Jun 15, 2020 at 8:23 PM Andres Freund <andres@anarazel.de> wrote:

Hi,

We've removed the use of "slave" from most of the repo (one use
remained, included here), but we didn't do the same for master. In the
attached series I replaced most of the uses.

0001: tap tests: s/master/primary/
Pretty clear cut imo.

0002: code: s/master/primary/
This also includes a few minor other changes (s/in master/on the
primary/, a few 'the's added). Perhaps it'd be better to do those
separately?

0003: code: s/master/leader/
This feels pretty obvious. We've largely used the leader / worker
terminology, but there were a few uses of master left.

0004: code: s/master/$other/
This is most of the remaining uses of master in code. A number of
references to 'master' in the context of toast, a few uses of 'master
copy'. I guess some of these are a bit less clear cut.

0005: docs: s/master/primary/
These seem mostly pretty straightforward to me. The changes in
high-availability.sgml probably deserve the most attention.

0006: docs: s/master/root/
Here using root seems a lot better than master anyway (master seems
confusing in regard to inheritance scenarios). But perhaps parent
would be better? Went with root since it's about the topmost table.

0007: docs: s/master/supervisor/
I guess this could be a bit more contentious. Supervisor seems clearer
to me, but I can see why people would disagree. See also later point
about changes I have not done at this stage.

0008: docs: WIP multi-master rephrasing.
I like neither the new nor the old language much. I'd welcome input.

After this series there are only two widespread use of 'master' in the
tree.
1) 'postmaster'. As changing that would be somewhat invasive, the word
is a bit more ambiguous, and it's largely just internal, I've left
this alone for now. I personally would rather see this renamed as
supervisor, which'd imo actually would also be a lot more
descriptive. I'm willing to do the work, but only if there's at least
some agreement.
2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

In looking at this I realize we also have exactly one thing referred to as
"blacklist" in our codebase, which is the "enum blacklist" (and then a
small internal variable in pgindent). AFAICT, it's not actually exposed to
userspace anywhere, so we could probably make the attached change to
blocklist at no "cost" (the only thing changed is the name of the hash
table, and we definitely change things like that in normal releases with no
specific thought on backwards compat).

//Magnus

Attachments:

blocklist.patchtext/x-patch; charset=US-ASCII; name=blocklist.patchDownload

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 14a8690019..6f93b41e4f 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -75,7 +75,7 @@
 #define PARALLEL_KEY_PENDING_SYNCS			UINT64CONST(0xFFFFFFFFFFFF000B)
 #define PARALLEL_KEY_REINDEX_STATE			UINT64CONST(0xFFFFFFFFFFFF000C)
 #define PARALLEL_KEY_RELMAPPER_STATE		UINT64CONST(0xFFFFFFFFFFFF000D)
-#define PARALLEL_KEY_ENUMBLACKLIST			UINT64CONST(0xFFFFFFFFFFFF000E)
+#define PARALLEL_KEY_ENUMBLOCKLIST			UINT64CONST(0xFFFFFFFFFFFF000E)
 
 /* Fixed-size parallel state. */
 typedef struct FixedParallelState
@@ -211,7 +211,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	Size		pendingsyncslen = 0;
 	Size		reindexlen = 0;
 	Size		relmapperlen = 0;
-	Size		enumblacklistlen = 0;
+	Size		enumblocklistlen = 0;
 	Size		segsize = 0;
 	int			i;
 	FixedParallelState *fps;
@@ -267,8 +267,8 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		shm_toc_estimate_chunk(&pcxt->estimator, reindexlen);
 		relmapperlen = EstimateRelationMapSpace();
 		shm_toc_estimate_chunk(&pcxt->estimator, relmapperlen);
-		enumblacklistlen = EstimateEnumBlacklistSpace();
-		shm_toc_estimate_chunk(&pcxt->estimator, enumblacklistlen);
+		enumblocklistlen = EstimateEnumBlocklistSpace();
+		shm_toc_estimate_chunk(&pcxt->estimator, enumblocklistlen);
 		/* If you add more chunks here, you probably need to add keys. */
 		shm_toc_estimate_keys(&pcxt->estimator, 11);
 
@@ -348,7 +348,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		char	   *error_queue_space;
 		char	   *session_dsm_handle_space;
 		char	   *entrypointstate;
-		char	   *enumblacklistspace;
+		char	   *enumblocklistspace;
 		Size		lnamelen;
 
 		/* Serialize shared libraries we have loaded. */
@@ -404,11 +404,11 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		shm_toc_insert(pcxt->toc, PARALLEL_KEY_RELMAPPER_STATE,
 					   relmapperspace);
 
-		/* Serialize enum blacklist state. */
-		enumblacklistspace = shm_toc_allocate(pcxt->toc, enumblacklistlen);
-		SerializeEnumBlacklist(enumblacklistspace, enumblacklistlen);
-		shm_toc_insert(pcxt->toc, PARALLEL_KEY_ENUMBLACKLIST,
-					   enumblacklistspace);
+		/* Serialize enum blocklist state. */
+		enumblocklistspace = shm_toc_allocate(pcxt->toc, enumblocklistlen);
+		SerializeEnumBlocklist(enumblocklistspace, enumblocklistlen);
+		shm_toc_insert(pcxt->toc, PARALLEL_KEY_ENUMBLOCKLIST,
+					   enumblocklistspace);
 
 		/* Allocate space for worker information. */
 		pcxt->worker = palloc0(sizeof(ParallelWorkerInfo) * pcxt->nworkers);
@@ -1257,7 +1257,7 @@ ParallelWorkerMain(Datum main_arg)
 	char	   *pendingsyncsspace;
 	char	   *reindexspace;
 	char	   *relmapperspace;
-	char	   *enumblacklistspace;
+	char	   *enumblocklistspace;
 	StringInfoData msgbuf;
 	char	   *session_dsm_handle_space;
 
@@ -1449,10 +1449,10 @@ ParallelWorkerMain(Datum main_arg)
 	relmapperspace = shm_toc_lookup(toc, PARALLEL_KEY_RELMAPPER_STATE, false);
 	RestoreRelationMap(relmapperspace);
 
-	/* Restore enum blacklist. */
-	enumblacklistspace = shm_toc_lookup(toc, PARALLEL_KEY_ENUMBLACKLIST,
+	/* Restore enum blocklist. */
+	enumblocklistspace = shm_toc_lookup(toc, PARALLEL_KEY_ENUMBLOCKLIST,
 										false);
-	RestoreEnumBlacklist(enumblacklistspace);
+	RestoreEnumBlocklist(enumblocklistspace);
 
 	/* Attach to the leader's serializable transaction, if SERIALIZABLE. */
 	AttachSerializableXact(fps->serializable_xact_handle);
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index 27e4100a6f..0221313433 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -41,10 +41,10 @@ Oid			binary_upgrade_next_pg_enum_oid = InvalidOid;
  * committed; otherwise, they might get into indexes where we can't clean
  * them up, and then if the transaction rolls back we have a broken index.
  * (See comments for check_safe_enum_use() in enum.c.)  Values created by
- * EnumValuesCreate are *not* blacklisted; we assume those are created during
+ * EnumValuesCreate are *not* blocklisted; we assume those are created during
  * CREATE TYPE, so they can't go away unless the enum type itself does.
  */
-static HTAB *enum_blacklist = NULL;
+static HTAB *enum_blocklist = NULL;
 
 static void RenumberEnumType(Relation pg_enum, HeapTuple *existing, int nelems);
 static int	sort_order_cmp(const void *p1, const void *p2);
@@ -181,10 +181,10 @@ EnumValuesDelete(Oid enumTypeOid)
 }
 
 /*
- * Initialize the enum blacklist for this transaction.
+ * Initialize the enum blocklist for this transaction.
  */
 static void
-init_enum_blacklist(void)
+init_enum_blocklist(void)
 {
 	HASHCTL		hash_ctl;
 
@@ -192,7 +192,7 @@ init_enum_blacklist(void)
 	hash_ctl.keysize = sizeof(Oid);
 	hash_ctl.entrysize = sizeof(Oid);
 	hash_ctl.hcxt = TopTransactionContext;
-	enum_blacklist = hash_create("Enum value blacklist",
+	enum_blocklist = hash_create("Enum value blocklist",
 								 32,
 								 &hash_ctl,
 								 HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
@@ -491,12 +491,12 @@ restart:
 
 	table_close(pg_enum, RowExclusiveLock);
 
-	/* Set up the blacklist hash if not already done in this transaction */
-	if (enum_blacklist == NULL)
-		init_enum_blacklist();
+	/* Set up the blocklist hash if not already done in this transaction */
+	if (enum_blocklist == NULL)
+		init_enum_blocklist();
 
-	/* Add the new value to the blacklist */
-	(void) hash_search(enum_blacklist, &newOid, HASH_ENTER, NULL);
+	/* Add the new value to the blocklist */
+	(void) hash_search(enum_blocklist, &newOid, HASH_ENTER, NULL);
 }
 
 
@@ -585,19 +585,19 @@ RenameEnumLabel(Oid enumTypeOid,
 
 
 /*
- * Test if the given enum value is on the blacklist
+ * Test if the given enum value is on the blocklist
  */
 bool
-EnumBlacklisted(Oid enum_id)
+EnumBlocklisted(Oid enum_id)
 {
 	bool		found;
 
-	/* If we've made no blacklist table, all values are safe */
-	if (enum_blacklist == NULL)
+	/* If we've made no blocklist table, all values are safe */
+	if (enum_blocklist == NULL)
 		return false;
 
 	/* Else, is it in the table? */
-	(void) hash_search(enum_blacklist, &enum_id, HASH_FIND, &found);
+	(void) hash_search(enum_blocklist, &enum_id, HASH_FIND, &found);
 	return found;
 }
 
@@ -609,11 +609,11 @@ void
 AtEOXact_Enum(void)
 {
 	/*
-	 * Reset the blacklist table, as all our enum values are now committed.
+	 * Reset the blocklist table, as all our enum values are now committed.
 	 * The memory will go away automatically when TopTransactionContext is
 	 * freed; it's sufficient to clear our pointer.
 	 */
-	enum_blacklist = NULL;
+	enum_blocklist = NULL;
 }
 
 
@@ -692,12 +692,12 @@ sort_order_cmp(const void *p1, const void *p2)
 }
 
 Size
-EstimateEnumBlacklistSpace(void)
+EstimateEnumBlocklistSpace(void)
 {
 	size_t		entries;
 
-	if (enum_blacklist)
-		entries = hash_get_num_entries(enum_blacklist);
+	if (enum_blocklist)
+		entries = hash_get_num_entries(enum_blocklist);
 	else
 		entries = 0;
 
@@ -706,7 +706,7 @@ EstimateEnumBlacklistSpace(void)
 }
 
 void
-SerializeEnumBlacklist(void *space, Size size)
+SerializeEnumBlocklist(void *space, Size size)
 {
 	Oid		   *serialized = (Oid *) space;
 
@@ -714,15 +714,15 @@ SerializeEnumBlacklist(void *space, Size size)
 	 * Make sure the hash table hasn't changed in size since the caller
 	 * reserved the space.
 	 */
-	Assert(size == EstimateEnumBlacklistSpace());
+	Assert(size == EstimateEnumBlocklistSpace());
 
 	/* Write out all the values from the hash table, if there is one. */
-	if (enum_blacklist)
+	if (enum_blocklist)
 	{
 		HASH_SEQ_STATUS status;
 		Oid		   *value;
 
-		hash_seq_init(&status, enum_blacklist);
+		hash_seq_init(&status, enum_blocklist);
 		while ((value = (Oid *) hash_seq_search(&status)))
 			*serialized++ = *value;
 	}
@@ -738,11 +738,11 @@ SerializeEnumBlacklist(void *space, Size size)
 }
 
 void
-RestoreEnumBlacklist(void *space)
+RestoreEnumBlocklist(void *space)
 {
 	Oid		   *serialized = (Oid *) space;
 
-	Assert(!enum_blacklist);
+	Assert(!enum_blocklist);
 
 	/*
 	 * As a special case, if the list is empty then don't even bother to
@@ -753,9 +753,9 @@ RestoreEnumBlacklist(void *space)
 		return;
 
 	/* Read all the values into a new hash table. */
-	init_enum_blacklist();
+	init_enum_blocklist();
 	do
 	{
-		hash_search(enum_blacklist, serialized++, HASH_ENTER, NULL);
+		hash_search(enum_blocklist, serialized++, HASH_ENTER, NULL);
 	} while (OidIsValid(*serialized));
 }
diff --git a/src/backend/utils/adt/enum.c b/src/backend/utils/adt/enum.c
index 5ead794e34..ac78ca853f 100644
--- a/src/backend/utils/adt/enum.c
+++ b/src/backend/utils/adt/enum.c
@@ -83,12 +83,12 @@ check_safe_enum_use(HeapTuple enumval_tup)
 		return;
 
 	/*
-	 * Check if the enum value is blacklisted.  If not, it's safe, because it
+	 * Check if the enum value is blocklisted.  If not, it's safe, because it
 	 * was made during CREATE TYPE AS ENUM and can't be shorter-lived than its
 	 * owning type.  (This'd also be false for values made by other
 	 * transactions; but the previous tests should have handled all of those.)
 	 */
-	if (!EnumBlacklisted(en->oid))
+	if (!EnumBlocklisted(en->oid))
 		return;
 
 	/*
diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h
index b28d441ba7..fcf9bc537c 100644
--- a/src/include/catalog/pg_enum.h
+++ b/src/include/catalog/pg_enum.h
@@ -53,10 +53,10 @@ extern void AddEnumLabel(Oid enumTypeOid, const char *newVal,
 						 bool skipIfExists);
 extern void RenameEnumLabel(Oid enumTypeOid,
 							const char *oldVal, const char *newVal);
-extern bool EnumBlacklisted(Oid enum_id);
-extern Size EstimateEnumBlacklistSpace(void);
-extern void SerializeEnumBlacklist(void *space, Size size);
-extern void RestoreEnumBlacklist(void *space);
+extern bool EnumBlocklisted(Oid enum_id);
+extern Size EstimateEnumBlocklistSpace(void);
+extern void SerializeEnumBlocklist(void *space, Size size);
+extern void RestoreEnumBlocklist(void *space);
 extern void AtEOXact_Enum(void);
 
 #endif							/* PG_ENUM_H */
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 457e328824..67582048b9 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -55,11 +55,11 @@ $excludes ||= "$code_base/src/tools/pgindent/exclude_file_patterns"
 # according to <stdbool.h>), and may include some names we don't want
 # treated as typedefs, although various headers that some builds include
 # might make them so.  For the moment we just hardwire a whitelist of names
-# to add and a blacklist of names to remove; eventually this may need to be
+# to add and a blocklist of names to remove; eventually this may need to be
 # easier to configure.  Note that the typedefs need trailing newlines.
 my @whitelist = ("bool\n");
 
-my %blacklist = map { +"$_\n" => 1 } qw(
+my %blocklist = map { +"$_\n" => 1 } qw(
   ANY FD_SET U abs allocfunc boolean date digit ilist interval iterator other
   pointer printfunc reference string timestamp type wrap
 );
@@ -137,8 +137,8 @@ sub load_typedefs
 	# add whitelisted entries
 	push(@typedefs, @whitelist);
 
-	# remove blacklisted entries
-	@typedefs = grep { !$blacklist{$_} } @typedefs;
+	# remove blocklisted entries
+	@typedefs = grep { !$blocklist{$_} } @typedefs;
 
 	# write filtered typedefs
 	my $filter_typedefs_fh = new File::Temp(TEMPLATE => "pgtypedefXXXXX");

#24

Jonathan S. Katz

jkatz@postgresql.org

over 5 years ago

In reply to: Magnus Hagander (#23)

1 attachment(s)

Re: language cleanups in code and docs

On 6/17/20 6:32 AM, Magnus Hagander wrote:

In looking at this I realize we also have exactly one thing referred to
as "blacklist" in our codebase, which is the "enum blacklist" (and then
a small internal variable in pgindent). AFAICT, it's not actually
exposed to userspace anywhere, so we could probably make the attached
change to blocklist at no "cost" (the only thing changed is the name of
the hash table, and we definitely change things like that in normal
releases with no specific thought on backwards compat).

+1. Though if we are doing that, we should also handle "whitelist" too,
as this attached patch does. It's mostly in comments (with one Perl
variable), but I switched the language around to use "allowed"

Jonathan

Attachments:

allowed.patchtext/plain; charset=UTF-8; name=allowed.patch; x-mac-creator=0; x-mac-type=0Download

diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index eef410db39..3364a203c7 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -77,7 +77,7 @@ typedef struct PgFdwRelationInfo
 	bool		use_remote_estimate;
 	Cost		fdw_startup_cost;
 	Cost		fdw_tuple_cost;
-	List	   *shippable_extensions;	/* OIDs of whitelisted extensions */
+	List	   *shippable_extensions;	/* OIDs of allowed extensions */
 
 	/* Cached catalog information. */
 	ForeignTable *table;
diff --git a/contrib/postgres_fdw/shippable.c b/contrib/postgres_fdw/shippable.c
index 3433c19712..8a3dca9e90 100644
--- a/contrib/postgres_fdw/shippable.c
+++ b/contrib/postgres_fdw/shippable.c
@@ -111,7 +111,8 @@ InitializeShippableCache(void)
  *
  * Right now "shippability" is exclusively a function of whether the object
  * belongs to an extension declared by the user.  In the future we could
- * additionally have a whitelist of functions/operators declared one at a time.
+ * additionally have a list of allowed functions/operators declared one at a
+ * time.
  */
 static bool
 lookup_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo)
diff --git a/src/backend/access/hash/hashvalidate.c b/src/backend/access/hash/hashvalidate.c
index 6f14a9fb45..744d22b042 100644
--- a/src/backend/access/hash/hashvalidate.c
+++ b/src/backend/access/hash/hashvalidate.c
@@ -309,7 +309,7 @@ check_hash_func_signature(Oid funcid, int16 amprocnum, Oid argtype)
 		 * that are different from but physically compatible with the opclass
 		 * datatype.  In some of these cases, even a "binary coercible" check
 		 * fails because there's no relevant cast.  For the moment, fix it by
-		 * having a whitelist of allowed cases.  Test the specific function
+		 * having a list of allowed cases.  Test the specific function
 		 * identity, not just its input type, because hashvarlena() takes
 		 * INTERNAL and allowing any such function seems too scary.
 		 */
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index e992d1bbfc..267fc7adfd 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -632,9 +632,9 @@ pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
 	/*
 	 * Check if blocked_pid is waiting for a safe snapshot.  We could in
 	 * theory check the resulting array of blocker PIDs against the
-	 * interesting PIDs whitelist, but since there is no danger of autovacuum
-	 * blocking GetSafeSnapshot there seems to be no point in expending cycles
-	 * on allocating a buffer and searching for overlap; so it's presently
+	 * interesting list of allowed PIDs, but since there is no danger of
+	 * autovacuum blocking GetSafeSnapshot there seems to be no point in expending
+	 * cycles on allocating a buffer and searching for overlap; so it's presently
 	 * sufficient for the isolation tester's purposes to use a single element
 	 * buffer and check if the number of safe snapshot blockers is non-zero.
 	 */
diff --git a/src/tools/pginclude/README b/src/tools/pginclude/README
index a067c7f472..d1906a3516 100644
--- a/src/tools/pginclude/README
+++ b/src/tools/pginclude/README
@@ -64,7 +64,7 @@ with no prerequisite headers other than postgres.h (or postgres_fe.h
 or c.h, as appropriate).
 
 A small number of header files are exempted from this requirement,
-and are whitelisted in the headerscheck script.
+and are allowed in the headerscheck script.
 
 The easy way to run the script is to say "make -s headerscheck" in
 the top-level build directory after completing a build.  You should
@@ -86,7 +86,7 @@ the project's coding language is C, some people write extensions in C++,
 so it's helpful for include files to be C++-clean.
 
 A small number of header files are exempted from this requirement,
-and are whitelisted in the cpluspluscheck script.
+and are allowed in the cpluspluscheck script.
 
 The easy way to run the script is to say "make -s cpluspluscheck" in
 the top-level build directory after completing a build.  You should
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 457e328824..06e4d68991 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -54,10 +54,10 @@ $excludes ||= "$code_base/src/tools/pgindent/exclude_file_patterns"
 # some names we want to treat like typedefs, e.g. "bool" (which is a macro
 # according to <stdbool.h>), and may include some names we don't want
 # treated as typedefs, although various headers that some builds include
-# might make them so.  For the moment we just hardwire a whitelist of names
-# to add and a blacklist of names to remove; eventually this may need to be
-# easier to configure.  Note that the typedefs need trailing newlines.
-my @whitelist = ("bool\n");
+# might make them so.  For the moment we just hardwire a list of allowed of
+# names to add and a blacklist of names to remove; eventually this may need to
+# be  easier to configure.  Note that the typedefs need trailing newlines.
+my @allowed = ("bool\n");
 
 my %blacklist = map { +"$_\n" => 1 } qw(
   ANY FD_SET U abs allocfunc boolean date digit ilist interval iterator other
@@ -134,8 +134,8 @@ sub load_typedefs
 		}
 	}
 
-	# add whitelisted entries
-	push(@typedefs, @whitelist);
+	# add allowed entries
+	push(@typedefs, @allowed);
 
 	# remove blacklisted entries
 	@typedefs = grep { !$blacklist{$_} } @typedefs;

#25

Jonathan S. Katz

jkatz@postgresql.org

over 5 years ago

In reply to: Laurenz Albe (#22)

Re: language cleanups in code and docs

On 6/17/20 6:06 AM, Laurenz Albe wrote:

On Tue, 2020-06-16 at 19:55 -0400, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/15/20 2:22 PM, Andres Freund wrote:

2) 'master' as a reference to the branch. Personally I be in favor of
changing the branch name, but it seems like it'd be better done as a
somewhat separate discussion to me, as it affects development
practices to some degree.

I'm OK with this, but I would need plenty of notice to get the buildfarm
adjusted.

"master" is the default branch name established by git, is it not? Not
something we picked. I don't feel like we need to be out front of the
rest of the world in changing that. It'd be a cheaper change than some of
the other proposals in this thread, no doubt, but it would still create
confusion for new hackers who are used to the standard git convention.

I have the feeling that all this is going somewhat too far.

First, I +1 the changes Andres proposed overall. In addition to it being
the right thing to do, it brings inline a lot of the terminology we have
been using to describe concepts in PostgreSQL for years (e.g.
primary/replica).

For the name of the git branch, I +1 following the convention of the git
upstream, and make changes based on that. Understandably, it could break
things for a bit, but that will occur for a lot of other projects as
well and everyone will adopt. We have the benefit that we're just
beginning our new development cycle too, so this is a good time to
introduce breaking change ;)

Jonathan

#26

andrew.dunstan@2ndquadrant.com

over 5 years ago

In reply to: Magnus Hagander (#23)

Re: language cleanups in code and docs

On 6/17/20 6:32 AM, Magnus Hagander wrote:

In looking at this I realize we also have exactly one thing referred
to as "blacklist" in our codebase, which is the "enum blacklist" (and
then a small internal variable in pgindent). AFAICT, it's not actually
exposed to userspace anywhere, so we could probably make the attached
change to blocklist at no "cost" (the only thing changed is the name
of the hash table, and we definitely change things like that in normal
releases with no specific thought on backwards compat).

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There are
too many other uses of Block in the sources. Forbidden might be a better
substitution, or Banned maybe. BanList is even less characters than
BlackList.

I know, bikeshedding here.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#27

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Andrew Dunstan (#26)

Re: language cleanups in code and docs

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/17/20 6:32 AM, Magnus Hagander wrote:

In looking at this I realize we also have exactly one thing referred
to as "blacklist" in our codebase, which is the "enum blacklist" (and
then a small internal variable in pgindent). AFAICT, it's not actually
exposed to userspace anywhere, so we could probably make the attached
change to blocklist at no "cost" (the only thing changed is the name
of the hash table, and we definitely change things like that in normal
releases with no specific thought on backwards compat).

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There are
too many other uses of Block in the sources. Forbidden might be a better
substitution, or Banned maybe. BanList is even less characters than
BlackList.

I think worrying about blacklist/whitelist is carrying things a bit far
in the first place.

regards, tom lane

#28

andrew.dunstan@2ndquadrant.com

over 5 years ago

In reply to: Tom Lane (#27)

Re: language cleanups in code and docs

On 6/17/20 11:00 AM, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/17/20 6:32 AM, Magnus Hagander wrote:

In looking at this I realize we also have exactly one thing referred
to as "blacklist" in our codebase, which is the "enum blacklist" (and
then a small internal variable in pgindent). AFAICT, it's not actually
exposed to userspace anywhere, so we could probably make the attached
change to blocklist at no "cost" (the only thing changed is the name
of the hash table, and we definitely change things like that in normal
releases with no specific thought on backwards compat).

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There are
too many other uses of Block in the sources. Forbidden might be a better
substitution, or Banned maybe. BanList is even less characters than
BlackList.

I think worrying about blacklist/whitelist is carrying things a bit far
in the first place.

For the small effort and minimal impact involved, I think it's worth
avoiding the bad publicity.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#29

magnus@hagander.net

over 5 years ago

In reply to: Andrew Dunstan (#26)

Re: language cleanups in code and docs

On Wed, Jun 17, 2020 at 4:15 PM Andrew Dunstan <
andrew.dunstan@2ndquadrant.com> wrote:

On 6/17/20 6:32 AM, Magnus Hagander wrote:

In looking at this I realize we also have exactly one thing referred
to as "blacklist" in our codebase, which is the "enum blacklist" (and
then a small internal variable in pgindent). AFAICT, it's not actually
exposed to userspace anywhere, so we could probably make the attached
change to blocklist at no "cost" (the only thing changed is the name
of the hash table, and we definitely change things like that in normal
releases with no specific thought on backwards compat).

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There are
too many other uses of Block in the sources. Forbidden might be a better
substitution, or Banned maybe. BanList is even less characters than
BlackList.

I know, bikeshedding here.

I'd be OK with either of those really -- I went with block because it was
the easiest one :)

Not sure the number of characters is the important part :) Banlist does
make sense to me for other reasons though -- it's what it is, isn't it? It
bans those oids from being used in the current session -- I don't think
there's any struggle to "make that sentence work", which means that seems
like the relevant term.

I do think it's worth doing -- it's a small round of changes, and it
doesn't change anything user-exposed, so the cost for us is basically zero.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

#30

Jonathan S. Katz

jkatz@postgresql.org

over 5 years ago

In reply to: Magnus Hagander (#29)

Re: language cleanups in code and docs

On 6/17/20 12:08 PM, Magnus Hagander wrote:

On Wed, Jun 17, 2020 at 4:15 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com <mailto:andrew.dunstan@2ndquadrant.com>>
wrote:

On 6/17/20 6:32 AM, Magnus Hagander wrote:

In looking at this I realize we also have exactly one thing referred
to as "blacklist" in our codebase, which is the "enum blacklist" (and
then a small internal variable in pgindent). AFAICT, it's not actually
exposed to userspace anywhere, so we could probably make the attached
change to blocklist at no "cost" (the only thing changed is the name
of the hash table, and we definitely change things like that in normal
releases with no specific thought on backwards compat).

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There are
too many other uses of Block in the sources. Forbidden might be a better
substitution, or Banned maybe. BanList is even less characters than
BlackList.

I know, bikeshedding here.

I'd be OK with either of those really -- I went with block because it
was the easiest one :)

Not sure the number of characters is the important part :) Banlist does
make sense to me for other reasons though -- it's what it is, isn't it?
It bans those oids from being used in the current session -- I don't
think there's any struggle to "make that sentence work", which means
that seems like the relevant term.

I do think it's worth doing -- it's a small round of changes, and it
doesn't change anything user-exposed, so the cost for us is basically zero.

+1. I know post efforts for us to update our language have been
well-received, even long after the fact, and given this set has been
voiced actively and other fora and, as Magnus states, the cost for us to
change it is basically zero, we should just do it.

Jonathan

#31

david@pgmasters.net

over 5 years ago

In reply to: Magnus Hagander (#29)

Re: language cleanups in code and docs

On 6/17/20 12:08 PM, Magnus Hagander wrote:

On Wed, Jun 17, 2020 at 4:15 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com <mailto:andrew.dunstan@2ndquadrant.com>>

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There are
too many other uses of Block in the sources. Forbidden might be a better
substitution, or Banned maybe. BanList is even less characters than
BlackList.

I'd be OK with either of those really -- I went with block because it
was the easiest one :)

Not sure the number of characters is the important part :) Banlist does
make sense to me for other reasons though -- it's what it is, isn't it?
It bans those oids from being used in the current session -- I don't
think there's any struggle to "make that sentence work", which means
that seems like the relevant term.

I've seen also seen allowList/denyList as an alternative. I do agree
that blockList is a bit confusing since we often use block in a very
different context.

I do think it's worth doing -- it's a small round of changes, and it
doesn't change anything user-exposed, so the cost for us is basically zero.

Regards,
--
-David
david@pgmasters.net

#32

Robert Haas

robertmhaas@gmail.com

over 5 years ago

In reply to: Andres Freund (#1)

Re: language cleanups in code and docs

On Mon, Jun 15, 2020 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:

0002: code: s/master/primary/
0003: code: s/master/leader/
0006: docs: s/master/root/
0007: docs: s/master/supervisor/

I'd just like to make the pointer here that there's value in trying to
use different terminology for different things. I picked "leader" and
"worker" for parallel query and tried to use them consistently because
"master" and "slave" were being used widely to refer to physical
replication, and I thought it would be clearer to use something
different, so I did. It's confusing if we use the same word for the
server from which others replicate, the table from which others
inherit, the process which initiates parallelism, and the first
process that is launched across the whole cluster, regardless of
*which* word we use for those things. So, I think there is every
possibility that with careful thought, we can actually make things
clearer, in addition to avoiding the use of terms that are no longer
welcome.

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

#33

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Robert Haas (#32)

Re: language cleanups in code and docs

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jun 15, 2020 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:

0002: code: s/master/primary/
0003: code: s/master/leader/
0006: docs: s/master/root/
0007: docs: s/master/supervisor/

I'd just like to make the pointer here that there's value in trying to
use different terminology for different things.

+1 for that.

regards, tom lane

#34

andres@anarazel.de

over 5 years ago

In reply to: Robert Haas (#32)

Re: language cleanups in code and docs

Hi,

On 2020-06-17 13:59:26 -0400, Robert Haas wrote:

On Mon, Jun 15, 2020 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:

0002: code: s/master/primary/
0003: code: s/master/leader/
0006: docs: s/master/root/
0007: docs: s/master/supervisor/

I'd just like to make the pointer here that there's value in trying to
use different terminology for different things. I picked "leader" and
"worker" for parallel query and tried to use them consistently because
"master" and "slave" were being used widely to refer to physical
replication, and I thought it would be clearer to use something
different, so I did.

Just to be clear, that's exactly what I tried to do in the above
patches. E.g. in 0003 I tried to follow the scheme you just
outlined. There's a part of that patch that addresses pg_dump, but most
of the rest is just parallelism related pieces that ended up using
master, even though leader is the more widely used term. I assume you
were just saying that the above use of different terms is actually
helpful:

It's confusing if we use the same word for the server from which
others replicate, the table from which others inherit, the process
which initiates parallelism, and the first process that is launched
across the whole cluster, regardless of *which* word we use for those
things. So, I think there is every possibility that with careful
thought, we can actually make things clearer, in addition to avoiding
the use of terms that are no longer welcome.

With which I wholeheartedly agree.

Greetings,

Andres Freund

#35

Bruce Momjian

bruce@momjian.us

over 5 years ago

In reply to: Robert Haas (#32)

Re: language cleanups in code and docs

On Wed, Jun 17, 2020 at 01:59:26PM -0400, Robert Haas wrote:

On Mon, Jun 15, 2020 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:

0002: code: s/master/primary/
0003: code: s/master/leader/
0006: docs: s/master/root/
0007: docs: s/master/supervisor/

I'd just like to make the pointer here that there's value in trying to
use different terminology for different things. I picked "leader" and
"worker" for parallel query and tried to use them consistently because
"master" and "slave" were being used widely to refer to physical
replication, and I thought it would be clearer to use something
different, so I did. It's confusing if we use the same word for the
server from which others replicate, the table from which others
inherit, the process which initiates parallelism, and the first
process that is launched across the whole cluster, regardless of
*which* word we use for those things. So, I think there is every
possibility that with careful thought, we can actually make things
clearer, in addition to avoiding the use of terms that are no longer
welcome.

I think the question is whether we can improve our terms as part of this
rewording, or if we make them worse. When we got rid of slave and made
it standby, I think we made things worse since many of the replicas were
not functioning for the purpose of standby. Standby is a role, not a
status, while replica is a status.

The other issue is how the terms interlink with other terms. When we
used master/slave, multi-master matched the wording, but replication
didn't match. If we go with replica, replication works, and
primary/replica kind of fits, e.g., master/replica does not.
Multi-master then no longer fits, multi-primary sounds odd, and
active-active doesn't match, though active-active is not used as much as
primary/replica, so maybe that is OK. Ideally we would have all terms
matching, but maybe that is impossible.

My point is that these terms are symbolic (similes) --- the new terms
should link to their roles and to other terms in a logical way.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

#36

andres@anarazel.de

over 5 years ago

In reply to: David Steele (#15)

1 attachment(s)

Re: language cleanups in code and docs

Hi,

I've pushed most of the changes.

On 2020-06-16 18:59:25 -0400, David Steele wrote:

On 6/16/20 6:27 PM, Andres Freund wrote:

On 2020-06-16 17:14:57 -0400, David Steele wrote:

On 6/15/20 2:22 PM, Andres Freund wrote:

0008: docs: WIP multi-master rephrasing.
I like neither the new nor the old language much. I'd welcome input.

Why not multi-primary?

My understanding of primary is that there really can't be two things
that are primary in relation to each other.

Well, I think the same is true for multi-master and that's pretty common.

active/active is probably
the most common term in use besides multi-master.

Works for me and can always be updated later if we come up with something
better. At least active-active will be easier to search for.

What do you think about the attached?

Greetings,

Andres Freund

Attachments:

0001-docs-use-active-active-instead-of-multi-master-repli.patchtext/x-diff; charset=us-asciiDownload

From 254c1f45beb8e9dee840a497d5c040b00015a8f3 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 15 Jun 2020 10:39:15 -0700
Subject: [PATCH] docs: use active-active instead of multi-master replication.

Author: Andres Freund
Reviewed-By: David Steele
Discussion: https://postgr.es/m/20200615182235.x7lch5n6kcjq4aue@alap3.anarazel.de
---
 doc/src/sgml/high-availability.sgml | 34 +++++++++++++++--------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 6a9184f314e..2e090419f18 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -38,12 +38,14 @@
  <para>
   Some solutions deal with synchronization by allowing only one
   server to modify the data.  Servers that can modify data are
-  called read/write, <firstterm>master</firstterm> or <firstterm>primary</firstterm> servers.
-  Servers that track changes in the master are called <firstterm>standby</firstterm>
-  or <firstterm>secondary</firstterm> servers. A standby server that cannot be connected
-  to until it is promoted to a master server is called a <firstterm>warm
-  standby</firstterm> server, and one that can accept connections and serves read-only
-  queries is called a <firstterm>hot standby</firstterm> server.
+  called read/write, <firstterm>primary</firstterm>,
+  <firstterm>active</firstterm> (or <firstterm>master</firstterm>) servers.
+  Servers that track changes in the primary are called
+  <firstterm>standby</firstterm> or <firstterm>secondary</firstterm> servers.
+  A standby server that cannot be connected to until it is promoted to a
+  primary server is called a <firstterm>warm standby</firstterm> server, and
+  one that can accept connections and serves read-only queries is called a
+  <firstterm>hot standby</firstterm> server.
  </para>
 
  <para>
@@ -177,14 +179,14 @@ protocol to make nodes agree on a serializable transactional order.
   </varlistentry>
 
   <varlistentry>
-   <term>Trigger-Based Master-Standby Replication</term>
+   <term>Trigger-Based Primary-Standby Replication</term>
    <listitem>
 
     <para>
-     A master-standby replication setup sends all data modification
-     queries to the master server.  The master server asynchronously
+     A primary-standby replication setup sends all data modification
+     queries to the primary server.  The primary server asynchronously
      sends data changes to the standby server.  The standby can answer
-     read-only queries while the master server is running.  The
+     read-only queries while the primary server is running.  The
      standby server is ideal for data warehouse queries.
     </para>
 
@@ -233,14 +235,14 @@ protocol to make nodes agree on a serializable transactional order.
   </varlistentry>
 
   <varlistentry>
-   <term>Asynchronous Multimaster Replication</term>
+   <term>Asynchronous Active-Active Replication</term>
    <listitem>
 
     <para>
      For servers that are not regularly connected or have slow
      communication links, like laptops or
      remote servers, keeping data consistent among servers is a
-     challenge.  Using asynchronous multimaster replication, each
+     challenge.  Using asynchronous active-active replication, each
      server works independently, and periodically communicates with
      the other servers to identify conflicting transactions.  The
      conflicts can be resolved by users or conflict resolution rules.
@@ -250,17 +252,17 @@ protocol to make nodes agree on a serializable transactional order.
   </varlistentry>
 
   <varlistentry>
-   <term>Synchronous Multimaster Replication</term>
+   <term>Synchronous Active-Active Replication</term>
    <listitem>
 
     <para>
-     In synchronous multimaster replication, each server can accept
+     In synchronous active-active replication, each server can accept
      write requests, and modified data is transmitted from the
      original server to every other server before each transaction
      commits.  Heavy write activity can cause excessive locking and
      commit delays, leading to poor performance.  Read requests can
      be sent to any server.  Some implementations use shared disk
-     to reduce the communication overhead.  Synchronous multimaster
+     to reduce the communication overhead.  Synchronous active-active
      replication is best for mostly read workloads, though its big
      advantage is that any server can accept write requests &mdash;
      there is no need to partition workloads between primary and
@@ -351,7 +353,7 @@ protocol to make nodes agree on a serializable transactional order.
     </row>
 
     <row>
-     <entry>Allows multiple master servers</entry>
+     <entry>Allows multiple primary servers</entry>
      <entry align="center"></entry>
      <entry align="center"></entry>
      <entry align="center"></entry>
-- 
2.25.0.114.g5b0ca878e0

#37

david@pgmasters.net

over 5 years ago

In reply to: Andres Freund (#36)

Re: language cleanups in code and docs

On 7/8/20 4:39 PM, Andres Freund wrote:

Hi,

I've pushed most of the changes.

On 2020-06-16 18:59:25 -0400, David Steele wrote:

On 6/16/20 6:27 PM, Andres Freund wrote:

On 2020-06-16 17:14:57 -0400, David Steele wrote:

On 6/15/20 2:22 PM, Andres Freund wrote:

0008: docs: WIP multi-master rephrasing.
I like neither the new nor the old language much. I'd welcome input.

Why not multi-primary?

My understanding of primary is that there really can't be two things
that are primary in relation to each other.

Well, I think the same is true for multi-master and that's pretty common.

active/active is probably
the most common term in use besides multi-master.

Works for me and can always be updated later if we come up with something
better. At least active-active will be easier to search for.

What do you think about the attached?

I think this phrasing in the original/updated version is pretty awkward:

+  A standby server that cannot be connected to until it is promoted to a
+  primary server is called a ...

How about:

+ A standby server that must be promoted to a primary server before
+ accepting connections is called a ...

Other than that it looks good to me.

Regards,
--
-David
david@pgmasters.net

#38

Alvaro Herrera

alvherre@2ndquadrant.com

over 5 years ago

In reply to: David Steele (#37)

Re: language cleanups in code and docs

On 2020-Jul-08, David Steele wrote:

On 7/8/20 4:39 PM, Andres Freund wrote:

I think this phrasing in the original/updated version is pretty awkward:
+  A standby server that cannot be connected to until it is promoted to a
+  primary server is called a ...

Yeah.

How about:

+ A standby server that must be promoted to a primary server before
+ accepting connections is called a ...

How about just reducing it to "A standby server that doesn't accept
connection is called a ..."? We don't really need to explain that if
you do promote the standby it will start accept connections -- do we?
It should be pretty obvious if you promote a standby, it will cease to
be a standby in the first place. This verbiage seems excessive.

--
ï¿½lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#39

david@pgmasters.net

over 5 years ago

In reply to: Alvaro Herrera (#38)

Re: language cleanups in code and docs

On 7/8/20 5:17 PM, Alvaro Herrera wrote:

On 2020-Jul-08, David Steele wrote:

On 7/8/20 4:39 PM, Andres Freund wrote:
I think this phrasing in the original/updated version is pretty awkward:
+  A standby server that cannot be connected to until it is promoted to a
+  primary server is called a ...
Yeah.
How about:
+ A standby server that must be promoted to a primary server before
+ accepting connections is called a ...
How about just reducing it to "A standby server that doesn't accept
connection is called a ..."? We don't really need to explain that if
you do promote the standby it will start accept connections -- do we?
It should be pretty obvious if you promote a standby, it will cease to
be a standby in the first place. This verbiage seems excessive.

Works for me.

Regards,
--
-David
david@pgmasters.net

#40

Ashwin Agrawal

ashwinstar@gmail.com

over 5 years ago

In reply to: David Steele (#31)

Re: language cleanups in code and docs

On Wed, Jun 17, 2020 at 9:27 AM David Steele <david@pgmasters.net> wrote:

On 6/17/20 12:08 PM, Magnus Hagander wrote:

On Wed, Jun 17, 2020 at 4:15 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com <mailto:andrew.dunstan@2ndquadrant.com>>

I'm not sure I like doing s/Black/Block/ here. It reads oddly. There

are

too many other uses of Block in the sources. Forbidden might be a

better

substitution, or Banned maybe. BanList is even less characters than
BlackList.

I'd be OK with either of those really -- I went with block because it
was the easiest one :)

Not sure the number of characters is the important part :) Banlist does
make sense to me for other reasons though -- it's what it is, isn't it?
It bans those oids from being used in the current session -- I don't
think there's any struggle to "make that sentence work", which means
that seems like the relevant term.

I've seen also seen allowList/denyList as an alternative. I do agree
that blockList is a bit confusing since we often use block in a very
different context.

+1 for allowList/denyList as alternative

I do think it's worth doing -- it's a small round of changes, and it

doesn't change anything user-exposed, so the cost for us is basically

zero.

+1

Agree number of occurrences for whitelist and blacklist are not many, so
cleaning these would be helpful and patches already proposed for it

git grep whitelist | wc -l
10
git grep blacklist | wc -l
40

Thanks a lot for language cleanups. Greenplum, fork of PostgreSQL, wishes
to perform similar cleanups and upstream doing it really helps us
downstream.

#41

thomas.munro@gmail.com

about 5 years ago

In reply to: Magnus Hagander (#23)

1 attachment(s)

Re: language cleanups in code and docs

On Wed, Jun 17, 2020 at 10:32 PM Magnus Hagander <magnus@hagander.net> wrote:

In looking at this I realize we also have exactly one thing referred to as "blacklist" in our codebase, which is the "enum blacklist" (and then a small internal variable in pgindent). AFAICT, it's not actually exposed to userspace anywhere, so we could probably make the attached change to blocklist at no "cost" (the only thing changed is the name of the hash table, and we definitely change things like that in normal releases with no specific thought on backwards compat).

Hmm, can we find a more descriptive name for this mechanism? What
about calling it the "uncommitted enum table"? See attached.

Attachments:

0001-Rename-enum-blacklist-to-uncommitted-enum-table.patchtext/x-patch; charset=US-ASCII; name=0001-Rename-enum-blacklist-to-uncommitted-enum-table.patchDownload

From 39dfbc83691ee9f48ea844172e38231740674aed Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Thu, 22 Oct 2020 10:09:55 +1300
Subject: [PATCH] Rename "enum blacklist" to "uncommitted enum table".

Internal data structures relating to uncommitted enum values are better
described that way, and the earlier term is now unpopular in the
software industry for its (unintended) connotations.

Discussion: https://postgr.es/m/20200615182235.x7lch5n6kcjq4aue%40alap3.anarazel.de
---
 src/backend/access/transam/parallel.c | 32 ++++++-------
 src/backend/catalog/pg_enum.c         | 65 ++++++++++++++-------------
 src/backend/utils/adt/enum.c          |  4 +-
 src/include/catalog/pg_enum.h         |  8 ++--
 4 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index b0426960c7..1f499b5131 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -75,7 +75,7 @@
 #define PARALLEL_KEY_PENDING_SYNCS			UINT64CONST(0xFFFFFFFFFFFF000B)
 #define PARALLEL_KEY_REINDEX_STATE			UINT64CONST(0xFFFFFFFFFFFF000C)
 #define PARALLEL_KEY_RELMAPPER_STATE		UINT64CONST(0xFFFFFFFFFFFF000D)
-#define PARALLEL_KEY_ENUMBLACKLIST			UINT64CONST(0xFFFFFFFFFFFF000E)
+#define PARALLEL_KEY_UNCOMMITTEDENUMTABLE	UINT64CONST(0xFFFFFFFFFFFF000E)
 
 /* Fixed-size parallel state. */
 typedef struct FixedParallelState
@@ -211,7 +211,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	Size		pendingsyncslen = 0;
 	Size		reindexlen = 0;
 	Size		relmapperlen = 0;
-	Size		enumblacklistlen = 0;
+	Size		uncommittedenumtablelen = 0;
 	Size		segsize = 0;
 	int			i;
 	FixedParallelState *fps;
@@ -267,8 +267,8 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		shm_toc_estimate_chunk(&pcxt->estimator, reindexlen);
 		relmapperlen = EstimateRelationMapSpace();
 		shm_toc_estimate_chunk(&pcxt->estimator, relmapperlen);
-		enumblacklistlen = EstimateEnumBlacklistSpace();
-		shm_toc_estimate_chunk(&pcxt->estimator, enumblacklistlen);
+		uncommittedenumtablelen = EstimateUncommittedEnumTableSpace();
+		shm_toc_estimate_chunk(&pcxt->estimator, uncommittedenumtablelen);
 		/* If you add more chunks here, you probably need to add keys. */
 		shm_toc_estimate_keys(&pcxt->estimator, 11);
 
@@ -348,7 +348,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		char	   *error_queue_space;
 		char	   *session_dsm_handle_space;
 		char	   *entrypointstate;
-		char	   *enumblacklistspace;
+		char	   *uncommittedenumtablespace;
 		Size		lnamelen;
 
 		/* Serialize shared libraries we have loaded. */
@@ -404,11 +404,13 @@ InitializeParallelDSM(ParallelContext *pcxt)
 		shm_toc_insert(pcxt->toc, PARALLEL_KEY_RELMAPPER_STATE,
 					   relmapperspace);
 
-		/* Serialize enum blacklist state. */
-		enumblacklistspace = shm_toc_allocate(pcxt->toc, enumblacklistlen);
-		SerializeEnumBlacklist(enumblacklistspace, enumblacklistlen);
-		shm_toc_insert(pcxt->toc, PARALLEL_KEY_ENUMBLACKLIST,
-					   enumblacklistspace);
+		/* Serialize uncommitted enum state. */
+		uncommittedenumtablespace = shm_toc_allocate(pcxt->toc,
+													 uncommittedenumtablelen);
+		SerializeUncommittedEnumTable(uncommittedenumtablespace,
+									  uncommittedenumtablelen);
+		shm_toc_insert(pcxt->toc, PARALLEL_KEY_UNCOMMITTEDENUMTABLE,
+					   uncommittedenumtablespace);
 
 		/* Allocate space for worker information. */
 		pcxt->worker = palloc0(sizeof(ParallelWorkerInfo) * pcxt->nworkers);
@@ -1257,7 +1259,7 @@ ParallelWorkerMain(Datum main_arg)
 	char	   *pendingsyncsspace;
 	char	   *reindexspace;
 	char	   *relmapperspace;
-	char	   *enumblacklistspace;
+	char	   *uncommittedenumtablespace;
 	StringInfoData msgbuf;
 	char	   *session_dsm_handle_space;
 
@@ -1449,10 +1451,10 @@ ParallelWorkerMain(Datum main_arg)
 	relmapperspace = shm_toc_lookup(toc, PARALLEL_KEY_RELMAPPER_STATE, false);
 	RestoreRelationMap(relmapperspace);
 
-	/* Restore enum blacklist. */
-	enumblacklistspace = shm_toc_lookup(toc, PARALLEL_KEY_ENUMBLACKLIST,
-										false);
-	RestoreEnumBlacklist(enumblacklistspace);
+	/* Restore uncommitted enum table. */
+	uncommittedenumtablespace =
+		shm_toc_lookup(toc,PARALLEL_KEY_UNCOMMITTEDENUMTABLE, false);
+	RestoreUncommittedEnumTable(uncommittedenumtablespace);
 
 	/* Attach to the leader's serializable transaction, if SERIALIZABLE. */
 	AttachSerializableXact(fps->serializable_xact_handle);
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index 27e4100a6f..749b9dbee1 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -41,10 +41,11 @@ Oid			binary_upgrade_next_pg_enum_oid = InvalidOid;
  * committed; otherwise, they might get into indexes where we can't clean
  * them up, and then if the transaction rolls back we have a broken index.
  * (See comments for check_safe_enum_use() in enum.c.)  Values created by
- * EnumValuesCreate are *not* blacklisted; we assume those are created during
- * CREATE TYPE, so they can't go away unless the enum type itself does.
+ * EnumValuesCreate are *not* entered into the table; we assume those are
+ * created during CREATE TYPE, so they can't go away unless the enum type
+ * itself does.
  */
-static HTAB *enum_blacklist = NULL;
+static HTAB *uncommitted_enum_table = NULL;
 
 static void RenumberEnumType(Relation pg_enum, HeapTuple *existing, int nelems);
 static int	sort_order_cmp(const void *p1, const void *p2);
@@ -181,10 +182,10 @@ EnumValuesDelete(Oid enumTypeOid)
 }
 
 /*
- * Initialize the enum blacklist for this transaction.
+ * Initialize the uncommitted enum table for this transaction.
  */
 static void
-init_enum_blacklist(void)
+init_uncommitted_enum_table(void)
 {
 	HASHCTL		hash_ctl;
 
@@ -192,10 +193,10 @@ init_enum_blacklist(void)
 	hash_ctl.keysize = sizeof(Oid);
 	hash_ctl.entrysize = sizeof(Oid);
 	hash_ctl.hcxt = TopTransactionContext;
-	enum_blacklist = hash_create("Enum value blacklist",
-								 32,
-								 &hash_ctl,
-								 HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
+	uncommitted_enum_table = hash_create("Uncommitted enum table",
+										 32,
+										 &hash_ctl,
+										 HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 }
 
 /*
@@ -491,12 +492,12 @@ restart:
 
 	table_close(pg_enum, RowExclusiveLock);
 
-	/* Set up the blacklist hash if not already done in this transaction */
-	if (enum_blacklist == NULL)
-		init_enum_blacklist();
+	/* Set up the enum table if not already done in this transaction */
+	if (uncommitted_enum_table == NULL)
+		init_uncommitted_enum_table();
 
-	/* Add the new value to the blacklist */
-	(void) hash_search(enum_blacklist, &newOid, HASH_ENTER, NULL);
+	/* Add the new value to the table */
+	(void) hash_search(uncommitted_enum_table, &newOid, HASH_ENTER, NULL);
 }
 
 
@@ -585,19 +586,19 @@ RenameEnumLabel(Oid enumTypeOid,
 
 
 /*
- * Test if the given enum value is on the blacklist
+ * Test if the given enum value is in the table of blocked enums.
  */
 bool
-EnumBlacklisted(Oid enum_id)
+EnumUncommitted(Oid enum_id)
 {
 	bool		found;
 
-	/* If we've made no blacklist table, all values are safe */
-	if (enum_blacklist == NULL)
+	/* If we've made no uncommitted table, all values are safe */
+	if (uncommitted_enum_table == NULL)
 		return false;
 
 	/* Else, is it in the table? */
-	(void) hash_search(enum_blacklist, &enum_id, HASH_FIND, &found);
+	(void) hash_search(uncommitted_enum_table, &enum_id, HASH_FIND, &found);
 	return found;
 }
 
@@ -609,11 +610,11 @@ void
 AtEOXact_Enum(void)
 {
 	/*
-	 * Reset the blacklist table, as all our enum values are now committed.
+	 * Reset the uncommitted table, as all our enum values are now committed.
 	 * The memory will go away automatically when TopTransactionContext is
 	 * freed; it's sufficient to clear our pointer.
 	 */
-	enum_blacklist = NULL;
+	uncommitted_enum_table = NULL;
 }
 
 
@@ -692,12 +693,12 @@ sort_order_cmp(const void *p1, const void *p2)
 }
 
 Size
-EstimateEnumBlacklistSpace(void)
+EstimateUncommittedEnumTableSpace(void)
 {
 	size_t		entries;
 
-	if (enum_blacklist)
-		entries = hash_get_num_entries(enum_blacklist);
+	if (uncommitted_enum_table)
+		entries = hash_get_num_entries(uncommitted_enum_table);
 	else
 		entries = 0;
 
@@ -706,7 +707,7 @@ EstimateEnumBlacklistSpace(void)
 }
 
 void
-SerializeEnumBlacklist(void *space, Size size)
+SerializeUncommittedEnumTable(void *space, Size size)
 {
 	Oid		   *serialized = (Oid *) space;
 
@@ -714,15 +715,15 @@ SerializeEnumBlacklist(void *space, Size size)
 	 * Make sure the hash table hasn't changed in size since the caller
 	 * reserved the space.
 	 */
-	Assert(size == EstimateEnumBlacklistSpace());
+	Assert(size == EstimateUncommittedEnumTableSpace());
 
 	/* Write out all the values from the hash table, if there is one. */
-	if (enum_blacklist)
+	if (uncommitted_enum_table)
 	{
 		HASH_SEQ_STATUS status;
 		Oid		   *value;
 
-		hash_seq_init(&status, enum_blacklist);
+		hash_seq_init(&status, uncommitted_enum_table);
 		while ((value = (Oid *) hash_seq_search(&status)))
 			*serialized++ = *value;
 	}
@@ -738,11 +739,11 @@ SerializeEnumBlacklist(void *space, Size size)
 }
 
 void
-RestoreEnumBlacklist(void *space)
+RestoreUncommittedEnumTable(void *space)
 {
 	Oid		   *serialized = (Oid *) space;
 
-	Assert(!enum_blacklist);
+	Assert(!uncommitted_enum_table);
 
 	/*
 	 * As a special case, if the list is empty then don't even bother to
@@ -753,9 +754,9 @@ RestoreEnumBlacklist(void *space)
 		return;
 
 	/* Read all the values into a new hash table. */
-	init_enum_blacklist();
+	init_uncommitted_enum_table();
 	do
 	{
-		hash_search(enum_blacklist, serialized++, HASH_ENTER, NULL);
+		hash_search(uncommitted_enum_table, serialized++, HASH_ENTER, NULL);
 	} while (OidIsValid(*serialized));
 }
diff --git a/src/backend/utils/adt/enum.c b/src/backend/utils/adt/enum.c
index 5ead794e34..ed7da7e007 100644
--- a/src/backend/utils/adt/enum.c
+++ b/src/backend/utils/adt/enum.c
@@ -83,12 +83,12 @@ check_safe_enum_use(HeapTuple enumval_tup)
 		return;
 
 	/*
-	 * Check if the enum value is blacklisted.  If not, it's safe, because it
+	 * Check if the enum value is uncommitted.  If not, it's safe, because it
 	 * was made during CREATE TYPE AS ENUM and can't be shorter-lived than its
 	 * owning type.  (This'd also be false for values made by other
 	 * transactions; but the previous tests should have handled all of those.)
 	 */
-	if (!EnumBlacklisted(en->oid))
+	if (!EnumUncommitted(en->oid))
 		return;
 
 	/*
diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h
index b28d441ba7..fd90365f45 100644
--- a/src/include/catalog/pg_enum.h
+++ b/src/include/catalog/pg_enum.h
@@ -53,10 +53,10 @@ extern void AddEnumLabel(Oid enumTypeOid, const char *newVal,
 						 bool skipIfExists);
 extern void RenameEnumLabel(Oid enumTypeOid,
 							const char *oldVal, const char *newVal);
-extern bool EnumBlacklisted(Oid enum_id);
-extern Size EstimateEnumBlacklistSpace(void);
-extern void SerializeEnumBlacklist(void *space, Size size);
-extern void RestoreEnumBlacklist(void *space);
+extern bool EnumUncommitted(Oid enum_id);
+extern Size EstimateUncommittedEnumTableSpace(void);
+extern void SerializeUncommittedEnumTable(void *space, Size size);
+extern void RestoreUncommittedEnumTable(void *space);
 extern void AtEOXact_Enum(void);
 
 #endif							/* PG_ENUM_H */
-- 
2.20.1

#42

magnus@hagander.net

about 5 years ago

In reply to: Thomas Munro (#41)

Re: language cleanups in code and docs

On Wed, Oct 21, 2020 at 11:23 PM Thomas Munro <thomas.munro@gmail.com> wrote:

On Wed, Jun 17, 2020 at 10:32 PM Magnus Hagander <magnus@hagander.net> wrote:

In looking at this I realize we also have exactly one thing referred to as "blacklist" in our codebase, which is the "enum blacklist" (and then a small internal variable in pgindent). AFAICT, it's not actually exposed to userspace anywhere, so we could probably make the attached change to blocklist at no "cost" (the only thing changed is the name of the hash table, and we definitely change things like that in normal releases with no specific thought on backwards compat).

+1

Hmm, can we find a more descriptive name for this mechanism? What
about calling it the "uncommitted enum table"? See attached.

Thanks for picking this one up again!

Agreed, that's a much better choice.

The term itself is a bit of a mouthful, but it does describe what it
is in a much more clear way than what the old term did anyway.

Maybe consider just calling it "uncomitted enums", which would as a
bonus have it not end up talking about uncommittedenumtablespace which
gets hits on searches for tablespace.. Though I'm not sure that's
important.

I'm +1 to the change with or without that adjustment.

As for the code, I note that:
+ /* Set up the enum table if not already done in this transaction */

forgets to say it's *uncomitted* enum table -- which is the important
part of I believe.

And
+ * Test if the given enum value is in the table of blocked enums.

should probably talk about uncommitted rather than blocked?

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

#43

thomas.munro@gmail.com

about 5 years ago

In reply to: Magnus Hagander (#42)

Re: language cleanups in code and docs

On Wed, Nov 4, 2020 at 4:10 AM Magnus Hagander <magnus@hagander.net> wrote:

On Wed, Oct 21, 2020 at 11:23 PM Thomas Munro <thomas.munro@gmail.com> wrote:

Hmm, can we find a more descriptive name for this mechanism? What
about calling it the "uncommitted enum table"? See attached.

Thanks for picking this one up again!

Agreed, that's a much better choice.

The term itself is a bit of a mouthful, but it does describe what it
is in a much more clear way than what the old term did anyway.

Maybe consider just calling it "uncomitted enums", which would as a
bonus have it not end up talking about uncommittedenumtablespace which
gets hits on searches for tablespace.. Though I'm not sure that's
important.

I'm +1 to the change with or without that adjustment.

Cool. I went with your suggestion.

As for the code, I note that:
+ /* Set up the enum table if not already done in this transaction */

forgets to say it's *uncomitted* enum table -- which is the important
part of I believe.

Fixed.

And
+ * Test if the given enum value is in the table of blocked enums.

should probably talk about uncommitted rather than blocked?

Fixed.

And pushed.

#44

Noname

ilmari@ilmari.org

about 5 years ago

In reply to: Magnus Hagander (#23)

1 attachment(s)

Re: language cleanups in code and docs

Magnus Hagander <magnus@hagander.net> writes:

In looking at this I realize we also have exactly one thing referred to as
"blacklist" in our codebase, which is the "enum blacklist" (and then a
small internal variable in pgindent).

Here's a patch that renames the @whitelist and %blacklist variables in
pgindent to @additional and %excluded, and adjusts the comments to
match.

- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law

Attachments:

0001-Rename-whitelist-blacklist-in-pgindent-to-additional.patchtext/x-diffDownload

From 6525826bdf87ce02bd0a1648f94c7122290907f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 5 Jan 2021 00:10:07 +0000
Subject: [PATCH] Rename whitelist/blacklist in pgindent to additional/excluded

---
 src/tools/pgindent/pgindent | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 4124d27dea..feb02067c5 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -54,12 +54,12 @@ $excludes ||= "$code_base/src/tools/pgindent/exclude_file_patterns"
 # some names we want to treat like typedefs, e.g. "bool" (which is a macro
 # according to <stdbool.h>), and may include some names we don't want
 # treated as typedefs, although various headers that some builds include
-# might make them so.  For the moment we just hardwire a whitelist of names
-# to add and a blacklist of names to remove; eventually this may need to be
+# might make them so.  For the moment we just hardwire a list of names
+# to add and a list of names to exclude; eventually this may need to be
 # easier to configure.  Note that the typedefs need trailing newlines.
-my @whitelist = ("bool\n");
+my @additional = ("bool\n");
 
-my %blacklist = map { +"$_\n" => 1 } qw(
+my %excluded = map { +"$_\n" => 1 } qw(
   ANY FD_SET U abs allocfunc boolean date digit ilist interval iterator other
   pointer printfunc reference string timestamp type wrap
 );
@@ -134,11 +134,11 @@ sub load_typedefs
 		}
 	}
 
-	# add whitelisted entries
-	push(@typedefs, @whitelist);
+	# add additional entries
+	push(@typedefs, @additional);
 
-	# remove blacklisted entries
-	@typedefs = grep { !$blacklist{$_} } @typedefs;
+	# remove excluded entries
+	@typedefs = grep { !$excluded{$_} } @typedefs;
 
 	# write filtered typedefs
 	my $filter_typedefs_fh = new File::Temp(TEMPLATE => "pgtypedefXXXXX");
-- 
2.29.2

#45

thomas.munro@gmail.com

about 5 years ago

In reply to: Noname (#44)

Re: language cleanups in code and docs

On Tue, Jan 5, 2021 at 1:12 PM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:

Magnus Hagander <magnus@hagander.net> writes:

In looking at this I realize we also have exactly one thing referred to as
"blacklist" in our codebase, which is the "enum blacklist" (and then a
small internal variable in pgindent).

Here's a patch that renames the @whitelist and %blacklist variables in
pgindent to @additional and %excluded, and adjusts the comments to
match.

Pushed. Thanks!

#46

Noname

ilmari@ilmari.org

about 5 years ago

In reply to: Thomas Munro (#45)

1 attachment(s)

Re: language cleanups in code and docs

Thomas Munro <thomas.munro@gmail.com> writes:

On Tue, Jan 5, 2021 at 1:12 PM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:

Magnus Hagander <magnus@hagander.net> writes:

In looking at this I realize we also have exactly one thing referred to as
"blacklist" in our codebase, which is the "enum blacklist" (and then a
small internal variable in pgindent).

Here's a patch that renames the @whitelist and %blacklist variables in
pgindent to @additional and %excluded, and adjusts the comments to
match.

Pushed. Thanks!

Thanks! Just after sending that, I thought to grep for "white\W*list"
as well, and found a few more occurrences that were trivially reworded,
per the attached patch.

- ilmari
--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

Attachments:

0001-Replace-remaining-uses-of-whitelist.patchtext/x-diffDownload

From 43e9c60bac7b1702e5be2362a439f67adc8a5e06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 5 Jan 2021 00:20:49 +0000
Subject: [PATCH] Replace remaining uses of "whitelist"

Instead describe the action that the list effects, or just use "list"
where the meaning is obvious from context.
---
 contrib/postgres_fdw/postgres_fdw.h    | 2 +-
 contrib/postgres_fdw/shippable.c       | 4 ++--
 src/backend/access/hash/hashvalidate.c | 2 +-
 src/backend/utils/adt/lockfuncs.c      | 2 +-
 src/tools/pginclude/README             | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 277a30f500..19ea27a1bc 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -77,7 +77,7 @@ typedef struct PgFdwRelationInfo
 	bool		use_remote_estimate;
 	Cost		fdw_startup_cost;
 	Cost		fdw_tuple_cost;
-	List	   *shippable_extensions;	/* OIDs of whitelisted extensions */
+	List	   *shippable_extensions;	/* OIDs of shippable extensions */
 
 	/* Cached catalog information. */
 	ForeignTable *table;
diff --git a/contrib/postgres_fdw/shippable.c b/contrib/postgres_fdw/shippable.c
index c43e7e5ec5..b27f82e015 100644
--- a/contrib/postgres_fdw/shippable.c
+++ b/contrib/postgres_fdw/shippable.c
@@ -7,7 +7,7 @@
  * data types are shippable to a remote server for execution --- that is,
  * do they exist and have the same behavior remotely as they do locally?
  * Built-in objects are generally considered shippable.  Other objects can
- * be shipped if they are white-listed by the user.
+ * be shipped if they are declared as such by the user.
  *
  * Note: there are additional filter rules that prevent shipping mutable
  * functions or functions using nonportable collations.  Those considerations
@@ -110,7 +110,7 @@ InitializeShippableCache(void)
  *
  * Right now "shippability" is exclusively a function of whether the object
  * belongs to an extension declared by the user.  In the future we could
- * additionally have a whitelist of functions/operators declared one at a time.
+ * additionally have a list of functions/operators declared one at a time.
  */
 static bool
 lookup_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo)
diff --git a/src/backend/access/hash/hashvalidate.c b/src/backend/access/hash/hashvalidate.c
index 8462540017..1e343df0af 100644
--- a/src/backend/access/hash/hashvalidate.c
+++ b/src/backend/access/hash/hashvalidate.c
@@ -312,7 +312,7 @@ check_hash_func_signature(Oid funcid, int16 amprocnum, Oid argtype)
 		 * that are different from but physically compatible with the opclass
 		 * datatype.  In some of these cases, even a "binary coercible" check
 		 * fails because there's no relevant cast.  For the moment, fix it by
-		 * having a whitelist of allowed cases.  Test the specific function
+		 * having a list of allowed cases.  Test the specific function
 		 * identity, not just its input type, because hashvarlena() takes
 		 * INTERNAL and allowing any such function seems too scary.
 		 */
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index 9f2c4946c9..0db8be6c91 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -644,7 +644,7 @@ pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
 	/*
 	 * Check if blocked_pid is waiting for a safe snapshot.  We could in
 	 * theory check the resulting array of blocker PIDs against the
-	 * interesting PIDs whitelist, but since there is no danger of autovacuum
+	 * interesting PIDs list, but since there is no danger of autovacuum
 	 * blocking GetSafeSnapshot there seems to be no point in expending cycles
 	 * on allocating a buffer and searching for overlap; so it's presently
 	 * sufficient for the isolation tester's purposes to use a single element
diff --git a/src/tools/pginclude/README b/src/tools/pginclude/README
index a067c7f472..49eb4b6907 100644
--- a/src/tools/pginclude/README
+++ b/src/tools/pginclude/README
@@ -64,7 +64,7 @@ with no prerequisite headers other than postgres.h (or postgres_fe.h
 or c.h, as appropriate).
 
 A small number of header files are exempted from this requirement,
-and are whitelisted in the headerscheck script.
+and are skipped by the headerscheck script.
 
 The easy way to run the script is to say "make -s headerscheck" in
 the top-level build directory after completing a build.  You should
@@ -86,7 +86,7 @@ the project's coding language is C, some people write extensions in C++,
 so it's helpful for include files to be C++-clean.
 
 A small number of header files are exempted from this requirement,
-and are whitelisted in the cpluspluscheck script.
+and are skipped by the cpluspluscheck script.
 
 The easy way to run the script is to say "make -s cpluspluscheck" in
 the top-level build directory after completing a build.  You should
-- 
2.29.2

#47