[PATCH] Implement motd for PostgreSQL

Started by Joel Jacobsonalmost 5 years ago23 messages
#1Joel Jacobson
joel@compiler.org
2 attachment(s)

Dear fellow hackers,

This patch is one day late, my apologies for missing the deadline this year.

PostgreSQL has since long been suffering from the lack of a proper UNIX style motd (message of the day).

DBAs have no ways of conveying important information to users,
having to rely on external protocols, such as HTTPS and "websites" to provide such information.

By adding a motd configuration parameter, the DBA can set this to a text string,
which will be automatically presented to the user as a NOTICE when logging on to the server.

While at it, fix escape_single_quotes_ascii() to properly escape newlines,
so that such can be used in ALTER SYSTEM values.
This makes sense, since parsing \n in config values works just fine.

To demonstrate the usefulness of this feature,
I've setup an open public PostgreSQL server at "pit.org",
to which anyone can connect without a password.

You need to know the username though,
which will hopefully make problems for bots.

$ psql -U brad -h pit.org motd
NOTICE:
____ ______ ___
/ )/ /
( / __ _ )
(/ o) ( o) )
_ (_ ) ) /
/_/ )_/
/ //| |\
v | | v
__/

This was accomplished by setting the "motd",
which requires superuser privileges:

$ psql motd
psql (14devel)
Type "help" for help.

motd=# ALTER SYSTEM SET motd TO E'\u001B[94m'
'\n ____ ______ ___ '
'\n / )/ \/ \ '
'\n ( / __ _\ )'
'\n \ (/ o) ( o) )'
'\n \_ (_ ) \ ) / '
'\n \ /\_/ \)_/ '
'\n \/ //| |\\ '
'\n v | | v '
'\n \__/ '
'\u001b[0m';
ALTER SYSTEM
motd=# SELECT pg_reload_conf();
pg_reload_conf
----------------
t
(1 row)

motd=# \q

Ascii elephant in example by Michael Paquier [1]/messages/by-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta+CoDLOEg@mail.gmail.com, with ANSI colors added by me.

[1]: /messages/by-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta+CoDLOEg@mail.gmail.com

Happy Easter!

/Joel

Attachments:

0001-quote-newlines.patchapplication/octet-stream; name=0001-quote-newlines.patchDownload
diff --git a/src/port/quotes.c b/src/port/quotes.c
index 63a219f5f4..493d6dc8d9 100644
--- a/src/port/quotes.c
+++ b/src/port/quotes.c
@@ -16,7 +16,7 @@
 #include "c.h"
 
 /*
- * Escape (by doubling) any single quotes or backslashes in given string
+ * Escape (by doubling) any single quotes, backslashes or newlines in given string
  *
  * Note: this is used to process postgresql.conf entries and to quote
  * string literals in pg_basebackup for writing the recovery configuration.
@@ -43,8 +43,20 @@ escape_single_quotes_ascii(const char *src)
 	for (i = 0, j = 0; i < len; i++)
 	{
 		if (SQL_STR_DOUBLE(src[i], true))
+    {
 			result[j++] = src[i];
-		result[j++] = src[i];
+			result[j++] = src[i];
+    }
+    else if (src[i] == '\n')
+    {
+			result[j++] = '\\';
+			result[j++] = 'n';
+    }
+    else
+    {
+  		result[j++] = src[i];
+    }
+
 	}
 	result[j] = '\0';
 	return result;
0002-motd.patchapplication/octet-stream; name=0002-motd.patchDownload
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 584daffc8a..b7bfcb269c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -635,6 +635,7 @@ static char *recovery_target_string;
 static char *recovery_target_xid_string;
 static char *recovery_target_name_string;
 static char *recovery_target_lsn_string;
+static char *motd_string;
 
 
 /* should be static, but commands/variable.c needs to get at this */
@@ -4250,6 +4251,16 @@ static struct config_string ConfigureNamesString[] =
 		check_timezone_abbreviations, assign_timezone_abbreviations, NULL
 	},
 
