public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v9 2/4] WIP: Check for volatile defaults 2+ messages / 2 participants [nested] [flat]
* [PATCH v9 2/4] WIP: Check for volatile defaults @ 2020-12-02 05:11 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-12-02 05:11 UTC (permalink / raw) We want to check if any column *uses* a volatile default value, but after parsing and rewriting, that information appears to be lost about which column values are defaults and which were specified. insertedcols doesn't appear to be useful for this. So add a field to track if a TargetEntry is planned with column default. --- src/backend/executor/nodeModifyTable.c | 64 ++++++++++++++++++++++++-- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/makefuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/util/tlist.c | 1 + src/backend/rewrite/rewriteHandler.c | 3 ++ src/include/nodes/primnodes.h | 2 + 9 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index a53cbeeb2c..2059428e2a 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -50,6 +50,7 @@ #include "foreign/fdwapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" +#include "optimizer/optimizer.h" #include "rewrite/rewriteHandler.h" #include "storage/bufmgr.h" #include "storage/lmgr.h" @@ -2258,6 +2259,61 @@ ExecModifyTable(PlanState *pstate) return NULL; } +/* + * Determine if a table has volatile column defaults which are used by a given + * planned statement (if the column is not specified or specified as DEFAULT). + * This works only for INSERT. + */ +static bool +has_volatile_defaults(ResultRelInfo *resultRelInfo, ModifyTable *node) +{ + TupleDesc tupDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc); + Plan *plan; + + Assert(list_length(node->plans) == 1); + plan = linitial(node->plans); + + for (int attnum = 1; attnum <= tupDesc->natts; attnum++) + { + Form_pg_attribute att = TupleDescAttr(tupDesc, attnum - 1); + Expr *defexpr; + TargetEntry *tle; + + /* We don't need to check dropped/generated attributes */ + if (att->attisdropped || att->attgenerated) + continue; + + tle = list_nth(plan->targetlist, attnum - 1); + Assert(tle != NULL); + Assert(tle->resno == attnum); + + /* + * If the column was specified with a non-default value, then don't + * check the volatility of its default + */ + if (!tle->isdefault) + continue; + + /* Check the column's default value if one exists */ + defexpr = (Expr *) build_column_default(resultRelInfo->ri_RelationDesc, attnum); + if (defexpr == NULL) + continue; + + /* Run the expression through planner */ + // defexpr = expression_planner(defexpr); + // (void) ExecInitExpr(defexpr, NULL); + expression_planner(defexpr); + + if (contain_volatile_functions_not_nextval((Node *) defexpr)) + { + elog(DEBUG1, "found volatile att %d", attnum); + return true; + } + } + + return false; +} + /* ---------------------------------------------------------------- * ExecInitModifyTable * ---------------------------------------------------------------- @@ -2352,10 +2408,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * are any statement level insert triggers. */ mtstate->miinfo = NULL; - else if (mtstate->rootResultRelInfo->ri_FdwRoutine != NULL - /* || cstate->volatile_defexprs */ ) - // XXX contain_volatile_functions_not_nextval((Node *) defexpr); - /* Can't support multi-inserts to foreign tables or if there are any */ + else if (mtstate->rootResultRelInfo->ri_FdwRoutine != NULL || + has_volatile_defaults(mtstate->rootResultRelInfo, node)) + /* Can't support multi-inserts to foreign tables or if there are any + * volatile default expressions in the table. */ mtstate->miinfo = NULL; else { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index ba3ccc712c..2008e8e5d6 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2178,6 +2178,7 @@ _copyTargetEntry(const TargetEntry *from) COPY_SCALAR_FIELD(resorigtbl); COPY_SCALAR_FIELD(resorigcol); COPY_SCALAR_FIELD(resjunk); + COPY_SCALAR_FIELD(isdefault); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index a2ef853dc2..aa3cdf3729 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -770,6 +770,7 @@ _equalTargetEntry(const TargetEntry *a, const TargetEntry *b) COMPARE_SCALAR_FIELD(resorigtbl); COMPARE_SCALAR_FIELD(resorigcol); COMPARE_SCALAR_FIELD(resjunk); + COMPARE_SCALAR_FIELD(isdefault); return true; } diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index 01c110cd2f..aeeba7032f 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -254,6 +254,7 @@ makeTargetEntry(Expr *expr, tle->ressortgroupref = 0; tle->resorigtbl = InvalidOid; tle->resorigcol = 0; + tle->isdefault = false; tle->resjunk = resjunk; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 8392be6d44..924ffda1c6 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1661,6 +1661,7 @@ _outTargetEntry(StringInfo str, const TargetEntry *node) WRITE_OID_FIELD(resorigtbl); WRITE_INT_FIELD(resorigcol); WRITE_BOOL_FIELD(resjunk); + WRITE_BOOL_FIELD(isdefault); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index d2c8d58070..642b6c63c5 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1275,6 +1275,7 @@ _readTargetEntry(void) READ_OID_FIELD(resorigtbl); READ_INT_FIELD(resorigcol); READ_BOOL_FIELD(resjunk); + READ_BOOL_FIELD(isdefault); READ_DONE(); } diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 89853a0630..7ef1517027 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -349,6 +349,7 @@ apply_tlist_labeling(List *dest_tlist, List *src_tlist) dest_tle->resorigtbl = src_tle->resorigtbl; dest_tle->resorigcol = src_tle->resorigcol; dest_tle->resjunk = src_tle->resjunk; + dest_tle->isdefault = src_tle->isdefault; } } diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 0c7508a0d8..7ab13e51e5 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -986,10 +986,13 @@ rewriteTargetListIU(List *targetList, } if (new_expr) + { new_tle = makeTargetEntry((Expr *) new_expr, attrno, pstrdup(NameStr(att_tup->attname)), false); + new_tle->isdefault = true; + } } /* diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index d4ce037088..888bd36a07 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1437,6 +1437,8 @@ typedef struct TargetEntry AttrNumber resorigcol; /* column's number in source table */ bool resjunk; /* set to true to eliminate the attribute from * final target list */ + bool isdefault; /* true if using the column default, either + * by "DEFAULT" or omission of the column */ } TargetEntry; -- 2.17.0 --3yNHWXBV/QO9xKNm Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0003-COPY-flush-multi-insert-buffer-based-on-accumulat.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Securing PostgreSQL for rootless containers @ 2025-02-24 19:51 Yogesh Sharma <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Yogesh Sharma @ 2025-02-24 19:51 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> Hello Hackers, When running PostgreSQL in container as rootless and bridged network, all connection will appear as local connection not matter what their origin is and pg_hba.conf based allow/deny will not be effective. One approach is to make PostgreSQL aware of systemd socket activation, where systemd creates socket FDs and passes them to PostgreSQL. Thus providing real connection originator. Many services have adopted systemd socket activation and attached patch enables same for PostgreSQL. This patch has effect on current use of socket unless systemd socket are used. Code is also guarded when postgres is not compiled with systemd flag. Attached patch is based on HEAD. Here is a sample systemd .socket ( ~/.config/systemd/user/PostgreSQL-18.socket ) 8<------ [Unit] Description=PostgreSQL Server Socket Conflicts=postgresql-18.service [Socket] ListenStream=127.0.0.1:5432 ListenStream=192.168.1.100:5432 ListenStream=/tmp/.s.PGSQL.5432 ListenStream=/run/user/1000/.s.PGSQL.5432 [Install] WantedBy=sockets.target 8<------ Match this name with quadlet .container name, for more details https://docs.podman.io/en/latest/markdown/podman-systemd.unit.5.html For local testing one can also use systemd-socket-activate, for more details https://www.freedesktop.org/software/systemd/man/latest/systemd-socket-activate.html Notes: 1. PostgreSQL config variables must match above sockets and order. listen_addresses = '127.0.0.1,192.168.100.49' unix_socket_directories = '/tmp/,/run/user/1000/' Two TCP sockets and 2 unix sockets and in same order. 2. Since postgres container will be started on first connection, "database system is starting up" message will be visible only for first connection but not for subsequent connections. make check-world passes with or without sytsemd and with socket activation. -- Kind Regards, Yogesh Sharma PostgreSQL, Linux, and Networking Expert Open Source Enthusiast and Advocate PostgreSQL Contributors Team @ RDS Open Source Databases Amazon Web Services: https://aws.amazon.com Attachments: [text/x-patch] 0001-Add-systemd-socket-activation-supoort-in-postgresql.patch (10.3K, ../../[email protected]/2-0001-Add-systemd-socket-activation-supoort-in-postgresql.patch) download | inline diff: From 565c83e701406ed504eb8738fbc78c41f3618371 Mon Sep 17 00:00:00 2001 From: Yogesh Sharma <[email protected]> Date: Wed, 5 Feb 2025 17:06:21 -0500 Subject: [PATCH] Add systemd socket activation supoort in postgresql. Socket activiation enables systemd to start the configured configured sockets and pass FDs to PostgreSQL. These changes are only effective when postgres is compiled with --with-systemd and systemd .socket is used. Why add socket activation support? Socket activiation helps in the case where postgers container is running as rootlessi under podman. Without this change, postgres will see all connections as originating from local and pg_hba will not be able to block connections from intranet or internet. postgresql-18.socket 8<------ [Unit] Description=YS Server Socket Conflicts=postgresql-18.service [Socket] ListenStream=127.0.0.1:5432 ListenStream=192.168.100.49:5432 ListenStream=/tmp/.s.PGSQL.5432 ListenStream=/run/user/1000/.s.PGSQL.5432 [Install] WantedBy=sockets.target 8<------ Match this name with quadlet .container name https://docs.podman.io/en/latest/markdown/podman-systemd.unit.5.html This .socket file get installed in ~/.config/system/user and enabled. For local testing one can also use systemd-socket-activate https://www.freedesktop.org/software/systemd/man/latest/systemd-socket-activate.html Postgresql Config variables shall match above sockets. listen_addresses = '127.0.0.1,192.168.100.49' unix_socket_directories = '/tmp/,/run/user/1000/' --- src/backend/libpq/pqcomm.c | 76 +++++++++++------ src/test/postmaster/Makefile | 2 + .../t/004_systemd_socket_activation.pl | 84 +++++++++++++++++++ 3 files changed, 136 insertions(+), 26 deletions(-) create mode 100644 src/test/postmaster/t/004_systemd_socket_activation.pl diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index bddd6465de2..48f1b0b2f66 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -72,6 +72,10 @@ #include <mstcpip.h> #endif +#ifdef USE_SYSTEMD +#include <systemd/sd-daemon.h> +#endif + #include "common/ip.h" #include "libpq/libpq.h" #include "miscadmin.h" @@ -423,6 +427,7 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, int err; int maxconn; int ret; + int systemd_fd_count = 0; char portNumberStr[32]; const char *familyDesc; char familyDescBuf[64]; @@ -444,6 +449,12 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, hint.ai_flags = AI_PASSIVE; hint.ai_socktype = SOCK_STREAM; +#ifdef USE_SYSTEMD + systemd_fd_count = sd_listen_fds(0); + if (systemd_fd_count > 0 && systemd_fd_count <= (*NumListenSockets)) + elog(FATAL, "Sockets passed by systemd (%d) are less than listen_addresses configured in postgresql.conf", systemd_fd_count); +#endif + if (family == AF_UNIX) { /* @@ -459,12 +470,16 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, (int) (UNIXSOCK_PATH_BUFLEN - 1)))); return STATUS_ERROR; } - if (Lock_AF_UNIX(unixSocketDir, unixSocketPath) != STATUS_OK) + if (systemd_fd_count == 0 && Lock_AF_UNIX(unixSocketDir, unixSocketPath) != STATUS_OK) return STATUS_ERROR; service = unixSocketPath; + if (systemd_fd_count > 0 && sd_is_socket_unix(SD_LISTEN_FDS_START + (*NumListenSockets), SOCK_STREAM, 1, unixSocketPath, 0) < 1) + elog(FATAL, "Sockets (%d) passed by systemd is not same type as configured in postgresql.conf", *NumListenSockets); } else { + if (systemd_fd_count > 0 && sd_is_socket_inet(SD_LISTEN_FDS_START + (*NumListenSockets), AF_UNSPEC, SOCK_STREAM, 1, portNumber) < 1) + elog(FATAL, "Sockets (%d) passed by systemd is not same type as configured in postgresql.conf", *NumListenSockets); snprintf(portNumberStr, sizeof(portNumberStr), "%d", portNumber); service = portNumberStr; } @@ -538,7 +553,14 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, addrDesc = addrBuf; } - if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET) + if (systemd_fd_count > 0) + { + fd = SD_LISTEN_FDS_START + (*NumListenSockets); + ereport(LOG, (errmsg("Using systemd FD %d at %d", fd, *NumListenSockets))); + } + else + fd = socket(addr->ai_family, SOCK_STREAM, 0); + if (fd == PGINVALID_SOCKET) { ereport(LOG, (errcode_for_socket_access(), @@ -598,32 +620,34 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, } } #endif - - /* - * Note: This might fail on some OS's, like Linux older than - * 2.4.21-pre3, that don't have the IPV6_V6ONLY socket option, and map - * ipv4 addresses to ipv6. It will show ::ffff:ipv4 for all ipv4 - * connections. - */ - err = bind(fd, addr->ai_addr, addr->ai_addrlen); - if (err < 0) + if (systemd_fd_count == 0) { - int saved_errno = errno; + /* + * Note: This might fail on some OS's, like Linux older than + * 2.4.21-pre3, that don't have the IPV6_V6ONLY socket option, and + * map ipv4 addresses to ipv6. It will show ::ffff:ipv4 for all + * ipv4 connections. + */ + err = bind(fd, addr->ai_addr, addr->ai_addrlen); + if (err < 0) + { + int saved_errno = errno; - ereport(LOG, - (errcode_for_socket_access(), - /* translator: first %s is IPv4, IPv6, or Unix */ - errmsg("could not bind %s address \"%s\": %m", - familyDesc, addrDesc), - saved_errno == EADDRINUSE ? - (addr->ai_family == AF_UNIX ? - errhint("Is another postmaster already running on port %d?", - (int) portNumber) : - errhint("Is another postmaster already running on port %d?" - " If not, wait a few seconds and retry.", - (int) portNumber)) : 0)); - closesocket(fd); - continue; + ereport(LOG, + (errcode_for_socket_access(), + /* translator: first %s is IPv4, IPv6, or Unix */ + errmsg("could not bind %s address \"%s\": %m", + familyDesc, addrDesc), + saved_errno == EADDRINUSE ? + (addr->ai_family == AF_UNIX ? + errhint("Is another postmaster already running on port %d?", + (int) portNumber) : + errhint("Is another postmaster already running on port %d?" + " If not, wait a few seconds and retry.", + (int) portNumber)) : 0)); + closesocket(fd); + continue; + } } if (addr->ai_family == AF_UNIX) diff --git a/src/test/postmaster/Makefile b/src/test/postmaster/Makefile index e06b81f8c47..e1d79ee2a07 100644 --- a/src/test/postmaster/Makefile +++ b/src/test/postmaster/Makefile @@ -13,6 +13,8 @@ subdir = src/test/postmaster top_builddir = ../../.. include $(top_builddir)/src/Makefile.global +export with_systemd + check: $(prove_check) diff --git a/src/test/postmaster/t/004_systemd_socket_activation.pl b/src/test/postmaster/t/004_systemd_socket_activation.pl new file mode 100644 index 00000000000..374ccf1abb1 --- /dev/null +++ b/src/test/postmaster/t/004_systemd_socket_activation.pl @@ -0,0 +1,84 @@ + +# Copyright (c) 2024-2025, PostgreSQL Global Development Group + +use strict; +use warnings FATAL => 'all'; +use locale; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Time::HiRes qw(sleep); +use Test::More; + +if ($ENV{with_systemd} ne 'yes') +{ + plan skip_all => 'systemd not supported by this build'; +} + +# Node initialization +my $node = PostgreSQL::Test::Cluster->new('socket'); + +$node->init(); + +my $port = $node->port; +my $pgdata = $node->data_dir; +my $logfile = $node->logfile; +my $ret; +my $logstart; + +# Starting postgres using systemd-socket-activate +# -l 127.0.0.1:5432 -l 192.168.100.49:5432 +# -l [2600:1700:31f0:6b7f:5a47:caff:fe73:924f]:5432 +# -l /tmp/.s.PGSQL.5432 +# -l /run/user/1000/.s.PGSQL.5432 $PG_DEV_INST/bin/postgres +# -D $PGDATA +# Start postgresql using system +my $tcp_fd = " -l 127.0.0.1:".$port; +my $unix_fd = " -l /tmp/.s.PGSQL.".$port; +my $pg_cmd = " postgres -D ".$pgdata." &>>".$logfile." &"; +my $systemd_cmd = "systemd-socket-activate ".$tcp_fd.$unix_fd.$pg_cmd; +my $systemd_bad1 = "systemd-socket-activate ".$unix_fd.$tcp_fd.$pg_cmd; +my $systemd_bad2 = "systemd-socket-activate ".$tcp_fd.$pg_cmd; + +# Setup listen_address +$node->append_conf('postgresql.conf', "listen_addresses = '127.0.0.1'"); +# Setup unix_socket_directory +$node->append_conf('postgresql.conf', "unix_socket_directories = '/tmp'"); + +# Start with an empty logfile +$ret = PostgreSQL::Test::Utils::system_log( "echo ''>".$logfile ); + +note 'Bad test 1'; +# Test passing FD in wrong order +$ret = PostgreSQL::Test::Utils::system_log( $systemd_bad1 ); +ok($ret == "0", "systemd-socket-activate started"); +$logstart = -s $node->logfile; +# psql: error: connection to server at "127.0.0.1", port 5432 failed: FATAL: the database system is starting up +$node->connect_fails("host=127.0.0.1", "Check for failure due to wrong FD order", + expected_stderr => qr/failed: server closed the connection unexpectedly/); +ok( $node->log_contains(qr/FATAL: Sockets \(0\) passed by systemd is not same type as configured in postgresql.conf/, $logstart), + "Check for socket 0 mismtach message"); + + +note 'Bad test 2'; +# Only pass TCP FD +$ret = PostgreSQL::Test::Utils::system_log( $systemd_bad2 ); +ok($ret == "0", "systemd-socket-activate started"); +$logstart = -s $node->logfile; +# psql: error: connection to server at "127.0.0.1", port 5432 failed: FATAL: the database system is starting up +$node->connect_fails("host=127.0.0.1", "Check for failure due to wrong FD passed", + expected_stderr => qr/failed: server closed the connection unexpectedly/); +ok( $node->log_contains(qr/FATAL: Sockets passed by systemd \(1\) are less than listen_addresses configured in postgresql.conf/, $logstart), + "Check for socket 0 mismtach message"); + +note 'Good test'; +# This is a good test with all the correct settings +$ret = PostgreSQL::Test::Utils::system_log( $systemd_cmd ); +ok($ret == "0", "systemd-socket-activate started"); +# psql: error: connection to server at "127.0.0.1", port 5432 failed: FATAL: the database system is starting up +$node->connect_fails("host=127.0.0.1", "database system is starting up", expected_stderr => qr/FATAL: the database system is starting up/); +sleep(1); +$node->connect_ok("host=127.0.0.1", "Connect to IP works"); +$node->connect_ok("host=/tmp", "Connect to unix_sock works"); +$node->stop(); +done_testing(); -- 2.48.1 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2025-02-24 19:51 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]> 2025-02-24 19:51 Securing PostgreSQL for rootless containers Yogesh Sharma <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox