Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Disallow newlines in parameter values to be set in ALTER SYSTEM.

  1. [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-02T20:46:16Z

    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], with ANSI colors added by me.

[1] https://www.postgresql.org/message-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta%2BCoDLOEg%40mail.gmail.com

Happy Easter!

/Joel

  2. Re: [PATCH] Implement motd for PostgreSQL

    Bruce Momjian <bruce@momjian.us> — 2021-04-02T20:51:39Z

    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.

  3. Re: [PATCH] Implement motd for PostgreSQL

    Chapman Flack <chap@anastigmatix.net> — 2021-04-02T20:59:22Z

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

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


Slonik's backslashes are falling off. Eww.

Regards,
-Chap

  4. Re: [PATCH] Implement motd for PostgreSQL

    Marko Tiikkaja <marko@joh.to> — 2021-04-02T21:09:09Z

     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

  5. Re: [PATCH] Implement motd for PostgreSQL

    Michael Paquier <michael@paquier.xyz> — 2021-04-03T02:47:49Z

    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] https://www.postgresql.org/message-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta%2BCoDLOEg%40mail.gmail.com

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

  6. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-03T05:13:34Z

    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

  7. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-03T05:16:05Z

    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] https://www.postgresql.org/message-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta%2BCoDLOEg%40mail.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

  8. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-03T05:20:23Z

    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

  9. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-03T06:50:54Z

    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

  10. Re: [PATCH] Implement motd for PostgreSQL

    Charles Clavadetscher <clavadetscher@swisspug.org> — 2021-04-03T07:08:13Z

    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]
>> 
> https://www.postgresql.org/message-id/CAB7nPqRaacwcaANOYY3Hxd3T0No5RdZXyqM5HB6fta%2BCoDLOEg%40mail.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         |
|                           |
+---------------------------+

  11. Re: [PATCH] Implement motd for PostgreSQL

    Fabien COELHO <coelho@cri.ensmp.fr> — 2021-04-03T08:14:21Z

    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.

  12. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-03T09:54:56Z

    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

  13. Re: [PATCH] Implement motd for PostgreSQL

    Chapman Flack <chap@anastigmatix.net> — 2021-04-03T13:43:56Z

    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

  14. Re: [PATCH] Implement motd for PostgreSQL

    Fabien COELHO <coelho@cri.ensmp.fr> — 2021-04-03T15:50:16Z

    
> 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.

  15. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-03T18:24:10Z

    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

  16. Re: [PATCH] Implement motd for PostgreSQL

    Chapman Flack <chap@anastigmatix.net> — 2021-04-03T18:48:53Z

    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], 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]
https://www.postgresql.org/message-id/b8855527-88f5-4613-a258-8523cbded8be%40www.fastmail.com

  17. Re: [PATCH] Implement motd for PostgreSQL

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2021-04-03T19:16:39Z

    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.

  18. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-04T05:55:16Z

    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

  19. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-04T06:23:56Z

    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

  20. Re: [PATCH] Implement motd for PostgreSQL

    Fabien COELHO <coelho@cri.ensmp.fr> — 2021-04-04T07:16:30Z

    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.

  21. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-04T07:25:31Z

    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

  22. Re: [PATCH] Implement motd for PostgreSQL

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2021-04-04T18:42:30Z

    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

  23. Re: [PATCH] Implement motd for PostgreSQL

    Joel Jacobson <joel@compiler.org> — 2021-04-05T06:19:27Z

    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