Horton

Thread

Commits

  1. Use pg_assume() to avoid compiler warning below exec_set_found()

  2. Add pg_assume(expr) macro

  3. Fix gcc >= 10 warning

  4. Add support event triggers on authenticated login

  1. pgsql: Add support event triggers on authenticated login

    Alexander Korotkov <akorotkov@postgresql.org> — 2023-10-16T00:18:33Z

    Add support event triggers on authenticated login

This commit introduces trigger on login event, allowing to fire some actions
right on the user connection.  This can be useful for logging or connection
check purposes as well as for some personalization of environment.  Usage
details are described in the documentation included, but shortly usage is
the same as for other triggers: create function returning event_trigger and
then create event trigger on login event.

In order to prevent the connection time overhead when there are no triggers
the commit introduces pg_database.dathasloginevt flag, which indicates database
has active login triggers.  This flag is set by CREATE/ALTER EVENT TRIGGER
command, and unset at connection time when no active triggers found.

Author: Konstantin Knizhnik, Mikhail Gribkov
Discussion: https://postgr.es/m/0d46d29f-4558-3af9-9c85-7774e14a7709%40postgrespro.ru
Reviewed-by: Pavel Stehule, Takayuki Tsunakawa, Greg Nancarrow, Ivan Panchenko
Reviewed-by: Daniel Gustafsson, Teodor Sigaev, Robert Haas, Andres Freund
Reviewed-by: Tom Lane, Andrey Sokolov, Zhihong Yu, Sergey Shinderuk
Reviewed-by: Gregory Stark, Nikita Malakhov, Ted Yu

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/e83d1b0c40ccda8955f1245087f0697652c4df86

Modified Files
--------------
doc/src/sgml/bki.sgml                          |   2 +-
doc/src/sgml/catalogs.sgml                     |  13 ++
doc/src/sgml/ecpg.sgml                         |   2 +
doc/src/sgml/event-trigger.sgml                |  94 ++++++++++++
src/backend/commands/dbcommands.c              |  17 ++-
src/backend/commands/event_trigger.c           | 179 +++++++++++++++++++++--
src/backend/storage/lmgr/lmgr.c                |  38 +++++
src/backend/tcop/postgres.c                    |   4 +
src/backend/utils/cache/evtcache.c             |   2 +
src/backend/utils/init/globals.c               |   2 +
src/backend/utils/init/postinit.c              |   1 +
src/bin/pg_dump/pg_dump.c                      |   5 +
src/bin/psql/tab-complete.c                    |   4 +-
src/include/catalog/catversion.h               |   2 +-
src/include/catalog/pg_database.dat            |   2 +-
src/include/catalog/pg_database.h              |   3 +
src/include/commands/event_trigger.h           |   1 +
src/include/miscadmin.h                        |   2 +
src/include/storage/lmgr.h                     |   2 +
src/include/tcop/cmdtaglist.h                  |   1 +
src/include/utils/evtcache.h                   |   3 +-
src/test/authentication/t/005_login_trigger.pl | 189 +++++++++++++++++++++++++
src/test/recovery/t/001_stream_rep.pl          |  26 ++++
src/test/regress/expected/event_trigger.out    |  45 ++++++
src/test/regress/sql/event_trigger.sql         |  26 ++++
25 files changed, 644 insertions(+), 21 deletions(-)

  2. Re: pgsql: Add support event triggers on authenticated login

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2024-01-12T01:58:26Z

    On Mon, Oct 16, 2023 at 5:49 AM Alexander Korotkov
<akorotkov@postgresql.org> wrote:
>
> Add support event triggers on authenticated login

Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.

In file included from dbcommands.c:20:
dbcommands.c: In function ‘createdb’:
../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
be used uninitialized in this function [-Wmaybe-uninitialized]
  104 |         return (Datum) (X ? 1 : 0);
      |                ^~~~~~~~~~~~~~~~~~~
dbcommands.c:683:25: note: ‘src_hasloginevt’ was declared here
  683 |         bool            src_hasloginevt;
      |                         ^~~~~~~~~~~~~~~

The configure command I used is ./configure --prefix=$PWD/inst/
CFLAGS="-ggdb3 -O3" > install.log && make -j 8 install > install.log
2>&1 &:

CONFIGURE =  '--prefix=/home/ubuntu/postgres/inst/' 'CFLAGS=-ggdb3 -O3'
CC = gcc
CPPFLAGS = -D_GNU_SOURCE
CFLAGS = -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3
-Wcast-function-type -Wshadow=compatible-local -Wformat-security
-fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation -Wno-stringop-truncation -ggdb3 -O3
CFLAGS_SL = -fPIC

The compiler version is:
gcc --version
gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  3. Re: pgsql: Add support event triggers on authenticated login

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-01-12T02:55:19Z

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.

> In file included from dbcommands.c:20:
> dbcommands.c: In function ‘createdb’:
> ../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
> be used uninitialized in this function [-Wmaybe-uninitialized]

Hmm, I also see that at -O3 (not at -O2) when using Fedora 39's
gcc 13.2.1, but *not* when using RHEL8's gcc 8.5.0.

I'm not sure how excited I am about curing that, though, because gcc
13.2.1 spews several other totally baseless warnings (see attached).
Some of them match up with warnings we're seeing on buildfarm member
serinus, which I seem to recall that Andres had tracked to a known gcc
bug.

			regards, tom lane

  4. gcc build warnings at -O3

    Andres Freund <andres@anarazel.de> — 2024-02-07T20:31:38Z

    Hi,

On 2024-01-11 21:55:19 -0500, Tom Lane wrote:
> Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> > Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.
> 
> > In file included from dbcommands.c:20:
> > dbcommands.c: In function ‘createdb’:
> > ../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
> > be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> Hmm, I also see that at -O3 (not at -O2) when using Fedora 39's
> gcc 13.2.1, but *not* when using RHEL8's gcc 8.5.0.

It's visible here with gcc >= 10. That's enough versions that I think we
should care.  Interestingly enough, it seems to have recently have gotten
fixed in gcc master (14 to be).


> Some of them match up with warnings we're seeing on buildfarm member
> serinus, which I seem to recall that Andres had tracked to a known gcc bug.

Some, but I don't think all.


> In file included from ../../../../src/include/executor/instrument.h:16,
>                  from pgstat_io.c:19:
> pgstat_io.c: In function 'pgstat_count_io_op_time':
> pgstat_io.c:149:60: warning: array subscript 2 is above array bounds of 'instr_time[2][4][8]' [-Warray-bounds=]
>   149 |                 INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
>       |                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~

Huh, I don't see that with any version of gcc I tried.


> In file included from ../../../../src/include/access/htup_details.h:22,
>                  from pl_exec.c:21:
> In function 'assign_simple_var',
>     inlined from 'exec_set_found' at pl_exec.c:8349:2:
> ../../../../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of 'char[0]' [-Warray-bounds=]
>   230 |         (((varattrib_1b_e *) (PTR))->va_tag)
>       |                                    ^
> ../../../../src/include/varatt.h:94:12: note: in definition of macro 'VARTAG_IS_EXPANDED'
>    94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
>       |            ^~~
> ../../../../src/include/varatt.h:284:57: note: in expansion of macro 'VARTAG_1B_E'
>   284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
>       |                                                         ^~~~~~~~~~~
> ../../../../src/include/varatt.h:301:57: note: in expansion of macro 'VARTAG_EXTERNAL'
>   301 |         (VARATT_IS_EXTERNAL(PTR) && !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
>       |                                                         ^~~~~~~~~~~~~~~
> pl_exec.c:8537:17: note: in expansion of macro 'VARATT_IS_EXTERNAL_NON_EXPANDED'
>  8537 |                 VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function 'exec_set_found':
> cc1: note: source object is likely at address zero

This I see.  If I hint to the compiler that var->datatype->typlen != 1 when
called from exec_set_found(), the warning vanishes. E.g. with

	if (var->datatype->typlen == -1)
		__builtin_unreachable();

I see one more warning:

[1390/2375 42  58%] Compiling C object src/backend/postgres_lib.a.p/utils_adt_jsonb_util.c.o
../../../../../home/andres/src/postgresql/src/backend/utils/adt/jsonb_util.c: In function 'compareJsonbContainers':
../../../../../home/andres/src/postgresql/src/backend/utils/adt/jsonb_util.c:296:34: warning: 'va.type' may be used uninitialized [-Wmaybe-uninitialized]
  296 |                         res = (va.type > vb.type) ? 1 : -1;
      |                                ~~^~~~~
../../../../../home/andres/src/postgresql/src/backend/utils/adt/jsonb_util.c:204:33: note: 'va' declared here
  204 |                 JsonbValue      va,
      |                                 ^~


I can't really blame the compiler here.  There's a fairly lengthy comment
explaining that va.type/vb.type are set, and it took me a while to understand:

			/*
			 * It's safe to assume that the types differed, and that the va
			 * and vb values passed were set.
			 *
			 * If the two values were of the same container type, then there'd
			 * have been a chance to observe the variation in the number of
			 * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
			 * either two heterogeneously-typed containers, or a container and
			 * some scalar type.
			 *
			 * We don't have to consider the WJB_END_ARRAY and WJB_END_OBJECT
			 * cases here, because we would have seen the corresponding
			 * WJB_BEGIN_ARRAY and WJB_BEGIN_OBJECT tokens first, and
			 * concluded that they don't match.
			 */

It's not surprising that the compiler can't understand that you can't get
ra = WJB_DONE, rb = WJB_END_ARRAY or such.

Greetings,

Andres Freund

  5. Re: gcc build warnings at -O3

    Alexander Korotkov <aekorotkov@gmail.com> — 2024-02-08T20:00:56Z

    On Wed, Feb 7, 2024 at 10:31 PM Andres Freund <andres@anarazel.de> wrote:
> On 2024-01-11 21:55:19 -0500, Tom Lane wrote:
> > Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> > > Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.
> >
> > > In file included from dbcommands.c:20:
> > > dbcommands.c: In function ‘createdb’:
> > > ../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
> > > be used uninitialized in this function [-Wmaybe-uninitialized]
> >
> > Hmm, I also see that at -O3 (not at -O2) when using Fedora 39's
> > gcc 13.2.1, but *not* when using RHEL8's gcc 8.5.0.
>
> It's visible here with gcc >= 10. That's enough versions that I think we
> should care.  Interestingly enough, it seems to have recently have gotten
> fixed in gcc master (14 to be).

I managed to reproduce this warning locally.  Fixed.

------
Regards,
Alexander Korotkov