agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH v16 04/10] Add pg_ls_dir_metadata to list a dir with file metadata.. 2+ messages / 2 participants [nested] [flat]
* [PATCH v16 04/10] Add pg_ls_dir_metadata to list a dir with file metadata.. @ 2020-03-10 03:40 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-03-10 03:40 UTC (permalink / raw) Generalize pg_ls_dir_files and retire pg_ls_dir Need catversion bumped? --- doc/src/sgml/func.sgml | 19 ++ src/backend/catalog/system_views.sql | 1 + src/backend/utils/adt/genfile.c | 229 +++++++++++-------- src/include/catalog/pg_proc.dat | 12 + src/test/regress/expected/misc_functions.out | 24 ++ src/test/regress/input/tablespace.source | 5 + src/test/regress/output/tablespace.source | 8 + src/test/regress/sql/misc_functions.sql | 11 + 8 files changed, 218 insertions(+), 91 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 96b08d0500..9d617f95b9 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -25300,6 +25300,17 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); List the contents of a directory. Restricted to superusers by default, but other users can be granted EXECUTE to run the function. </entry> </row> + <row> + <entry> + <literal><function>pg_ls_dir_metadata(<parameter>dirname</parameter> <type>text</type> [, <parameter>missing_ok</parameter> <type>boolean</type>, <parameter>include_dot_dirs</parameter> <type>boolean</type>])</function></literal> + </entry> + <entry><type>setof text</type></entry> + <entry> + For each file in a directory, list the file and its metadata. + Restricted to superusers by default, but other users can be granted + EXECUTE to run the function. + </entry> + </row> <row> <entry> <literal><function>pg_ls_logdir()</function></literal> @@ -25400,6 +25411,14 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); empty directory from an non-existent directory. </para> + <indexterm> + <primary>pg_ls_dir_metadata</primary> + </indexterm> + <para> + <function>pg_ls_dir_metadata</function> lists the files in the specified + directory along with the file's metadata. + </para> + <indexterm> <primary>pg_ls_logdir</primary> </indexterm> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 2bd5f5ea14..1c77430f0c 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1467,6 +1467,7 @@ REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public; +REVOKE EXECUTE ON FUNCTION pg_ls_dir_metadata(text,boolean,boolean) FROM public; -- -- We also set up some things as accessible to standard roles. diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index 219ac160f8..4824a55480 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -36,6 +36,21 @@ #include "utils/syscache.h" #include "utils/timestamp.h" +static Datum pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags); + +#define LS_DIR_ISDIR (1<<0) /* Show column: isdir */ +#define LS_DIR_METADATA (1<<1) /* Show columns: mtime, size */ +#define LS_DIR_MISSING_OK (1<<2) /* Ignore ENOENT if the toplevel dir is missing */ +#define LS_DIR_SKIP_DOT_DIRS (1<<3) /* Do not show . or .. */ +#define LS_DIR_SKIP_HIDDEN (1<<4) /* Do not show anything begining with . */ +#define LS_DIR_SKIP_DIRS (1<<5) /* Do not show directories */ +#define LS_DIR_SKIP_SPECIAL (1<<6) /* Do not show special file types */ + +/* + * Shortcut for the historic behavior of the pg_ls_* functions (not including + * pg_ls_dir, which skips different files and doesn't show metadata. + */ +#define LS_DIR_HISTORIC (LS_DIR_SKIP_DIRS|LS_DIR_SKIP_HIDDEN|LS_DIR_SKIP_SPECIAL|LS_DIR_METADATA) /* * Convert a "text" filename argument to C string, and check it's allowable. @@ -413,6 +428,11 @@ pg_stat_file(PG_FUNCTION_ARGS) values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime)); #endif values[5] = BoolGetDatum(S_ISDIR(fst.st_mode)); +#ifdef WIN32 + /* Links should have isdir=false */ + if (pgwin32_is_junction(filename)) + values[5] = BoolGetDatum(false); +#endif tuple = heap_form_tuple(tupdesc, values, isnull); @@ -440,79 +460,9 @@ pg_stat_file_1arg(PG_FUNCTION_ARGS) Datum pg_ls_dir(PG_FUNCTION_ARGS) { - ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; - char *location; - bool missing_ok = false; - bool include_dot_dirs = false; - bool randomAccess; - TupleDesc tupdesc; - Tuplestorestate *tupstore; - DIR *dirdesc; - struct dirent *de; - MemoryContext oldcontext; - - location = convert_and_check_filename(PG_GETARG_TEXT_PP(0)); - - /* check the optional arguments */ - if (PG_NARGS() == 3) - { - if (!PG_ARGISNULL(1)) - missing_ok = PG_GETARG_BOOL(1); - if (!PG_ARGISNULL(2)) - include_dot_dirs = PG_GETARG_BOOL(2); - } - - /* check to see if caller supports us returning a tuplestore */ - if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that cannot accept a set"))); - if (!(rsinfo->allowedModes & SFRM_Materialize)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("materialize mode required, but it is not allowed in this context"))); - - /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */ - oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory); - - tupdesc = CreateTemplateTupleDesc(1); - TupleDescInitEntry(tupdesc, (AttrNumber) 1, "pg_ls_dir", TEXTOID, -1, 0); - - randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0; - tupstore = tuplestore_begin_heap(randomAccess, false, work_mem); - rsinfo->returnMode = SFRM_Materialize; - rsinfo->setResult = tupstore; - rsinfo->setDesc = tupdesc; - - MemoryContextSwitchTo(oldcontext); - - dirdesc = AllocateDir(location); - if (!dirdesc) - { - /* Return empty tuplestore if appropriate */ - if (missing_ok && errno == ENOENT) - return (Datum) 0; - /* Otherwise, we can let ReadDir() throw the error */ - } - - while ((de = ReadDir(dirdesc, location)) != NULL) - { - Datum values[1]; - bool nulls[1]; - - if (!include_dot_dirs && - (strcmp(de->d_name, ".") == 0 || - strcmp(de->d_name, "..") == 0)) - continue; - - values[0] = CStringGetTextDatum(de->d_name); - nulls[0] = false; - - tuplestore_putvalues(tupstore, tupdesc, values, nulls); - } - - FreeDir(dirdesc); - return (Datum) 0; + text *filename_t = PG_GETARG_TEXT_PP(0); + char *filename = convert_and_check_filename(filename_t); + return pg_ls_dir_files(fcinfo, filename, LS_DIR_SKIP_DOT_DIRS); } /* @@ -525,7 +475,9 @@ pg_ls_dir(PG_FUNCTION_ARGS) Datum pg_ls_dir_1arg(PG_FUNCTION_ARGS) { - return pg_ls_dir(fcinfo); + text *filename_t = PG_GETARG_TEXT_PP(0); + char *filename = convert_and_check_filename(filename_t); + return pg_ls_dir_files(fcinfo, filename, LS_DIR_SKIP_DOT_DIRS); } /* @@ -535,7 +487,7 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS) * Other unreadable-directory cases throw an error. */ static Datum -pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) +pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags) { ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; bool randomAccess; @@ -544,6 +496,32 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) DIR *dirdesc; struct dirent *de; MemoryContext oldcontext; + TypeFuncClass tuptype ; + + /* isdir depends on metadata */ + Assert(!(flags&LS_DIR_ISDIR) || (flags&LS_DIR_METADATA)); + /* Unreasonable to show isdir and skip dirs */ + Assert(!(flags&LS_DIR_ISDIR) || !(flags&LS_DIR_SKIP_DIRS)); + + /* check the optional arguments */ + if (PG_NARGS() == 3) + { + if (!PG_ARGISNULL(1)) + { + if (PG_GETARG_BOOL(1)) + flags |= LS_DIR_MISSING_OK; + else + flags &= ~LS_DIR_MISSING_OK; + } + + if (!PG_ARGISNULL(2)) + { + if (PG_GETARG_BOOL(2)) + flags &= ~LS_DIR_SKIP_DOT_DIRS; + else + flags |= LS_DIR_SKIP_DOT_DIRS; + } + } /* check to see if caller supports us returning a tuplestore */ if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) @@ -558,8 +536,20 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */ oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory); - if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) - elog(ERROR, "return type must be a row type"); + tuptype = get_call_result_type(fcinfo, NULL, &tupdesc); + if (flags & LS_DIR_METADATA) + { + if (tuptype != TYPEFUNC_COMPOSITE) + elog(ERROR, "return type must be a row type"); + } + else + { + /* pg_ls_dir returns a simple scalar */ + if (tuptype != TYPEFUNC_SCALAR) + elog(ERROR, "return type must be a scalar type"); + tupdesc = CreateTemplateTupleDesc(1); + TupleDescInitEntry(tupdesc, (AttrNumber) 1, "column", TEXTOID, -1, 0); + } randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0; tupstore = tuplestore_begin_heap(randomAccess, false, work_mem); @@ -578,20 +568,27 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) if (!dirdesc) { /* Return empty tuplestore if appropriate */ - if (missing_ok && errno == ENOENT) + if (flags & LS_DIR_MISSING_OK && errno == ENOENT) return (Datum) 0; /* Otherwise, we can let ReadDir() throw the error */ } while ((de = ReadDir(dirdesc, dir)) != NULL) { - Datum values[3]; - bool nulls[3]; + Datum values[4]; + bool nulls[4]; char path[MAXPGPATH * 2]; struct stat attrib; - /* Skip hidden files */ - if (de->d_name[0] == '.') + /* Skip dot dirs? */ + if (flags & LS_DIR_SKIP_DOT_DIRS && + (strcmp(de->d_name, ".") == 0 || + strcmp(de->d_name, "..") == 0)) + continue; + + /* Skip hidden files? */ + if (flags & LS_DIR_SKIP_HIDDEN && + de->d_name[0] == '.') continue; /* Get the file info */ @@ -606,13 +603,34 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) errmsg("could not stat file \"%s\": %m", path))); } - /* Ignore anything but regular files */ - if (!S_ISREG(attrib.st_mode)) - continue; + /* Skip dirs or special files? */ + if (S_ISDIR(attrib.st_mode)) + { + if (flags & LS_DIR_SKIP_DIRS) + continue; + } + else if (!S_ISREG(attrib.st_mode)) + { + if (flags & LS_DIR_SKIP_SPECIAL) + continue; + } values[0] = CStringGetTextDatum(de->d_name); - values[1] = Int64GetDatum((int64) attrib.st_size); - values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime)); + if (flags & LS_DIR_METADATA) + { + values[1] = Int64GetDatum((int64) attrib.st_size); + values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime)); + if (flags & LS_DIR_ISDIR) + { + values[3] = BoolGetDatum(S_ISDIR(attrib.st_mode)); +#ifdef WIN32 + /* Links should have isdir=false */ + if (pgwin32_is_junction(path)) + values[3] = BoolGetDatum(false); +#endif + } + } + memset(nulls, 0, sizeof(nulls)); tuplestore_putvalues(tupstore, tupdesc, values, nulls); @@ -626,14 +644,14 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) Datum pg_ls_logdir(PG_FUNCTION_ARGS) { - return pg_ls_dir_files(fcinfo, Log_directory, false); + return pg_ls_dir_files(fcinfo, Log_directory, LS_DIR_HISTORIC); } /* Function to return the list of files in the WAL directory */ Datum pg_ls_waldir(PG_FUNCTION_ARGS) { - return pg_ls_dir_files(fcinfo, XLOGDIR, false); + return pg_ls_dir_files(fcinfo, XLOGDIR, LS_DIR_HISTORIC); } /* @@ -651,7 +669,8 @@ pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc) tblspc))); TempTablespacePath(path, tblspc); - return pg_ls_dir_files(fcinfo, path, true); + return pg_ls_dir_files(fcinfo, path, + LS_DIR_HISTORIC | LS_DIR_MISSING_OK); } /* @@ -680,5 +699,33 @@ pg_ls_tmpdir_1arg(PG_FUNCTION_ARGS) Datum pg_ls_archive_statusdir(PG_FUNCTION_ARGS) { - return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", true); + return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", + LS_DIR_HISTORIC | LS_DIR_MISSING_OK); +} + +/* + * Function to return the list of files and metadata in an arbitrary directory. + */ +Datum +pg_ls_dir_metadata(PG_FUNCTION_ARGS) +{ + char *dirname = convert_and_check_filename(PG_GETARG_TEXT_PP(0)); + + return pg_ls_dir_files(fcinfo, dirname, + LS_DIR_METADATA | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR); +} + +/* + * Function to return the list of files and metadata in an arbitrary directory. + * note: this wrapper is necessary to pass the sanity check in opr_sanity, + * which checks that all built-in functions that share the implementing C + * function take the same number of arguments. + */ +Datum +pg_ls_dir_metadata_1arg(PG_FUNCTION_ARGS) +{ + char *dirname = convert_and_check_filename(PG_GETARG_TEXT_PP(0)); + + return pg_ls_dir_files(fcinfo, dirname, + LS_DIR_METADATA | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR); } diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 4bce3ad8de..9f46cba5ed 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10904,6 +10904,18 @@ proallargtypes => '{oid,text,int8,timestamptz}', proargmodes => '{i,o,o,o}', proargnames => '{tablespace,name,size,modification}', prosrc => 'pg_ls_tmpdir_1arg' }, +{ oid => '5032', descr => 'list directory with metadata', + proname => 'pg_ls_dir_metadata', procost => '10', prorows => '20', proretset => 't', + provolatile => 'v', prorettype => 'record', proargtypes => 'text bool bool', + proallargtypes => '{text,bool,bool,text,int8,timestamptz,bool}', proargmodes => '{i,i,i,o,o,o,o}', + proargnames => '{dirname,missing_ok,include_dot_dirs,name,size,modification,isdir}', + prosrc => 'pg_ls_dir_metadata' }, +{ oid => '5033', descr => 'list directory with metadata', + proname => 'pg_ls_dir_metadata', procost => '10', prorows => '20', proretset => 't', + provolatile => 'v', prorettype => 'record', proargtypes => 'text', + proallargtypes => '{text,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}', + proargnames => '{dirname,name,size,modification,isdir}', + prosrc => 'pg_ls_dir_metadata_1arg' }, # hash partitioning constraint function { oid => '5028', descr => 'hash partition CHECK constraint', diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index 2e87c548eb..7930909f02 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -219,6 +219,30 @@ select pg_ls_dir('does not exist', true, false); -- ok with missingok=true select pg_ls_dir('does not exist'); -- fails with missingok=false ERROR: could not open directory "does not exist": No such file or directory +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +select * from pg_ls_tmpdir() where name='Does not exist'; + name | size | modification +------+------+-------------- +(0 rows) + +select name, isdir from pg_ls_dir_metadata('.') where name='.'; + name | isdir +------+------- + . | t +(1 row) + +select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false + name | isdir +------+------- +(0 rows) + +-- Check that expected columns are present +select * from pg_ls_dir_metadata('.') limit 0; + name | size | modification | isdir +------+------+--------------+------- +(0 rows) + -- -- Test adding a support function to a subject function -- diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index a5f61a35dc..0b9cfe615e 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -11,6 +11,11 @@ DROP TABLESPACE regress_tblspacewith; -- create a tablespace we can use CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@'; +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +-- The query is written to ERROR if the tablespace doesn't exist, rather than silently failing to call pg_ls_tmpdir() +SELECT c.* FROM (SELECT oid FROM pg_tablespace b WHERE b.spcname='regress_tblspace' UNION SELECT 0 ORDER BY 1 DESC LIMIT 1) AS b , pg_ls_tmpdir(oid) AS c WHERE c.name='Does not exist'; + -- try setting and resetting some properties for the new tablespace ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1); ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- fail diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 162b591b31..a42714bf40 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -13,6 +13,14 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith'; DROP TABLESPACE regress_tblspacewith; -- create a tablespace we can use CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@'; +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +-- The query is written to ERROR if the tablespace doesn't exist, rather than silently failing to call pg_ls_tmpdir() +SELECT c.* FROM (SELECT oid FROM pg_tablespace b WHERE b.spcname='regress_tblspace' UNION SELECT 0 ORDER BY 1 DESC LIMIT 1) AS b , pg_ls_tmpdir(oid) AS c WHERE c.name='Does not exist'; + name | size | modification +------+------+-------------- +(0 rows) + -- try setting and resetting some properties for the new tablespace ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1); ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- fail diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index f6857ad177..372345720d 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -65,6 +65,17 @@ select * from (select pg_ls_dir('.', false, false) as name) as ls where ls.name= select pg_ls_dir('does not exist', true, false); -- ok with missingok=true select pg_ls_dir('does not exist'); -- fails with missingok=false +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +select * from pg_ls_tmpdir() where name='Does not exist'; + +select name, isdir from pg_ls_dir_metadata('.') where name='.'; + +select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false + +-- Check that expected columns are present +select * from pg_ls_dir_metadata('.') limit 0; + -- -- Test adding a support function to a subject function -- -- 2.17.0 --2FkSFaIQeDFoAt0B Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v16-0005-pg_ls_tmpdir-to-show-directories-and-isdir-argum.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* [PATCH] Test to demonstrate bug in commit 0d3dba38c7 and to verify a fix. @ 2026-05-12 10:27 Antonin Houska <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Antonin Houska @ 2026-05-12 10:27 UTC (permalink / raw) The fix: https://www.postgresql.org/message-id/77611.1778055944%40localhost --- src/backend/access/transam/xact.c | 2 + src/backend/commands/repack_worker.c | 2 + src/test/isolation/isolationtester.c | 9 +- .../expected/repack_running_xacts.out | 81 ++++++++++++ .../specs/repack_running_xacts.spec | 119 ++++++++++++++++++ 5 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/injection_points/expected/repack_running_xacts.out create mode 100644 src/test/modules/injection_points/specs/repack_running_xacts.spec diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 5586fbe5b07..b63ee166028 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -65,6 +65,7 @@ #include "utils/builtins.h" #include "utils/combocid.h" #include "utils/guc.h" +#include "utils/injection_point.h" #include "utils/inval.h" #include "utils/memutils.h" #include "utils/relmapper.h" @@ -2428,6 +2429,7 @@ CommitTransaction(void) * must be done _before_ releasing locks we hold and _after_ * RecordTransactionCommit. */ + INJECTION_POINT("before-end-transaction", NULL); ProcArrayEndTransaction(MyProc, latestXid); /* diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index c40f8c98e06..6835626d677 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -26,6 +26,7 @@ #include "storage/ipc.h" #include "storage/proc.h" #include "tcop/tcopprot.h" +#include "utils/injection_point.h" #include "utils/memutils.h" #define REPL_PLUGIN_NAME "pgrepack" @@ -233,6 +234,7 @@ repack_setup_logical_decoding(Oid relid) * Neither prepare_write nor do_write callback nor update_progress is * useful for us. */ + INJECTION_POINT("before-create-decoding-context", NULL); ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME, NIL, true, diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index 440c875b8ac..8f17ee412c9 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -216,15 +216,22 @@ main(int argc, char **argv) * exactly expect concurrent use of test tables. However, autovacuum will * occasionally take AccessExclusiveLock to truncate a table, and we must * ignore that transient wait. + * + * If the session's backend is blocked, and if its background worker is + * waiting on an injection point, we assume that the injection point is + * the reason for the backend to be blocked. That's what we check in the + * second query of the UNION. XXX Should we use a separate query for that? */ initPQExpBuffer(&wait_query); appendPQExpBufferStr(&wait_query, + "WITH blocking(res) AS (" "SELECT pg_catalog.pg_isolation_test_session_is_blocked($1, '{"); /* The spec syntax requires at least one session; assume that here. */ appendPQExpBufferStr(&wait_query, conns[1].backend_pid_str); for (i = 2; i < nconns; i++) appendPQExpBuffer(&wait_query, ",%s", conns[i].backend_pid_str); - appendPQExpBufferStr(&wait_query, "}')"); + appendPQExpBufferStr(&wait_query, "}') UNION " + "SELECT pg_catalog.pg_isolation_test_session_is_blocked(pid, '{}') FROM pg_stat_activity WHERE leader_pid=$1) SELECT bool_or(res) FROM blocking"); res = PQprepare(conns[0].conn, PREP_WAITING, wait_query.data, 0, NULL); if (PQresultStatus(res) != PGRES_COMMAND_OK) diff --git a/src/test/modules/injection_points/expected/repack_running_xacts.out b/src/test/modules/injection_points/expected/repack_running_xacts.out new file mode 100644 index 00000000000..271fe2b97cb --- /dev/null +++ b/src/test/modules/injection_points/expected/repack_running_xacts.out @@ -0,0 +1,81 @@ +Parsed test spec with 5 sessions + +starting permutation: repack s3_assign_xid wakeup_bcdc s4_changes s4_attach s4_commit s3_commit s5_assign_xid wakeup_bet s5_commit check +injection_points_attach +----------------------- + +(1 row) + +step repack: + REPACK (CONCURRENTLY) repack_test; + <waiting ...> +step s3_assign_xid: + BEGIN; + INSERT INTO aux VALUES (1); + +step wakeup_bcdc: + SELECT injection_points_wakeup('before-create-decoding-context'); + +injection_points_wakeup +----------------------- + +(1 row) + +step s4_changes: + BEGIN; + INSERT INTO repack_test(i, j) VALUES (1, 1); + +step s4_attach: + SELECT injection_points_set_local(); + SELECT injection_points_attach('before-end-transaction', 'wait'); + +injection_points_set_local +-------------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step s4_commit: + COMMIT; + <waiting ...> +step s3_commit: + COMMIT; + +step s5_assign_xid: + BEGIN; + INSERT INTO aux VALUES (2); + +step wakeup_bet: + SELECT injection_points_wakeup('before-end-transaction'); + +injection_points_wakeup +----------------------- + +(1 row) + +step repack: <... completed> +step s4_commit: <... completed> +step s5_commit: + COMMIT; + +step check: + TABLE repack_test; + +i|j +-+- +(0 rows) + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/specs/repack_running_xacts.spec b/src/test/modules/injection_points/specs/repack_running_xacts.spec new file mode 100644 index 00000000000..1f878514046 --- /dev/null +++ b/src/test/modules/injection_points/specs/repack_running_xacts.spec @@ -0,0 +1,119 @@ +setup +{ + CREATE EXTENSION injection_points; + CREATE TABLE repack_test(i int PRIMARY KEY, j int); + CREATE TABLE aux(i int); +} + +teardown +{ + DROP TABLE repack_test; + DROP TABLE aux; + DROP EXTENSION injection_points; +} + +session s1 +setup +{ + SELECT injection_points_attach('before-create-decoding-context', 'wait'); +} +step repack +{ + REPACK (CONCURRENTLY) repack_test; +} +step check +{ + TABLE repack_test; +} +teardown +{ + SELECT injection_points_detach('before-create-decoding-context'); +} + +session s2 +step wakeup_bcdc +{ + SELECT injection_points_wakeup('before-create-decoding-context'); +} +step wakeup_bet +{ + SELECT injection_points_wakeup('before-end-transaction'); +} + +session s3 +step s3_assign_xid +{ + BEGIN; + INSERT INTO aux VALUES (1); +} +step s3_commit +{ + COMMIT; +} + +session s4 +step s4_changes +{ + BEGIN; + INSERT INTO repack_test(i, j) VALUES (1, 1); +} +# Do not attach in the setup section, that would be too soon. +step s4_attach +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('before-end-transaction', 'wait'); +} +step s4_commit +{ + COMMIT; +} +teardown +{ + SELECT injection_points_detach('before-end-transaction'); +} + +session s5 +step s5_assign_xid +{ + BEGIN; + INSERT INTO aux VALUES (2); +} +step s5_commit +{ + COMMIT; +} + +permutation +repack +# Assign XID so that a running transaction prevents the snapshot builder from +# reaching CONSISTENT state immediately. It will wait for this to complete +# after having reached BUILDING_SNAPSHOT. +s3_assign_xid +# Let the decoding setup start. +wakeup_bcdc +# Likewise, the snapshot builder will wait for the s4's xact to complete after +# having reached FULL_SNAPSHOT. This is the problematic transaction, so let it +# do some changes. +s4_changes +# Attach to the 'before-end-transaction' injection point that s4 will need +# during commit. +s4_attach +# Only write commit record for s4, but do not remove the xact from procarray +# yet. Thus the snapshot builder still needs to wait. +s4_commit +# Let the snapshot builder proceed to FULL_SNAPSHOT. +s3_commit +# Start another transaction so that CONSISTENT is not reached "directly", +# i.e. due to no running transaction. It's important here that builder->xmin +# does not advance. +s5_assign_xid +# Remove s4 xact from procarray, and thus reach the CONSISTENT state. Since +# the COMMIT appeared in WAL too early (i.e. when the snapshot builder state +# did not allow decoding of COMMIT records yet), the snapshot builder will +# consider s4 running. This is also due to returning from +# SnapBuildProcessRunningXacts() too early, w/o advancing builder->xmin. +wakeup_bet +# s5 is not needed anymore +s5_commit +# Show that the data changes performed by s4 are lost. +check -- 2.47.3 --=-=-=-- ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-05-12 10:27 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-10 03:40 [PATCH v16 04/10] Add pg_ls_dir_metadata to list a dir with file metadata.. Justin Pryzby <[email protected]> 2026-05-12 10:27 [PATCH] Test to demonstrate bug in commit 0d3dba38c7 and to verify a fix. Antonin Houska <[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