public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v16 09/10] pg_ls_*dir to return all the metadata from pg_stat_file..
2+ messages / 2 participants
[nested] [flat]

* [PATCH v16 09/10] pg_ls_*dir to return all the metadata from pg_stat_file..
@ 2020-03-10 02:56  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Justin Pryzby @ 2020-03-10 02:56 UTC (permalink / raw)

..but it doesn't seem worth factoring out the common bits, since stat_file
doesn't return a name, so all the field numbers are off by one.

NOTE, the atime is now shown where the mtime used to be.

Need catversion bump
---
 doc/src/sgml/func.sgml                       | 30 ++++-----
 src/backend/utils/adt/genfile.c              | 65 +++++++++-----------
 src/include/catalog/pg_proc.dat              | 34 +++++-----
 src/test/regress/expected/misc_functions.out | 16 ++---
 src/test/regress/output/tablespace.source    |  8 +--
 5 files changed, 71 insertions(+), 82 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 3a2ad3eb50..1177799916 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25329,8 +25329,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
        </entry>
        <entry><type>setof record</type></entry>
        <entry>
-        For each file in the log directory, list the file's name, size, last
-        modification time, and a boolean indicating if it is a directory.
+        For each file in the log directory, list the file and its metadata.
         Access is granted to members of the <literal>pg_monitor</literal>
         role and may be granted to other non-superuser roles.
        </entry>
@@ -25341,8 +25340,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
        </entry>
        <entry><type>setof record</type></entry>
        <entry>
-        For each file in the WAL directory, list the file's name, size, last
-        modification time, and a boolean indicating if it is a directory.
+        For each file in the WAL directory, list the file and its metadata.
         Access is granted to members of the <literal>pg_monitor</literal>
         role and may be granted to other non-superuser roles.
        </entry>
@@ -25353,9 +25351,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
        </entry>
        <entry><type>setof record</type></entry>
        <entry>
-        For each file in the WAL archive status directory, list the file's
-        name, size, last modification time, and a boolean indicating if it is a
-        directory.  Access is granted to members of the
+        For each file in the WAL archive status directory, list the file and its metadata.
+        Access is granted to members of the
         <literal>pg_monitor</literal> role and may be granted to other
         non-superuser roles.
        </entry>
@@ -25367,8 +25364,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
        <entry><type>setof record</type></entry>
        <entry>
         For the temporary directory within <parameter>tablespace</parameter>,
-        list each file's name, size, last modification time, and a boolean
-        indicating if it is a directory.  If <parameter>tablespace</parameter>
+        list each file and its metadata.  If <parameter>tablespace</parameter>
         is not provided, the <literal>pg_default</literal> tablespace is used.
         Access is granted to members of the <literal>pg_monitor</literal> role
         and may be granted to other non-superuser roles.
@@ -25454,8 +25450,8 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
    </indexterm>
    <para>
     <function>pg_ls_logdir</function> lists each file in the log directory,
-    along with file's size, last modification time, and a boolean
-    indicating if the file is a directory.  By default, only superusers
+    along with the metadata columns returned by <function>pg_stat_file</function>.
+    By default, only superusers
     and members of the <literal>pg_monitor</literal> role can use this function.
     Access may be granted to others using <command>GRANT</command>.
     Filenames beginning with a dot and special file types are not shown.
@@ -25466,8 +25462,8 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
    </indexterm>
    <para>
     <function>pg_ls_waldir</function> lists each file in the WAL directory,
-    along with the file's size, last modification time, and a boolean
-    indicating if the file is a directory.  By default, only superusers
+    along with the metadata columns returned by <function>pg_stat_file</function>.
+    By default, only superusers
     and members of the <literal>pg_monitor</literal> role
     can use this function. Access may be granted to others using
     <command>GRANT</command>.
@@ -25479,8 +25475,8 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
    </indexterm>
    <para>
     <function>pg_ls_archive_statusdir</function> lists each file in the WAL
-    archive status directory, along with the file's size, last modification
-    time, and a boolean indicating if the file is a directory.  By default, only 
+    archive status directory, along with the metadata columns returned by
+    <function>pg_stat_file</function>.  By default, only 
     superusers and members of the <literal>pg_monitor</literal> role can
     use this function. Access may be granted to others using
     <command>GRANT</command>.
@@ -25493,8 +25489,8 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
    <para>
     <function>pg_ls_tmpdir</function> lists each file in the temporary file
     directory for the specified <parameter>tablespace</parameter>, along with
-    its size, last modified time (mtime) and a boolean indicating if the file is a
-    directory.  Directories are used for temporary files shared by parallel
+    the metadata columns returned by <function>pg_stat_file</function>.
+    Directories are used for temporary files shared by parallel
     processes.  If <parameter>tablespace</parameter> is
     not provided, the <literal>pg_default</literal> tablespace is used.  By
     default only superusers and members of the <literal>pg_monitor</literal>
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 91bf8c69e9..10780d3fb1 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -36,6 +36,8 @@
 #include "utils/syscache.h"
 #include "utils/timestamp.h"
 
+static void tuple_from_stat(struct stat *fst, const char *path, Datum *values,
+		bool *isnull);
 static Datum pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags);
 
 #define	LS_DIR_ISDIR				(1<<0) /* Show column: isdir */
@@ -367,6 +369,28 @@ pg_read_binary_file_all(PG_FUNCTION_ARGS)
 	return pg_read_binary_file(fcinfo);
 }
 
+/*
+ * Populate values and isnull from fst and path.
+ * Used for pg_stat_file() and pg_stat_dir_files()
+ * isnull is assumed to have been zeroed.
+ */
+static void
+tuple_from_stat(struct stat *fst, const char *path, Datum *values, bool *isnull)
+{
+	values[0] = Int64GetDatum((int64) fst->st_size);
+	values[1] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_atime));
+	values[2] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_mtime));
+	/* Unix has file status change time, while Win32 has creation time */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+	values[3] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_ctime));
+	isnull[4] = true;
+#else
+	isnull[3] = true;
+	values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_ctime));
+#endif
+	values[5] = BoolGetDatum(S_ISDIR(fst->st_mode));
+}
+
 /*
  * stat a file
  */
@@ -418,25 +442,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
 	BlessTupleDesc(tupdesc);
 
 	memset(isnull, false, sizeof(isnull));
-
-	values[0] = Int64GetDatum((int64) fst.st_size);
-	values[1] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_atime));
-	values[2] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_mtime));
-	/* Unix has file status change time, while Win32 has creation time */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-	values[3] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime));
-	isnull[4] = true;
-#else
-	isnull[3] = true;
-	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_from_stat(&fst, filename, values, isnull);
 	tuple = heap_form_tuple(tupdesc, values, isnull);
 
 	pfree(filename);
@@ -578,8 +584,8 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags)
 
 	while ((de = ReadDir(dirdesc, dir)) != NULL)
 	{
-		Datum		values[4];
-		bool		nulls[4];
+		Datum		values[7];
+		bool		nulls[7];
 		char		path[MAXPGPATH * 2];
 		struct stat attrib;
 
@@ -618,23 +624,10 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags)
 				continue;
 		}
 
+		memset(nulls, 0, sizeof(nulls));
 		values[0] = CStringGetTextDatum(de->d_name);
 		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));
+			tuple_from_stat(&attrib, path, 1+values, 1+nulls);
 
 		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index f94f403475..34ee5a8f97 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10880,48 +10880,48 @@
 { oid => '3353', descr => 'list files in the log directory',
   proname => 'pg_ls_logdir', procost => '10', prorows => '20', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => '',
-  proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}',
-  proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_logdir' },
+  proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}',
+  proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_logdir' },
 { oid => '3354', descr => 'list of files in the WAL directory',
   proname => 'pg_ls_waldir', procost => '10', prorows => '20', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => '',
-  proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}',
-  proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_waldir' },
+  proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}',
+  proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_waldir' },
 { oid => '5031', descr => 'list of files in the archive_status directory',
   proname => 'pg_ls_archive_statusdir', procost => '10', prorows => '20',
   proretset => 't', provolatile => 'v', prorettype => 'record',
-  proargtypes => '', proallargtypes => '{text,int8,timestamptz,bool}',
-  proargmodes => '{o,o,o,o}', proargnames => '{name,size,modification,isdir}',
+  proargtypes => '', proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}',
+  proargmodes => '{o,o,o,o,o,o,o}', proargnames => '{name,size,access,modification,change,creation,isdir}',
   prosrc => 'pg_ls_archive_statusdir' },
 { oid => '5029', descr => 'list files in the pgsql_tmp directory',
   proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => '',
-  proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}',
-  proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_tmpdir_noargs' },
+  proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}',
+  proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_tmpdir_noargs' },
 { oid => '5030', descr => 'list files in the pgsql_tmp directory',
   proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => 'oid',
-  proallargtypes => '{oid,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}',
-  proargnames => '{tablespace,name,size,modification,isdir}',
+  proallargtypes => '{oid,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,o,o,o,o,o,o,o}',
+  proargnames => '{tablespace,name,size,access,modification,change,creation,isdir}',
   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}',
