Retire PQfn().

PQfn() has been documented as somewhat obsolete since commit
efc3a25bb0 (2003), and commit bd48114937 recently marked it unsafe
because it may write past the end of result_buf when result_is_int
is 0.  Furthermore, searches of publicly available code turned up
no callers, only language bindings that expose the function
without using it, so removing it seems unlikely to cause too much
trouble.

Since the symbol is exported, deleting it outright isn't an option,
so instead this commit teaches PQfn() to fail unconditionally with
an error that suggests alternatives.  The documentation for the
fast-path interface is replaced with a stub in the "Obsolete or
Renamed Features" appendix.  The server's fast-path support is
retained for the benefit of older clients and the frontend large
object interface.

The frontend large object interface, the only in-tree caller, now
uses PQnfn(), the private version of PQfn() added by commit
bd48114937.  I considered converting this code away from the
fast-path protocol entirely, but prepared statements can be
deallocated out from under libpq (e.g., by DISCARD ALL), and
PQexecParams() proved noticeably slower, so that is left as a
future exercise.

Reviewed-by: Christoph Berg <myon@debian.org>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/ahXE28klgxIJXBLq%40nathan

doc/src/sgml/appendix-obsolete-libpq-fastpath.sgml

@@ -0,0 +1,30 @@
+<!-- doc/src/sgml/appendix-obsolete-libpq-fastpath.sgml -->
+<!--
+  See doc/src/sgml/appendix-obsolete.sgml for why this file exists. Do not change the id attribute.
+-->
+
+<sect1 id="libpq-fastpath">
+  <title><application>libpq</application> Fast-Path Interface Removed</title>
+
+   <indexterm zone="libpq-fastpath">
+    <primary>fast path</primary>
+   </indexterm>
+
+   <indexterm zone="libpq-fastpath">
+    <primary>PQfn</primary>
+   </indexterm>
+
+   <para>
+    In <productname>PostgreSQL</productname> 19 and below,
+    <application>libpq</application> supported a fast-path interface to send
+    simple function calls to the server via the <function>PQfn</function>
+    function.  This interface was unsafe and obsolete, and thus was removed in
+    <productname>PostgreSQL</productname> 20.  The <function>PQfn</function>
+    symbol still exists so that applications continue to link, but it now
+    always fails.  One can achieve similar performance and greater
+    functionality by setting up a prepared statement to define the function
+    call.  Then, executing the statement with binary transmission of parameters
+    and results substitutes for a fast-path function call.
+   </para>
+
+</sect1>

doc/src/sgml/appendix-obsolete.sgml

@@ -40,5 +40,6 @@
  &obsolete-pgreceivexlog;
  &obsolete-auth-radius;
  &obsolete-refint;
+ &obsolete-libpq-fastpath;
 
 </appendix>

doc/src/sgml/filelist.sgml

@@ -210,3 +210,4 @@
 <!ENTITY obsolete-pgreceivexlog SYSTEM "appendix-obsolete-pgreceivexlog.sgml">
 <!ENTITY obsolete-auth-radius SYSTEM "appendix-obsolete-auth-radius.sgml">
 <!ENTITY obsolete-refint SYSTEM "appendix-obsolete-refint.sgml">
+<!ENTITY obsolete-libpq-fastpath SYSTEM "appendix-obsolete-libpq-fastpath.sgml">

doc/src/sgml/libpq.sgml

@@ -5887,7 +5887,7 @@ int PQflush(PGconn *conn);
     are permitted, command strings containing multiple SQL commands are
     disallowed, and so is <literal>COPY</literal>.
     Using synchronous command execution functions
-    such as <function>PQfn</function>,
+    such as
     <function>PQexec</function>,
     <function>PQexecParams</function>,
     <function>PQprepare</function>,
@@ -7046,121 +7046,6 @@ int PQrequestCancel(PGconn *conn);
   </sect2>
  </sect1>
 