+	{
+		{"motd", PGC_USERSET, CLIENT_CONN_LOCALE,
+			gettext_noop("Sets the message of the day."),
+			NULL
+		},
+		&motd_string,
+		NULL,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"unix_socket_group", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
 			gettext_noop("Sets the owning group of the Unix-domain socket."),
@@ -6396,6 +6407,14 @@ BeginReportingGUCOptions(void)
 	}
 
 	report_needed = false;
+
+	char			*motd;
+	motd = GetConfigOptionByName("motd", NULL, false);
+	if (motd && strcmp(motd, "") != 0)
+	{
+		ereport(NOTICE,
+				(errmsg("%s", motd)));
+	}
 }
 
 /*
@@ -8463,16 +8482,6 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
 				free(newval.stringval);
 			if (newextra)
 				free(newextra);
-
-			/*
-			 * We must also reject values containing newlines, because the
-			 * grammar for config files doesn't support embedded newlines in
-			 * string literals.
-			 */
-			if (strchr(value, '\n'))
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("parameter value for ALTER SYSTEM must not contain a newline")));
 		}
 	}
 
#2Bruce Momjian
bruce@momjian.us
In reply to: Joel Jacobson (#1)
Re: [PATCH] Implement motd for PostgreSQL

On Fri, Apr 2, 2021 at 10:46:16PM +0200, Joel Jacobson wrote:

Dear fellow hackers,

This patch is one day late, my apologies for missing the deadline this year.

Uh, the deadline for the last commitfest was March 1, 2021, not April
1.

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

If only the physical world exists, free will is an illusion.

#3Chapman Flack
chap@anastigmatix.net
In reply to: Joel Jacobson (#1)
Re: [PATCH] Implement motd for PostgreSQL

On 04/02/21 16:46, Joel Jacobson wrote:

____ ______ ___
/ )/ /
( / __ _ )
(/ o) ( o) )
_ (_ ) ) /
/_/ )_/
/ //| |\
v | | v
__/

Slonik's backslashes are falling off. Eww.

Regards,
-Chap

#4Marko Tiikkaja
marko@joh.to
In reply to: Joel Jacobson (#1)
Re: [PATCH] Implement motd for PostgreSQL

Hi Joel

On Fri, Apr 2, 2021 at 11:47 PM Joel Jacobson <joel@compiler.org> wrote:

PostgreSQL has since long been suffering from the lack of a proper UNIX
style motd (message of the day).

First of all, thanks for your work on this! I think this is an important
feature to have, but I would love to see a way to have a set of strings
from which you choose a random one to display. That way you could brighten
your day with random positive messages.

-marko

#5Michael Paquier
michael@paquier.xyz
In reply to: Joel Jacobson (#1)
Re: [PATCH] Implement motd for PostgreSQL

On Fri, Apr 02, 2021 at 10:46:16PM +0200, Joel Jacobson wrote:

Ascii elephant in example by Michael Paquier [1], with ANSI colors added by me.

[1] /messages/by-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta+CoDLOEg@mail.gmail.com

The credit here goes to Charles Clavadetscher, not me.
--
Michael

#6Joel Jacobson
joel@compiler.org
In reply to: Bruce Momjian (#2)
Re: [PATCH] Implement motd for PostgreSQL

On Fri, Apr 2, 2021, at 22:51, Bruce Momjian wrote:

On Fri, Apr 2, 2021 at 10:46:16PM +0200, Joel Jacobson wrote:

Dear fellow hackers,

This patch is one day late, my apologies for missing the deadline this year.

Uh, the deadline for the last commitfest was March 1, 2021, not April
1.

Oh, I see. I'll make sure to submit it to next year's 1 April commitfest.

/Joel

#7Joel Jacobson
joel@compiler.org
In reply to: Michael Paquier (#5)
Re: [PATCH] Implement motd for PostgreSQL

On Sat, Apr 3, 2021, at 04:47, Michael Paquier wrote:

On Fri, Apr 02, 2021 at 10:46:16PM +0200, Joel Jacobson wrote:

Ascii elephant in example by Michael Paquier [1], with ANSI colors added by me.

[1] /messages/by-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta+CoDLOEg@mail.gmail.com

The credit here goes to Charles Clavadetscher, not me.
--
Michael

Right! Sorry about that. The initial ">" in front of the ascii art confused me, didn't understand it was part of the reply, since all text around it was your.

Many thanks Charles for the beautiful ascii art!

/Joel

#8Joel Jacobson
joel@compiler.org
In reply to: Chapman Flack (#3)
Re: [PATCH] Implement motd for PostgreSQL

On Fri, Apr 2, 2021, at 22:59, Chapman Flack wrote:

Slonik's backslashes are falling off. Eww.

Regards,
-Chap

Thanks for the bug report.

Fixed by properly escaping backslackes:

ALTER SYSTEM SET motd TO E'\u001B[94m'
'\n ____ ______ ___ '
'\n / )/ \\/ \\ '
'\n ( / __ _\\ )'
'\n \\ (/ o) ( o) )'
'\n \\_ (_ ) \\ ) / '
'\n \\ /\\_/ \\)_/ '
'\n \\/ //| |\\\\ '
'\n v | | v '
'\n \\__/ '
'\u001b[0m';

I've deployed the fix to production:

$ psql -U brad -h pit.org motd

NOTICE:
____ ______ ___
/ )/ \/ \
( / __ _\ )
\ (/ o) ( o) )
\_ (_ ) \ ) /
\ /\_/ \)_/
\/ //| |\\
v | | v
\__/
psql (14devel)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, bits: 256, compression: off)
Type "help" for help.