+  proallargtypes => '{text,bool,bool,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,i,i,o,o,o,o,o,o,o}',
+  proargnames => '{dirname,missing_ok,include_dot_dirs,name,size,access,modification,change,creation,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}',
+  proallargtypes => '{text,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,o,o,o,o,o,o,o}',
+  proargnames => '{dirname,name,size,access,modification,change,creation,isdir}',
   prosrc => 'pg_ls_dir_metadata_1arg' },
 { oid => '5034', descr => 'list all files in a directory recursively',
   proname => 'pg_ls_dir_recurse', prorows => '10000', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => 'text',
-  proallargtypes => '{text,text,int8,timestamptz,bool}',
-  proargnames => '{dirname,name,size,modification,isdir}', proargmodes => '{i,o,o,o,o}',
-  prolang => 'sql', prosrc => "with recursive ls as (select * from pg_ls_dir_metadata(dirname, true, false) union all select ls.name||'/'||a.name, a.size, a.modification, a.isdir from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.isdir) select * from ls" },
+  proallargtypes => '{text,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}',
+  proargnames => '{dirname,name,size,access,modification,change,creation,isdir}', proargmodes => '{i,o,o,o,o,o,o,o}',
+  prolang => 'sql', prosrc => "with recursive ls as (select * from pg_ls_dir_metadata(dirname, true, false) union all select ls.name||'/'||a.name, a.size, a.access, a.modification, a.change, a.creation, a.isdir from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.isdir) select * from ls" },
 
 # 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 4188d684f0..cce84a60a9 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -157,8 +157,8 @@ select count(*) > 0 as ok from (select pg_ls_waldir()) ss;
 
 -- Test not-run-to-completion cases.
 select * from pg_ls_waldir() limit 0;
- name | size | modification | isdir 
-------+------+--------------+-------
+ name | size | access | modification | change | creation | isdir 
+------+------+--------+--------------+--------+----------+-------
 (0 rows)
 
 select count(*) > 0 as ok from (select * from pg_ls_waldir() limit 1) ss;
@@ -222,8 +222,8 @@ 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 | isdir 
-------+------+--------------+-------
+ name | size | access | modification | change | creation | isdir 
+------+------+--------+--------------+--------+----------+-------
 (0 rows)
 
 select name, isdir from pg_ls_dir_metadata('.') where name='.';
@@ -239,8 +239,8 @@ select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; --
 
 -- Check that expected columns are present
 select * from pg_ls_dir_metadata('.') limit 0;
- name | size | modification | isdir 
-------+------+--------------+-------
+ name | size | access | modification | change | creation | isdir 
+------+------+--------+--------------+--------+----------+-------
 (0 rows)
 
 -- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix
@@ -253,8 +253,8 @@ SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal';
 
 -- Check that expected columns are present
 SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0;
- name | size | modification | isdir 
-------+------+--------------+-------
+ name | size | access | modification | change | creation | isdir 
+------+------+--------+--------------+--------+----------+-------
 (0 rows)
 
 --
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index ba9a3fe29a..1e1e02b589 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -17,15 +17,15 @@ CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@';
 -- 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 | isdir 
-------+------+--------------+-------
+ name | size | access | modification | change | creation | isdir 
+------+------+--------+--------------+--------+----------+-------
 (0 rows)
 
 -- This tests the missing_ok parameter.  If that's not functioning, this would ERROR if the logdir doesn't exist yet.
 -- The name='' condition is never true, so the function runs to completion but returns zero rows.
 SELECT * FROM pg_ls_logdir() WHERE name='Does not exist';
- name | size | modification | isdir 
-------+------+--------------+-------
+ name | size | access | modification | change | creation | isdir 
+------+------+--------+--------------+--------+----------+-------
 (0 rows)
 
 -- try setting and resetting some properties for the new tablespace
-- 
2.17.0


--2FkSFaIQeDFoAt0B
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v16-0010-pg_ls_-to-show-file-type-and-show-special-files.patch"



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: pgbench: using prepared BEGIN statement in a pipeline could cause an error
@ 2023-02-08 12:10  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Alvaro Herrera @ 2023-02-08 12:10 UTC (permalink / raw)
  To: Yugo NAGATA <[email protected]>; +Cc: Tom Lane <[email protected]>; Fabien COELHO <[email protected]>; pgsql-hackers

On 2023-Feb-08, Alvaro Herrera wrote:

> I propose instead the following: each command is prepared just before
> it's executed, as previously, and if we see a \startpipeline, then we
> prepare all commands starting with the one just after, and until the
> \endpipeline.

Here's the patch.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/