- <sect1 id="libpq-fastpath">
-  <title>The Fast-Path Interface</title>
-
-  <indexterm zone="libpq-fastpath">
-   <primary>fast path</primary>
-  </indexterm>
-
-  <para>
-   <productname>PostgreSQL</productname> provides a fast-path interface
-   to send simple function calls to the server.
-  </para>
-
-  <warning>
-   <para>
-    This interface is unsafe and should not be used.  When
-    <parameter>result_is_int</parameter> is set to <literal>0</literal>,
-    <function>PQfn</function> may write data beyond the end of
-    <parameter>result_buf</parameter>, regardless of whether the buffer has
-    enough space for the requested number of bytes.  Furthermore, it is
-    obsolete, as one can achieve similar
-    performance and greater functionality by setting up a prepared
-    statement to define the function call.  Then, executing the statement
-    with binary transmission of parameters and results substitutes for a
-    fast-path function call.
-   </para>
-  </warning>
-
-  <para>
-   The function <function id="libpq-PQfn">PQfn</function><indexterm><primary>PQfn</primary></indexterm>
-   requests execution of a server function via the fast-path interface:
-<synopsis>
-PGresult *PQfn(PGconn *conn,
-               int fnid,
-               int *result_buf,
-               int *result_len,
-               int result_is_int,
-               const PQArgBlock *args,
-               int nargs);
-
-typedef struct
-{
-    int len;
-    int isint;
-    union
-    {
-        int *ptr;
-        int integer;
-    } u;
-} PQArgBlock;
-</synopsis>
-  </para>
-
-  <para>
-   The <parameter>fnid</parameter> argument is the OID of the function to be
-   executed.  <parameter>args</parameter> and <parameter>nargs</parameter> define the
-   parameters to be passed to the function; they must match the declared
-   function argument list.  When the <parameter>isint</parameter> field of a
-   parameter structure is true, the <parameter>u.integer</parameter> value is sent
-   to the server as an integer of the indicated length (this must be
-   2 or 4 bytes); proper byte-swapping occurs.  When <parameter>isint</parameter>
-   is false, the indicated number of bytes at <parameter>*u.ptr</parameter> are
-   sent with no processing; the data must be in the format expected by
-   the server for binary transmission of the function's argument data
-   type.  (The declaration of <parameter>u.ptr</parameter> as being of
-   type <type>int *</type> is historical; it would be better to consider
-   it <type>void *</type>.)
-   <parameter>result_buf</parameter> points to the buffer in which to place
-   the function's return value.  The caller must have allocated sufficient
-   space to store the return value.  (There is no check!) The actual result
-   length in bytes will be returned in the integer pointed to by
-   <parameter>result_len</parameter>.  If a 2- or 4-byte integer result
-   is expected, set <parameter>result_is_int</parameter> to 1, otherwise
-   set it to 0.  Setting <parameter>result_is_int</parameter> to 1 causes
-   <application>libpq</application> to byte-swap the value if necessary, so that it
-   is delivered as a proper <type>int</type> value for the client machine;
-   note that a 4-byte integer is delivered into <parameter>*result_buf</parameter>
-   for either allowed result size.
-   When <parameter>result_is_int</parameter> is 0, the binary-format byte string
-   sent by the server is returned unmodified. (In this case it's better
-   to consider <parameter>result_buf</parameter> as being of
-   type <type>void *</type>.)
-  </para>
-
-  <para>
-   <function>PQfn</function> always returns a valid
-   <structname>PGresult</structname> pointer, with
-   status <literal>PGRES_COMMAND_OK</literal> for success
-   or <literal>PGRES_FATAL_ERROR</literal> if some problem was encountered.
-   The result status should be
-   checked before the result is used.   The caller is responsible for
-   freeing  the  <structname>PGresult</structname>  with
-   <xref linkend="libpq-PQclear"/> when it is no longer needed.
-  </para>
-
-  <para>
-   To pass a NULL argument to the function, set
-   the <parameter>len</parameter> field of that parameter structure
-   to <literal>-1</literal>; the <parameter>isint</parameter>
-   and <parameter>u</parameter> fields are then irrelevant.
-  </para>
-
-  <para>
-   If the function returns NULL, <parameter>*result_len</parameter> is set
-   to <literal>-1</literal>, and <parameter>*result_buf</parameter> is not
-   modified.
-  </para>
-
-  <para>
-   Note that it is not possible to handle set-valued results when using
-   this interface.  Also, the function must be a plain function, not an
-   aggregate, window function, or procedure.
-  </para>
-
- </sect1>
-
  <sect1 id="libpq-notify">
   <title>Asynchronous Notification</title>
 

src/backend/tcop/fastpath.c

@@ -11,7 +11,9 @@
  *	  src/backend/tcop/fastpath.c
  *
  * NOTES
- *	  This cruft is the server side of PQfn.
+ *	  This cruft is the server side of PQfn.  libpq's PQfn() was retired in
+ *	  v20 and now always errors, but the server code is retained for the
+ *	  benefit of older clients and the frontend LO interface.
  *
  *-------------------------------------------------------------------------
  */

src/include/tcop/dest.h

@@ -12,8 +12,8 @@
  *
  *	  - a remote process is the destination when we are
  *		running a backend with a frontend and the frontend executes
- *		PQexec() or PQfn().  In this case, the results are sent
- *		to the frontend via the functions in backend/libpq.
+ *		PQexec().  In this case, the results are sent to the frontend via
+ *		the functions in backend/libpq.
  *
  *	  - DestNone is the destination when the system executes
  *		a query internally.  The results are discarded.

src/interfaces/libpq/fe-exec.c

@@ -2986,10 +2986,10 @@ PQendcopy(PGconn *conn)
  *		nargs			: # of arguments in args array.
  *
  * RETURNS
- *		PGresult with status = PGRES_COMMAND_OK if successful.
- *			*result_len is > 0 if there is a return value, 0 if not.
- *		PGresult with status = PGRES_FATAL_ERROR if backend returns an error.
- *		NULL on communications failure.  conn->errorMessage will be set.
+ *		This function was unsafe and is no longer supported, so it now always
+ *		sets *result_len to 0 and returns a PGresult with status set to
+ *		PGRES_FATAL_ERROR.  As before, NULL is returned instead when conn is
+ *		NULL or when the connection state doesn't permit a query cycle.
  * ----------------
  */
 
@@ -3002,15 +3002,52 @@ PQfn(PGconn *conn,
 	 const PQArgBlock *args,
 	 int nargs)
 {
-	return PQnfn(conn, fnid, result_buf, -1, result_len,
-				 result_is_int, args, nargs);
+	*result_len = 0;
+
+	if (!conn)
+		return NULL;
+
+	/*
+	 * Since this is the beginning of a query cycle, reset the error state.
+	 * However, in pipeline mode with something already queued, the error
+	 * buffer belongs to that command and we shouldn't clear it.
+	 */
+	if (conn->cmd_queue_head == NULL)
+		pqClearConnErrorState(conn);
+
+	/*
+	 * The following state checks may look pointless for a function that can
+	 * no longer succeed, but they must stay ahead of the pqSaveErrorResult()
+	 * call below, which discards any result that is still being assembled.
+	 */
+	if (conn->pipelineStatus != PQ_PIPELINE_OFF)
+	{
+		libpq_append_conn_error(conn, "%s not allowed in pipeline mode", "PQfn");
+		return NULL;
+	}
+
+	if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE ||
+		pgHavePendingResult(conn))
+	{
+		libpq_append_conn_error(conn, "connection in wrong state");
+		return NULL;
+	}
+
+	libpq_append_conn_error(conn,
+							"PQfn() is no longer supported; use a prepared "
+							"statement or PQexecParams() with binary "
+							"parameters and results instead");
+	pqSaveErrorResult(conn);
+	return pqPrepareAsyncResult(conn);
 }
 
 /*
  * PQnfn
- *		Private version of PQfn() with verification that returned data fits in
- *		result_buf when result_is_int == 0.  Setting buf_size to -1 disables
- *		this verification.
+ *		Private version of the fast-path interface with verification that
+ *		returned data fits in result_buf when result_is_int == 0.  Setting
+ *		buf_size to -1 disables this verification.  This is currently only
+ *		used by the frontend LO interface and will hopefully be removed down
+ *		the road.
  */
 PGresult *
 PQnfn(PGconn *conn, int fnid, int *result_buf, int buf_size, int *result_len,

src/interfaces/libpq/fe-lobj.c

@@ -70,7 +70,7 @@ lo_open(PGconn *conn, Oid lobjId, int mode)
 	argv[1].len = 4;
 	argv[1].u.integer = mode;
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_open, &fd, &result_len, 1, argv, 2);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_open, &fd, -1, &result_len, 1, argv, 2);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -104,8 +104,8 @@ lo_close(PGconn *conn, int fd)
 	argv[0].isint = 1;
 	argv[0].len = 4;
 	argv[0].u.integer = fd;
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_close,
-			   &retval, &result_len, 1, argv, 1);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_close,
+				&retval, -1, &result_len, 1, argv, 1);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -167,8 +167,8 @@ lo_truncate(PGconn *conn, int fd, size_t len)
 	argv[1].len = 4;
 	argv[1].u.integer = (int) len;
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_truncate,
-			   &retval, &result_len, 1, argv, 2);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_truncate,
+				&retval, -1, &result_len, 1, argv, 2);
 
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
@@ -216,8 +216,8 @@ lo_truncate64(PGconn *conn, int fd, int64_t len)
 	argv[1].len = 8;
 	argv[1].u.ptr = (int *) &len;
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_truncate64,
-			   &retval, &result_len, 1, argv, 2);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_truncate64,
+				&retval, -1, &result_len, 1, argv, 2);
 
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
@@ -320,8 +320,8 @@ lo_write(PGconn *conn, int fd, const char *buf, size_t len)
 	argv[1].len = (int) len;
 	argv[1].u.ptr = (int *) unconstify(char *, buf);
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_write,
-			   &retval, &result_len, 1, argv, 2);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_write,
+				&retval, -1, &result_len, 1, argv, 2);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -361,8 +361,8 @@ lo_lseek(PGconn *conn, int fd, int offset, int whence)
 	argv[2].len = 4;
 	argv[2].u.integer = whence;
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_lseek,
-			   &retval, &result_len, 1, argv, 3);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_lseek,
+				&retval, -1, &result_len, 1, argv, 3);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -446,8 +446,8 @@ lo_creat(PGconn *conn, int mode)
 	argv[0].isint = 1;
 	argv[0].len = 4;
 	argv[0].u.integer = mode;
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_creat,
-			   &retval, &result_len, 1, argv, 1);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_creat,
+				&retval, -1, &result_len, 1, argv, 1);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -490,8 +490,8 @@ lo_create(PGconn *conn, Oid lobjId)
 	argv[0].isint = 1;
 	argv[0].len = 4;
 	argv[0].u.integer = lobjId;
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_create,
-			   &retval, &result_len, 1, argv, 1);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_create,
+				&retval, -1, &result_len, 1, argv, 1);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -524,8 +524,8 @@ lo_tell(PGconn *conn, int fd)
 	argv[0].len = 4;
 	argv[0].u.integer = fd;
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_tell,
-			   &retval, &result_len, 1, argv, 1);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_tell,
+				&retval, -1, &result_len, 1, argv, 1);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);
@@ -598,8 +598,8 @@ lo_unlink(PGconn *conn, Oid lobjId)
 	argv[0].len = 4;
 	argv[0].u.integer = lobjId;
 
-	res = PQfn(conn, conn->lobjfuncs->fn_lo_unlink,
-			   &retval, &result_len, 1, argv, 1);
+	res = PQnfn(conn, conn->lobjfuncs->fn_lo_unlink,
+				&retval, -1, &result_len, 1, argv, 1);
 	if (PQresultStatus(res) == PGRES_COMMAND_OK)
 	{
 		PQclear(res);

src/interfaces/libpq/fe-protocol3.c

@@ -2207,7 +2207,7 @@ pqEndcopy3(PGconn *conn)
 
 
 /*
- * PQfn - Send a function call to the POSTGRES backend.
+ * PQnfn - Send a function call to the POSTGRES backend.
  *
  * See fe-exec.c for documentation.
  */
@@ -2224,10 +2224,10 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
 	int			avail;
 	int			i;
 
-	/* already validated by PQfn */
+	/* already validated by PnQfn */
 	Assert(conn->pipelineStatus == PQ_PIPELINE_OFF);
 
-	/* PQfn already validated connection state */
+	/* PQnfn already validated connection state */
 
 	if (pqPutMsgStart(PqMsg_FunctionCall, conn) < 0 ||
 		pqPutInt(fnid, 4, conn) < 0 ||	/* function id */

src/interfaces/libpq/libpq-fe.h

@@ -584,8 +584,7 @@ extern PGPing PQpingParams(const char *const *keywords,
 extern int	PQflush(PGconn *conn);
 
 /*
- * "Fast path" interface --- not really recommended for application
- * use
+ * "Fast path" interface --- no longer supported, always fails
  */
 extern PGresult *PQfn(PGconn *conn,
 					  int fnid,