motd=> \q

/Joel

#9Joel Jacobson
joel@compiler.org
In reply to: Marko Tiikkaja (#4)
1 attachment(s)
Re: [PATCH] Implement motd for PostgreSQL

On Fri, Apr 2, 2021, at 23:09, Marko Tiikkaja wrote:

Hi Joel

On Fri, Apr 2, 2021 at 11:47 PM Joel Jacobson <joel@compiler.org> wrote:

PostgreSQL has since long been suffering from the lack of a proper UNIX style motd (message of the day).

First of all, thanks for your work on this! I think this is an important feature to have, but I would love to see a way to have a set of strings from which you choose a random one to display. That way you could brighten your day with random positive messages.

-marko

Fun idea! I implemented it as a Perl script using the fortune command.

There are quite a lot of elephant jokes in the fortune database actually.

$ sudo apt install fortune-mod

$ crontab -l
0 0 * * * bash -c "/usr/local/bin/fortune_slony.pl | psql"

$ psql
NOTICE:
____ ______ ___
/ )/ \/ \
( / __ _\ )
\ (/ o) ( o) )
\_ (_ ) \ ) /
\ /\_/ \)_/
\/ //| |\\
v | | v
\__/
Q: Know what the difference between your latest project
and putting wings on an elephant is?
A: Who knows? The elephant *might* fly, heh, heh...

/Joel

Attachments:

fortune_slony.pltext/x-perl-script; name=fortune_slony.plDownload
#10Charles Clavadetscher
clavadetscher@swisspug.org
In reply to: Joel Jacobson (#7)
Re: [PATCH] Implement motd for PostgreSQL

Hi

On 2021-04-03 07:16, Joel Jacobson wrote:

On Sat, Apr 3, 2021, at 04:47, Michael Paquier wrote:

On Fri, Apr 02, 2021 at 10:46:16PM +0200, Joel Jacobson wrote:

Ascii elephant in example by Michael Paquier [1], with ANSI colors

added by me.

[1]

/messages/by-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta+CoDLOEg@mail.gmail.com

The credit here goes to Charles Clavadetscher, not me.

--

Michael

Right! Sorry about that. The initial ">" in front of the ascii art
confused me, didn't understand it was part of the reply, since all
text around it was your.

Many thanks Charles for the beautiful ascii art!

/Joel

You are welcome. There were some discussions until it came to the final
form (that you can see below).
You may also want to have a look at this:

https://www.swisspug.org/wiki/index.php/Promotion

It includes a DB-Function that can be used to create the ASCII image
with texts.

Regards
Charles

--
Charles Clavadetscher
Swiss PostgreSQL Users Group
Treasurer
Spitzackerstrasse 9
CH - 8057 Zürich

http://www.swisspug.org

+---------------------------+
| ____ ______ ___ |
| / )/ \/ \ |
| ( / __ _\ ) |
| \ (/ o) ( o) ) |
| \_ (_ ) \ ) _/ |
| \ /\_/ \)/ |
| \/ <//| |\\> |
| _| | |
| \|_/ |
| |
| Swiss PostgreSQL |
| Users Group |
| |
+---------------------------+