Attachments:

  [text/x-diff] v7-0001-Prepare-commands-in-pipelines-in-advance.patch (7.7K, ../../[email protected]/2-v7-0001-Prepare-commands-in-pipelines-in-advance.patch)
  download | inline diff:
From 6d8938009b246463efe4104f5312e37b28b55235 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <[email protected]>
Date: Wed, 8 Feb 2023 13:01:47 +0100
Subject: [PATCH v7] Prepare commands in pipelines in advance

---
 src/bin/pgbench/pgbench.c                    | 147 +++++++++++++------
 src/bin/pgbench/t/001_pgbench_with_server.pl |  17 +++
 2 files changed, 117 insertions(+), 47 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 508ed218e8..a66fd51f92 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -628,7 +628,8 @@ typedef struct
 	pg_time_usec_t txn_begin;	/* used for measuring schedule lag times */
 	pg_time_usec_t stmt_begin;	/* used for measuring statement latencies */
 
-	bool		prepared[MAX_SCRIPTS];	/* whether client prepared the script */
+	/* whether client prepared each command of each script */
+	bool	  **prepared;
 
 	/*
 	 * For processing failures and repeating transactions with serialization
@@ -739,6 +740,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
  * aset			do gset on all possible queries of a combined query (\;).
  * expr			Parsed expression, if needed.
  * stats		Time spent in this command.
+ * prepname		The name that this command is prepared under, in prepare mode
  * retries		Number of retries after a serialization or deadlock error in the
  *				current command.
  * failures		Number of errors in the current command that were not retried.
@@ -754,6 +756,7 @@ typedef struct Command
 	char	   *varprefix;
 	PgBenchExpr *expr;
 	SimpleStats stats;
+	char	   *prepname;
 	int64		retries;
 	int64		failures;
 } Command;
@@ -3006,13 +3009,6 @@ runShellCommand(Variables *variables, char *variable, char **argv, int argc)
 	return true;
 }
 
-#define MAX_PREPARE_NAME		32
-static void
-preparedStatementName(char *buffer, int file, int state)
-{
-	sprintf(buffer, "P%d_%d", file, state);
-}
-
 /*
  * Report the abortion of the client when processing SQL commands.
  */
@@ -3053,6 +3049,87 @@ chooseScript(TState *thread)
 	return i - 1;
 }
 
+/*
+ * Prepare the SQL command from st->use_file at command_num.  The name of the
+ * new prepared statement is stored in command->prepname.
+ */
+static void
+prepareCommand(CState *st, int command_num)
+{
+	Command    *command = sql_script[st->use_file].commands[command_num];
+	static int	prepnr = 0;
+
+	/* No prepare for non-SQL commands */
+	if (command->type != SQL_COMMAND)
+		return;
+
+	if (!st->prepared)
+	{
+		st->prepared = pg_malloc(sizeof(bool *) * num_scripts);
+		for (int i = 0; i < num_scripts; i++)
+		{
+			ParsedScript *script = &sql_script[i];
+			int			numcmds;
+
+			for (numcmds = 0; script->commands[numcmds] != NULL; numcmds++)
+				;
+			st->prepared[i] = pg_malloc0(numcmds);
+		}
+	}
+
+	if (!st->prepared[st->use_file][command_num])
+	{
+		PGresult   *res;
+
+		if (!command->prepname)
+			command->prepname = psprintf("P%d_%d", st->use_file, prepnr++);
+		pg_log_debug("client %d preparing %s", st->id, command->prepname);
+		res = PQprepare(st->con, command->prepname,
+						command->argv[0], command->argc - 1, NULL);
+		if (PQresultStatus(res) != PGRES_COMMAND_OK)
+			pg_log_error("%s", PQerrorMessage(st->con));
+		PQclear(res);
+		st->prepared[st->use_file][command_num] = true;
+	}
+}
+
+/*
+ * Prepare all the commands in the script that come after the \startpipeline
+ * that's at position st->command, and the first \endpipeline we find.
+ *
+ * (This sets the ->prepared flag for each relevant command, but doesn't move
+ * the st->command counter)
+ */
+static void
+prepareCommandsInPipeline(CState *st)
+{
+	int			j;
+	Command   **commands = sql_script[st->use_file].commands;
+
+	Assert(commands[st->command]->type == META_COMMAND &&
+		   commands[st->command]->meta == META_STARTPIPELINE);
+
+	/*
+	 * We set the 'prepared' flag on the \startpipeline itself to flag that we
+	 * don't need to do this next time, even though we don't actually prepare
+	 * it.
+	 */
+	if (st->prepared &&
+		st->prepared[st->use_file][st->command])
+		return;
+
+	for (j = st->command + 1; commands[j] != NULL; j++)
+	{
+		if (commands[j]->type == META_COMMAND &&
+			commands[j]->meta == META_ENDPIPELINE)
+			break;
+
+		prepareCommand(st, j);
+	}
+
+	st->prepared[st->use_file][st->command] = true;
+}
+
 /* Send a SQL command, using the chosen querymode */
 static bool
 sendCommand(CState *st, Command *command)