#11Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Joel Jacobson (#1)
Re: [PATCH] Implement motd for PostgreSQL

Hello Joel,

This patch is one day late, my apologies for missing the deadline this year.

PostgreSQL has since long been suffering from the lack of a proper UNIX style motd (message of the day).

My 0.02ᅵ: apart from the Fool's day joke dimension, I'd admit that I would
not mind actually having such a fun feature in pg, possibly disabled by
default.

--
Fabien.

#12Joel Jacobson
joel@compiler.org
In reply to: Fabien COELHO (#11)
Re: [PATCH] Implement motd for PostgreSQL

On Sat, Apr 3, 2021, at 10:14, Fabien COELHO wrote:

Hello Joel,

This patch is one day late, my apologies for missing the deadline this year.

PostgreSQL has since long been suffering from the lack of a proper UNIX style motd (message of the day).

My 0.02€: apart from the Fool's day joke dimension, I'd admit that I would
not mind actually having such a fun feature in pg, possibly disabled by
default.

Fun to hear you find it useful.
I'm actually using it myself in production for something, to display instructions to users when they login.

When implementing this I stumbled upon newlines can't be used in ALTER SYSTEM parameter values.

I see they were disallowed in commit 99f3b5613bd1f145b5dbbe86000337bbe37fb094

However, reading escaped newlines seems to be working just fine.
The commit message from 2016 seems to imply otherwise:

"the configuration-file parser doesn't support embedded newlines in string literals"

The first patch, 0001-quote-newlines.patch, fixes the part of escaping newlines
before they are written to the configuration file.

Perhaps the configuration-file parser has been fixed since to support embedded newlines?
If so, then maybe it would actually be an idea to support newlines by escaping them?
Especially since newlines are supported by set_config().

/Joel

#13Chapman Flack
chap@anastigmatix.net
In reply to: Joel Jacobson (#8)
Re: [PATCH] Implement motd for PostgreSQL

On 04/03/21 01:20, Joel Jacobson wrote:

I've deployed the fix to production:

$ psql -U brad -h pit.org motd

NOTICE:
____ ______ ___
/ )/ \/ \
( / __ _\ )
\ (/ o) ( o) )
\_ (_ ) \ ) /
\ /\_/ \)_/
\/ //| |\\
v | | v
\__/

Now there's some kind of Max Headroom thing going on with the second row,
and this time I'm not sure how to explain it. (I knew the backslashes were
because they weren't doubled.)

I have done 'view source' in my mail client to make sure it's not just
some display artifact on my end. Something has eaten a space before that
left paren. What would do that?

Regards,
-Chap

#14Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Joel Jacobson (#12)
Re: [PATCH] Implement motd for PostgreSQL

Perhaps the configuration-file parser has been fixed since to support
embedded newlines? If so, then maybe it would actually be an idea to
support newlines by escaping them?

Dunno.

If such a feature gets considered, I'm not sure I'd like to actually edit
pg configuration file to change the message.

The actual source looks pretty straightforward. I'm wondering whether pg
style would suggest to write motd != NULL instead of just motd.

I'm wondering whether it should be possible to designate (1) a file the
content of which would be shown, or (2) a command, the output of which
would be shown [ok, there might be security implications on this one].

--
Fabien.

#15Joel Jacobson
joel@compiler.org
In reply to: Chapman Flack (#13)
Re: [PATCH] Implement motd for PostgreSQL

On Sat, Apr 3, 2021, at 15:43, Chapman Flack wrote:

Now there's some kind of Max Headroom thing going on with the second row,
and this time I'm not sure how to explain it. (I knew the backslashes were
because they weren't doubled.)

I have done 'view source' in my mail client to make sure it's not just
some display artifact on my end. Something has eaten a space before that
left paren. What would do that?

Thanks for noticing.
I've updated the ascii art now using the version from swisspug.org,
does it look correct now to you?

$ psql -U brad -h pit.org motd

NOTICE:
____ ______ ___
/ )/ \/ \
( / __ _\ )
\ (/ o) ( o) )
\_ (_ ) \ ) _/
\ /\_/ \)/
\/ <//| |\\>
_| |
\|_/
To be or not to be.
-- Shakespeare
To do is to be.
-- Nietzsche
To be is to do.
-- Sartre
Do be do be do.
-- Sinatra

/Joel

#16Chapman Flack
chap@anastigmatix.net
In reply to: Joel Jacobson (#15)
Re: [PATCH] Implement motd for PostgreSQL

On 04/03/21 14:24, Joel Jacobson wrote:

Thanks for noticing.
I've updated the ascii art now using the version from swisspug.org,
does it look correct now to you?

$ psql -U brad -h pit.org motd

NOTICE:
____ ______ ___
/ )/ \/ \
( / __ _\ )
\ (/ o) ( o) )
\_ (_ ) \ ) _/
\ /\_/ \)/
\/ <//| |\\>
_| |
\|_/

In the email as I received it (including in view-source, so it is
not a display artifact), rows 2 and 4 are now missing an initial space.

I've pulled up the version from the list archive also[1]/messages/by-id/b8855527-88f5-4613-a258-8523cbded8be@www.fastmail.com, and see the
same issue there.

I'm not /only/ trying to be funny here ... I'm wondering if there could
be something relevant to be learned from finding out where the initial
space is being dropped, and why.

Regards,
-Chap

[1]: /messages/by-id/b8855527-88f5-4613-a258-8523cbded8be@www.fastmail.com
/messages/by-id/b8855527-88f5-4613-a258-8523cbded8be@www.fastmail.com

#17Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Joel Jacobson (#12)
Re: [PATCH] Implement motd for PostgreSQL

On 2021-Apr-03, Joel Jacobson wrote:

I'm actually using it myself in production for something, to display
instructions to users when they login.

Yeah, such as

"If your CREATE sentences don't work, please run
CREATE SCHEMA AUTHORIZATION CURRENT_USER"

for systems where the PUBLIC schema has been dropped.

--
�lvaro Herrera Valdivia, Chile
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.