@@ -3083,49 +3160,14 @@ sendCommand(CState *st, Command *command)
 	}
 	else if (querymode == QUERY_PREPARED)
 	{
-		char		name[MAX_PREPARE_NAME];
 		const char *params[MAX_ARGS];
 
-		if (!st->prepared[st->use_file])
-		{
-			int			j;
-			Command   **commands = sql_script[st->use_file].commands;
-
-			for (j = 0; commands[j] != NULL; j++)
-			{
-				PGresult   *res;
-
-				if (commands[j]->type != SQL_COMMAND)
-					continue;
-				preparedStatementName(name, st->use_file, j);
-				if (PQpipelineStatus(st->con) == PQ_PIPELINE_OFF)
-				{
-					res = PQprepare(st->con, name,
-									commands[j]->argv[0], commands[j]->argc - 1, NULL);
-					if (PQresultStatus(res) != PGRES_COMMAND_OK)
-						pg_log_error("%s", PQerrorMessage(st->con));
-					PQclear(res);
-				}
-				else
-				{
-					/*
-					 * In pipeline mode, we use asynchronous functions. If a
-					 * server-side error occurs, it will be processed later
-					 * among the other results.
-					 */
-					if (!PQsendPrepare(st->con, name,
-									   commands[j]->argv[0], commands[j]->argc - 1, NULL))
-						pg_log_error("%s", PQerrorMessage(st->con));
-				}
-			}
-			st->prepared[st->use_file] = true;
-		}
+		prepareCommand(st, st->command);
 
 		getQueryParams(&st->variables, command, params);
-		preparedStatementName(name, st->use_file, st->command);
 
-		pg_log_debug("client %d sending %s", st->id, name);
-		r = PQsendQueryPrepared(st->con, name, command->argc - 1,
+		pg_log_debug("client %d sending %s", st->id, command->prepname);
+		r = PQsendQueryPrepared(st->con, command->prepname, command->argc - 1,
 								params, NULL, NULL, 0);
 	}
 	else						/* unknown sql mode */
@@ -3597,7 +3639,8 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 					thread->conn_duration += now - start;
 
 					/* Reset session-local state */
-					memset(st->prepared, 0, sizeof(st->prepared));
+					pg_free(st->prepared);
+					st->prepared = NULL;
 				}
 
 				/*
@@ -4360,6 +4403,16 @@ executeMetaCommand(CState *st, pg_time_usec_t *now)
 			return CSTATE_ABORTED;
 		}
 
+		/*
+		 * If we're in prepared-query mode, we need to prepare all the
+		 * commands that are inside the pipeline.  This solves the problem
+		 * that running BEGIN ISOLATION LEVEL SERIALIZABLE would fail, because
+		 * by the time the command is executed, a snapshot would have already
+		 * been acquired, so an error would be thrown.
+		 */
+		if (querymode == QUERY_PREPARED)
+			prepareCommandsInPipeline(st);
+
 		if (PQpipelineStatus(st->con) != PQ_PIPELINE_OFF)
 		{
 			commandFailed(st, "startpipeline", "already in pipeline mode");
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 4bf508ea96..dfdeb1f70f 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -839,6 +839,23 @@ select 1 \gset f
 }
 	});
 
+# Working \startpipeline in prepared query mode with serializable
+$node->pgbench(
+	'-t 1 -n -M prepared',
+	0,
+	[ qr{type: .*/001_pgbench_pipeline_serializable}, qr{actually processed: 1/1} ],
+	[],
+	'working \startpipeline with serializable',
+	{
+		'001_pgbench_pipeline_serializable' => q{
+-- test startpipeline with serializable
+\startpipeline
+BEGIN ISOLATION LEVEL SERIALIZABLE;
+} . "select 1;\n" x 10 . q{
+END;
+\endpipeline
+}
+	});
 
 # trigger many expression errors
 my @errors = (
-- 
2.30.2



^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2023-02-08 12:10 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 02:56 [PATCH v16 09/10] pg_ls_*dir to return all the metadata from pg_stat_file.. Justin Pryzby <[email protected]>
2023-02-08 12:10 Re: pgbench: using prepared BEGIN statement in a pipeline could cause an error Alvaro Herrera <[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