#18Joel Jacobson
joel@compiler.org
In reply to: Fabien COELHO (#14)
Re: [PATCH] Implement motd for PostgreSQL

On Sat, Apr 3, 2021, at 17:50, Fabien COELHO wrote:

Perhaps the configuration-file parser has been fixed since to support
embedded newlines? If so, then maybe it would actually be an idea to
support newlines by escaping them?

Dunno.

If such a feature gets considered, I'm not sure I'd like to actually edit
pg configuration file to change the message.

For the ALTER SYSTEM case, the value would be written to postgresql.auto.conf,
and that file we shouldn't edit manually anyway, right?

The actual source looks pretty straightforward. I'm wondering whether pg
style would suggest to write motd != NULL instead of just motd.

That's what I had originally, but when reviewing my code verifying code style,
I noticed the other case it more common:

if \([a-z]* != NULL &&
119 results in 72 files

if \([a-z]* &&
936 results in 311 files

I'm wondering whether it should be possible to designate (1) a file the
content of which would be shown, or (2) a command, the output of which
would be shown [ok, there might be security implications on this one].

Can't we just do that via plpgsql and EXECUTE somehow?

/Joel

#19Joel Jacobson
joel@compiler.org
In reply to: Joel Jacobson (#18)
Re: [PATCH] Implement motd for PostgreSQL

On Sun, Apr 4, 2021, at 07:55, Joel Jacobson wrote:

On Sat, Apr 3, 2021, at 17:50, Fabien COELHO wrote:

I'm wondering whether it should be possible to designate (1) a file the
content of which would be shown, or (2) a command, the output of which
would be shown [ok, there might be security implications on this one].

Can't we just do that via plpgsql and EXECUTE somehow?

Right, we can't since

ERROR: ALTER SYSTEM cannot be executed from a function

I wrongly thought a PROCEDURE would work, but it gives the same error.

/Joel

#20Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Joel Jacobson (#18)
Re: [PATCH] Implement motd for PostgreSQL

Hello Joel,

My 0.02€:

If such a feature gets considered, I'm not sure I'd like to actually edit
pg configuration file to change the message.

For the ALTER SYSTEM case, the value would be written to postgresql.auto.conf,
and that file we shouldn't edit manually anyway, right?

Sure. I meant change the configuration in any way, through manual editing,
alter system, whatever.

The actual source looks pretty straightforward. I'm wondering whether pg
style would suggest to write motd != NULL instead of just motd.

That's what I had originally, but when reviewing my code verifying code style,
I noticed the other case it more common:

If other cases are indeed pointers. For pgbench, all direct "if (xxx &&"
cases are simple booleans or integers, pointers seem to have "!= NULL".
While looking quickly at the grep output, ISTM that most obvious pointers
have "!= NULL" and other cases often look like booleans:

catalog/pg_operator.c: if (isDelete && t->oprcom == baseId)
catalog/toasting.c: if (check && lockmode != AccessExclusiveLock)
commands/async.c: if (amRegisteredListener && listenChannels == NIL)
commands/explain.c: if (es->analyze && es->timing)

I'm sure there are exceptions, but ISTM that the local style is "!= NULL".

I'm wondering whether it should be possible to designate (1) a file the
content of which would be shown, or (2) a command, the output of which
would be shown [ok, there might be security implications on this one].

Can't we just do that via plpgsql and EXECUTE somehow?

Hmmm.

Should we want to execute forcibly some PL/pgSQL on any new connection?
Not sure this is really desirable. I was thinking of something more
trivial, like the "motd" directeve could designate a file instead of the
message itself.

There could be a hook system to execute some user code on new connections
and other events. It could be a new kind of event trigger, eg with
connection_start, connection_end… That could be nice for other purposes,
i.e. auditing. Now, event triggers are not global, they work inside a
database, which would suggest that if extended a new connection event
would be fired per database connection, not just once per connection. Not
sure it would be a bad thing.

--
Fabien.

#21Joel Jacobson
joel@compiler.org
In reply to: Fabien COELHO (#20)
Re: [PATCH] Implement motd for PostgreSQL

On Sun, Apr 4, 2021, at 09:16, Fabien COELHO wrote:

If other cases are indeed pointers. For pgbench, all direct "if (xxx &&"
cases are simple booleans or integers, pointers seem to have "!= NULL".
While looking quickly at the grep output, ISTM that most obvious pointers
have "!= NULL" and other cases often look like booleans:

catalog/pg_operator.c: if (isDelete && t->oprcom == baseId)
catalog/toasting.c: if (check && lockmode != AccessExclusiveLock)
commands/async.c: if (amRegisteredListener && listenChannels == NIL)
commands/explain.c: if (es->analyze && es->timing)

I'm sure there are exceptions, but ISTM that the local style is "!= NULL".

Many thanks for explaining.

I'm wondering whether it should be possible to designate (1) a file the
content of which would be shown, or (2) a command, the output of which
would be shown [ok, there might be security implications on this one].

Can't we just do that via plpgsql and EXECUTE somehow?

Hmmm.

Should we want to execute forcibly some PL/pgSQL on any new connection?

Oh, of course, you want the command to be execute for each new connection.

My idea was to use PL/pgSQL to execute only when you wanted to update the stored motd value,
but of course, if you want a new value from the command for each new connection,
then that doesn't work (and it doesn't work anyway due to not being able to execute ALTER SYSTEM from functions).

Not sure this is really desirable. I was thinking of something more
trivial, like the "motd" directeve could designate a file instead of the
message itself.

There could be a hook system to execute some user code on new connections
and other events. It could be a new kind of event trigger, eg with
connection_start, connection_end… That could be nice for other purposes,
i.e. auditing. Now, event triggers are not global, they work inside a
database, which would suggest that if extended a new connection event
would be fired per database connection, not just once per connection. Not
sure it would be a bad thing.

Such a hook sounds like a good idea.
If we would have such a hook, then another possibility would be to implement motd as an extension, right?

/Joel

#22Noname
ilmari@ilmari.org
In reply to: Fabien COELHO (#20)
Re: [PATCH] Implement motd for PostgreSQL

Fabien COELHO <coelho@cri.ensmp.fr> writes:

The actual source looks pretty straightforward. I'm wondering whether pg
style would suggest to write motd != NULL instead of just motd.

That's what I had originally, but when reviewing my code verifying code style,
I noticed the other case it more common:

if \([a-z]* != NULL &&
119 results in 72 files

if \([a-z]* &&
936 results in 311 files

If other cases are indeed pointers. For pgbench, all direct "if (xxx &&"
cases are simple booleans or integers, pointers seem to have "!=
NULL". While looking quickly at the grep output, ISTM that most obvious
pointers have "!= NULL" and other cases often look like booleans:

catalog/pg_operator.c: if (isDelete && t->oprcom == baseId)
catalog/toasting.c: if (check && lockmode != AccessExclusiveLock)
commands/async.c: if (amRegisteredListener && listenChannels == NIL)
commands/explain.c: if (es->analyze && es->timing)

I'm sure there are exceptions, but ISTM that the local style is "!= NULL".

Looking specifically at code checking an expression before dereferencing
it, we get:

$ ag '(?:if|Assert)\s*\(\s*(\S+)\s*&&\s*\1->\w+' | wc -l
247

$ ag '(?:if|Assert)\s*\(\s*(\S+)\s*!=\s*NULL\s*&&\s*\1->\w+' | wc -l
74

So the shorter 'foo && foo->bar' form (which I personally prefer) is
considerably more common than the longer 'foo != NULL && foo->bar' form.

- 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

#23Joel Jacobson
joel@compiler.org
In reply to: Noname (#22)
Re: [PATCH] Implement motd for PostgreSQL

On Sun, Apr 4, 2021, at 20:42, Dagfinn Ilmari Mannsåker wrote:

Fabien COELHO <coelho@cri.ensmp.fr <mailto:coelho%40cri.ensmp.fr>> writes:

The actual source looks pretty straightforward. I'm wondering whether pg
style would suggest to write motd != NULL instead of just motd.

That's what I had originally, but when reviewing my code verifying code style,
I noticed the other case it more common:

if \([a-z]* != NULL &&
119 results in 72 files

if \([a-z]* &&
936 results in 311 files

If other cases are indeed pointers. For pgbench, all direct "if (xxx &&"
cases are simple booleans or integers, pointers seem to have "!=
NULL". While looking quickly at the grep output, ISTM that most obvious
pointers have "!= NULL" and other cases often look like booleans:

catalog/pg_operator.c: if (isDelete && t->oprcom == baseId)
catalog/toasting.c: if (check && lockmode != AccessExclusiveLock)
commands/async.c: if (amRegisteredListener && listenChannels == NIL)
commands/explain.c: if (es->analyze && es->timing)

I'm sure there are exceptions, but ISTM that the local style is "!= NULL".

Looking specifically at code checking an expression before dereferencing
it, we get:

$ ag '(?:if|Assert)\s*\(\s*(\S+)\s*&&\s*\1->\w+' | wc -l
247

$ ag '(?:if|Assert)\s*\(\s*(\S+)\s*!=\s*NULL\s*&&\s*\1->\w+' | wc -l
74

So the shorter 'foo && foo->bar' form (which I personally prefer) is
considerably more common than the longer 'foo != NULL && foo->bar' form.

Oh, I see. This gets more and more interesting.

More of the most popular variant like a good rule to follow,
except when a new improved pattern is invented and new code
written in a new way, but all old code written in the old way remains,
so less experienced developers following such a rule,
will continue to write code in the old way.

I sometimes do "git log -p" grepping for recent code changes,
to see how new code is written.

It would be nice if there would be a grep similar to "ag" that could
also dig the git repo and show date/time when such code lines
were added.

I was looking for some PostgreSQL coding convention document,
and found https://www.postgresql.org/docs/current/source-conventions.html

Maybe "foo != NULL && foo->bar" XOR "foo && foo->bar" should be added to such document?

Is it an ambition to normalize the entire code base, to use just one of the two?

If so, maybe we could use some C compiler to get the AST
for all the C files and search it for occurrences, and then after normalizing
compiling again to verify the AST is unchanged (or changed in the desired way)?

/Joel