public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v15 03/14] meson: prereq: De-special-case pgevent's rc file handling 7+ messages / 4 participants [nested] [flat]
* [PATCH v15 03/14] meson: prereq: De-special-case pgevent's rc file handling @ 2022-09-13 17:07 Andres Freund <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Andres Freund @ 2022-09-13 17:07 UTC (permalink / raw) There's really no need to build win32ver.rc as part of building pgmsgevent.rc. --- src/bin/pgevent/Makefile | 5 +++-- src/bin/pgevent/pgmsgevent.rc | 2 -- src/tools/msvc/Mkvcbuild.pm | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/bin/pgevent/Makefile b/src/bin/pgevent/Makefile index dea79d3f137..04d49711d8d 100644 --- a/src/bin/pgevent/Makefile +++ b/src/bin/pgevent/Makefile @@ -17,7 +17,8 @@ ifeq ($(PORTNAME), win32) OBJS = \ pgevent.o \ - pgmsgevent.o + pgmsgevent.o \ + $(WIN32RES) NAME=pgevent SHLIB_LINK = @@ -29,7 +30,7 @@ install: all install-lib include $(top_srcdir)/src/Makefile.shlib -pgmsgevent.o: pgmsgevent.rc win32ver.rc +pgmsgevent.o: pgmsgevent.rc $(WINDRES) $< -o $@ --include-dir=$(top_builddir)/src/include --include-dir=$(top_srcdir)/src/include --include-dir=$(srcdir) --include-dir=. clean distclean: clean-lib diff --git a/src/bin/pgevent/pgmsgevent.rc b/src/bin/pgevent/pgmsgevent.rc index e69862e882c..0885a897e6f 100644 --- a/src/bin/pgevent/pgmsgevent.rc +++ b/src/bin/pgevent/pgmsgevent.rc @@ -1,4 +1,2 @@ LANGUAGE 0x9,0x1 1 11 MSG00001.bin - -#include "win32ver.rc" diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 098bc3f1b09..ddb4f25eb12 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -425,7 +425,6 @@ sub mkvcbuild $pgevent->AddFiles('src/bin/pgevent', 'pgevent.c', 'pgmsgevent.rc'); $pgevent->AddResourceFile('src/bin/pgevent', 'Eventlog message formatter', 'win32'); - $pgevent->RemoveFile('src/bin/pgevent/win32ver.rc'); $pgevent->UseDef('src/bin/pgevent/pgevent.def'); $pgevent->DisableLinkerWarnings('4104'); -- 2.37.3.542.gdd3f6c4cae --ayx3t42hstokjw3l Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v15-0004-meson-prereq-win-remove-date-from-version-number.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v13 11/20] meson: prereq: De-special-case pgevent's rc file handling @ 2022-09-13 17:07 Andres Freund <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Andres Freund @ 2022-09-13 17:07 UTC (permalink / raw) There's really no need to build win32ver.rc as part of building pgmsgevent.rc. --- src/bin/pgevent/Makefile | 5 +++-- src/bin/pgevent/pgmsgevent.rc | 2 -- src/tools/msvc/Mkvcbuild.pm | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/bin/pgevent/Makefile b/src/bin/pgevent/Makefile index dea79d3f137..04d49711d8d 100644 --- a/src/bin/pgevent/Makefile +++ b/src/bin/pgevent/Makefile @@ -17,7 +17,8 @@ ifeq ($(PORTNAME), win32) OBJS = \ pgevent.o \ - pgmsgevent.o + pgmsgevent.o \ + $(WIN32RES) NAME=pgevent SHLIB_LINK = @@ -29,7 +30,7 @@ install: all install-lib include $(top_srcdir)/src/Makefile.shlib -pgmsgevent.o: pgmsgevent.rc win32ver.rc +pgmsgevent.o: pgmsgevent.rc $(WINDRES) $< -o $@ --include-dir=$(top_builddir)/src/include --include-dir=$(top_srcdir)/src/include --include-dir=$(srcdir) --include-dir=. clean distclean: clean-lib diff --git a/src/bin/pgevent/pgmsgevent.rc b/src/bin/pgevent/pgmsgevent.rc index e69862e882c..0885a897e6f 100644 --- a/src/bin/pgevent/pgmsgevent.rc +++ b/src/bin/pgevent/pgmsgevent.rc @@ -1,4 +1,2 @@ LANGUAGE 0x9,0x1 1 11 MSG00001.bin - -#include "win32ver.rc" diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 098bc3f1b09..ddb4f25eb12 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -425,7 +425,6 @@ sub mkvcbuild $pgevent->AddFiles('src/bin/pgevent', 'pgevent.c', 'pgmsgevent.rc'); $pgevent->AddResourceFile('src/bin/pgevent', 'Eventlog message formatter', 'win32'); - $pgevent->RemoveFile('src/bin/pgevent/win32ver.rc'); $pgevent->UseDef('src/bin/pgevent/pgevent.def'); $pgevent->DisableLinkerWarnings('4104'); -- 2.37.3.542.gdd3f6c4cae --4uoxke3bx6ymzqvl Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v13-0012-meson-prereq-win-remove-date-from-version-number.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v13 11/20] meson: prereq: De-special-case pgevent's rc file handling @ 2022-09-13 17:07 Andres Freund <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Andres Freund @ 2022-09-13 17:07 UTC (permalink / raw) There's really no need to build win32ver.rc as part of building pgmsgevent.rc. --- src/bin/pgevent/Makefile | 5 +++-- src/bin/pgevent/pgmsgevent.rc | 2 -- src/tools/msvc/Mkvcbuild.pm | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/bin/pgevent/Makefile b/src/bin/pgevent/Makefile index dea79d3f137..04d49711d8d 100644 --- a/src/bin/pgevent/Makefile +++ b/src/bin/pgevent/Makefile @@ -17,7 +17,8 @@ ifeq ($(PORTNAME), win32) OBJS = \ pgevent.o \ - pgmsgevent.o + pgmsgevent.o \ + $(WIN32RES) NAME=pgevent SHLIB_LINK = @@ -29,7 +30,7 @@ install: all install-lib include $(top_srcdir)/src/Makefile.shlib -pgmsgevent.o: pgmsgevent.rc win32ver.rc +pgmsgevent.o: pgmsgevent.rc $(WINDRES) $< -o $@ --include-dir=$(top_builddir)/src/include --include-dir=$(top_srcdir)/src/include --include-dir=$(srcdir) --include-dir=. clean distclean: clean-lib diff --git a/src/bin/pgevent/pgmsgevent.rc b/src/bin/pgevent/pgmsgevent.rc index e69862e882c..0885a897e6f 100644 --- a/src/bin/pgevent/pgmsgevent.rc +++ b/src/bin/pgevent/pgmsgevent.rc @@ -1,4 +1,2 @@ LANGUAGE 0x9,0x1 1 11 MSG00001.bin - -#include "win32ver.rc" diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 098bc3f1b09..ddb4f25eb12 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -425,7 +425,6 @@ sub mkvcbuild $pgevent->AddFiles('src/bin/pgevent', 'pgevent.c', 'pgmsgevent.rc'); $pgevent->AddResourceFile('src/bin/pgevent', 'Eventlog message formatter', 'win32'); - $pgevent->RemoveFile('src/bin/pgevent/win32ver.rc'); $pgevent->UseDef('src/bin/pgevent/pgevent.def'); $pgevent->DisableLinkerWarnings('4104'); -- 2.37.3.542.gdd3f6c4cae --4uoxke3bx6ymzqvl Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v13-0012-meson-prereq-win-remove-date-from-version-number.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement @ 2023-11-21 22:58 Ivan Trofimov <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Ivan Trofimov @ 2023-11-21 22:58 UTC (permalink / raw) To: [email protected] Hi! Currently libpq sends B(ind), D(escribe), E(execute), S(ync) when executing a prepared statement. The response for that D message is a RowDescription, which doesn't change during prepared statement lifetime (with the attributes format being an exception, as they aren't know before execution) . In a presumably very common case of repeatedly executing the same statement, this leads to both client and server parsing/sending exactly the same RowDescritpion data over and over again. Instead, library user could acquire a statement result RowDescription once (via PQdescribePrepared), and reuse it in subsequent calls to PQexecPrepared and/or its async friends. Doing it this way saves a measurable amount of CPU for both client and server and saves a lot of network traffic, for example: when selecting a single row from a table with 30 columns, where each column has 10-symbols name, and every value in a row is a 10-symbols TEXT, i'm seeing an amount of bytes sent to client decreased by a factor of 2.8, and the CPU time client spends in userland decreased by a factor of ~1.5. The patch attached adds a family of functions PQsendQueryPreparedPredescribed, PQgetResultPredescribed, PQisBusyPredescribed, which allow a user to do just that. If the idea seems reasonable i'd be happy to extend these to PQexecPrepared as well and cover it with tests. P.S. This is my first time ever sending a patch via email, so please don't hesitate to point at mistakes i'm doing in the process. Attachments: [text/x-patch] PQsendQueryPreparedPredescribed.patch (11.3K, ../../[email protected]/2-PQsendQueryPreparedPredescribed.patch) download | inline diff: diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index 850734ac96..989354ca06 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -191,3 +191,6 @@ PQclosePrepared 188 PQclosePortal 189 PQsendClosePrepared 190 PQsendClosePortal 191 +PQsendQueryPreparedPredescribed 192 +PQgetResultPredescribed 193 +PQisBusyPredescribed 194 diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 04610ccf5e..a18bead9e6 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -72,8 +72,19 @@ static int PQsendQueryGuts(PGconn *conn, const char *const *paramValues, const int *paramLengths, const int *paramFormats, - int resultFormat); -static void parseInput(PGconn *conn); + int resultFormat, + bool sendDescribe); +static int PQsendQueryPreparedInternal(PGconn *conn, + const char *stmtName, + int nParams, + const char *const *paramValues, + const int *paramLengths, + const int *paramFormats, + int resultFormat, + bool sendDescribe); +static int PQisBusyInternal(PGconn *conn, PGresult *description); +static PGresult *PQgetResultInternal(PGconn *conn, PGresult *description); +static void parseInput(PGconn *conn, PGresult *description); static PGresult *getCopyResult(PGconn *conn, ExecStatusType copytype); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); @@ -1528,7 +1539,8 @@ PQsendQueryParams(PGconn *conn, paramValues, paramLengths, paramFormats, - resultFormat); + resultFormat, + true /* send Describe */ ); } /* @@ -1643,6 +1655,26 @@ PQsendQueryPrepared(PGconn *conn, const int *paramLengths, const int *paramFormats, int resultFormat) +{ + return PQsendQueryPreparedInternal(conn, + stmtName, + nParams, + paramValues, + paramLengths, + paramFormats, + resultFormat, + true /* send Describe */ ); +} + +int +PQsendQueryPreparedInternal(PGconn *conn, + const char *stmtName, + int nParams, + const char *const *paramValues, + const int *paramLengths, + const int *paramFormats, + int resultFormat, + bool sendDescribe) { if (!PQsendQueryStart(conn, true)) return 0; @@ -1668,7 +1700,50 @@ PQsendQueryPrepared(PGconn *conn, paramValues, paramLengths, paramFormats, - resultFormat); + resultFormat, + sendDescribe); +} + +int +PQsendQueryPreparedPredescribed(PGconn *conn, + const char *stmtName, + int nParams, + const char *const *paramValues, + const int *paramLengths, + const int *paramFormats, + int resultFormat, + PGresult *description) +{ + int i; + int nFields; + int send_result; + + if (!description) + { + libpq_append_conn_error(conn, "query result description is a null pointer"); + return 0; + } + + send_result = PQsendQueryPreparedInternal(conn, + stmtName, + nParams, + paramValues, + paramLengths, + paramFormats, + resultFormat, + false /* send Describe */ ); + + /* We only need to adjust attributes format if send succeeded */ + if (send_result && description->attDescs != NULL) + { + nFields = description->numAttributes; + for (i = 0; i < nFields; i++) + { + description->attDescs[i].format = resultFormat; + } + } + + return send_result; } /* @@ -1766,7 +1841,8 @@ PQsendQueryGuts(PGconn *conn, const char *const *paramValues, const int *paramLengths, const int *paramFormats, - int resultFormat) + int resultFormat, + bool sendDescribe) { int i; PGcmdQueueEntry *entry; @@ -1873,12 +1949,15 @@ PQsendQueryGuts(PGconn *conn, if (pqPutMsgEnd(conn) < 0) goto sendFailed; - /* construct the Describe Portal message */ - if (pqPutMsgStart(PqMsg_Describe, conn) < 0 || - pqPutc('P', conn) < 0 || - pqPuts("", conn) < 0 || - pqPutMsgEnd(conn) < 0) - goto sendFailed; + if (sendDescribe) + { + /* construct the Describe Portal message */ + if (pqPutMsgStart(PqMsg_Describe, conn) < 0 || + pqPutc('P', conn) < 0 || + pqPuts("", conn) < 0 || + pqPutMsgEnd(conn) < 0) + goto sendFailed; + } /* construct the Execute message */ if (pqPutMsgStart(PqMsg_Execute, conn) < 0 || @@ -1990,9 +2069,9 @@ PQconsumeInput(PGconn *conn) * Note that this function will NOT attempt to read more data from the backend. */ static void -parseInput(PGconn *conn) +parseInput(PGconn *conn, PGresult *description) { - pqParseInput3(conn); + pqParseInput3Predescribed(conn, description); } /* @@ -2002,12 +2081,18 @@ parseInput(PGconn *conn) int PQisBusy(PGconn *conn) +{ + return PQisBusyInternal(conn, NULL); +} + +static int +PQisBusyInternal(PGconn *conn, PGresult *description) { if (!conn) return false; /* Parse any available data, if our state permits. */ - parseInput(conn); + parseInput(conn, description); /* * PQgetResult will return immediately in all states except BUSY. Also, @@ -2020,6 +2105,12 @@ PQisBusy(PGconn *conn) return conn->asyncStatus == PGASYNC_BUSY && conn->status != CONNECTION_BAD; } +int +PQisBusyPredescribed(PGconn *conn, PGresult *description) +{ + return PQisBusyInternal(conn, description); +} + /* * PQgetResult * Get the next PGresult produced by a query. Returns NULL if no @@ -2033,6 +2124,12 @@ PQisBusy(PGconn *conn) */ PGresult * PQgetResult(PGconn *conn) +{ + return PQgetResultInternal(conn, NULL); +} + +static PGresult * +PQgetResultInternal(PGconn *conn, PGresult *description) { PGresult *res; @@ -2040,7 +2137,7 @@ PQgetResult(PGconn *conn) return NULL; /* Parse any available data, if our state permits. */ - parseInput(conn); + parseInput(conn, description); /* If not ready to return something, block until we are. */ while (conn->asyncStatus == PGASYNC_BUSY) @@ -2078,7 +2175,7 @@ PQgetResult(PGconn *conn) } /* Parse it. */ - parseInput(conn); + parseInput(conn, description); /* * If we had a write error, but nothing above obtained a query result @@ -2182,6 +2279,12 @@ PQgetResult(PGconn *conn) return res; } +PGresult * +PQgetResultPredescribed(PGconn *conn, PGresult *description) +{ + return PQgetResultInternal(conn, description); +} + /* * getCopyResult * Helper for PQgetResult: generate result for COPY-in-progress cases @@ -2638,7 +2741,7 @@ PQnotifies(PGconn *conn) return NULL; /* Parse any available data to see if we can extract NOTIFY messages. */ - parseInput(conn); + parseInput(conn, NULL); event = conn->notifyHead; if (event) @@ -2677,7 +2780,7 @@ PQputCopyData(PGconn *conn, const char *buffer, int nbytes) * input data into the input buffer happens down inside pqSendSome, but * it's not authorized to get rid of the data again.) */ - parseInput(conn); + parseInput(conn, NULL); if (nbytes > 0) { diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 5613c56b14..8fa5853f84 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -55,6 +55,7 @@ static void reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding); static int build_startup_packet(const PGconn *conn, char *packet, const PQEnvironmentOption *options); +static void pqParseInput3Internal(PGconn *conn, PGresult *description); /* @@ -62,8 +63,8 @@ static int build_startup_packet(const PGconn *conn, char *packet, * until input is exhausted or a stopping state is reached. * Note that this function will NOT attempt to read more data from the backend. */ -void -pqParseInput3(PGconn *conn) +static void +pqParseInput3Internal(PGconn *conn, PGresult *description) { char id; int msgLength; @@ -402,11 +403,38 @@ pqParseInput3(PGconn *conn) } else { - /* Set up to report error at end of query */ - libpq_append_conn_error(conn, "server sent data (\"D\" message) without prior row description (\"T\" message)"); - pqSaveErrorResult(conn); - /* Discard the unexpected message */ - conn->inCursor += msgLength; + if (!description) + { + /* Set up to report error at end of query */ + libpq_append_conn_error(conn, + "server sent data (\"D\" message) without prior row description (\"T\" message)"); + pqSaveErrorResult(conn); + /* Discard the unexpected message */ + conn->inCursor += msgLength; + } + else + { + /* + * We didn't receive row description from the + * server, but we have externally provided + * description, use that. + */ + if (conn->result == NULL) + { + conn->result = PQmakeEmptyPGresult(conn, PGRES_TUPLES_OK); + } + if (!conn->result) + { + libpq_append_conn_error(conn, "out of memory"); + pqSaveErrorResult(conn); + } + else + { + PQsetResultAttrs(conn->result, description->numAttributes, description->attDescs); + if (getAnotherTuple(conn, msgLength)) + return; + } + } } break; case PqMsg_CopyInResponse: @@ -478,6 +506,18 @@ pqParseInput3(PGconn *conn) } } +void +pqParseInput3(PGconn *conn) +{ + pqParseInput3Internal(conn, NULL); +} + +void +pqParseInput3Predescribed(PGconn *conn, PGresult *description) +{ + pqParseInput3Internal(conn, description); +} + /* * handleSyncLoss: clean up after loss of message-boundary sync * diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 97762d56f5..573b68fea2 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -462,11 +462,21 @@ extern int PQsendQueryPrepared(PGconn *conn, const int *paramLengths, const int *paramFormats, int resultFormat); +extern int PQsendQueryPreparedPredescribed(PGconn *conn, + const char *stmtName, + int nParams, + const char *const *paramValues, + const int *paramLengths, + const int *paramFormats, + int resultFormat, + PGresult *description); extern int PQsetSingleRowMode(PGconn *conn); extern PGresult *PQgetResult(PGconn *conn); +extern PGresult *PQgetResultPredescribed(PGconn *conn, PGresult *description); /* Routines for managing an asynchronous query */ extern int PQisBusy(PGconn *conn); +extern int PQisBusyPredescribed(PGconn *conn, PGresult *description); extern int PQconsumeInput(PGconn *conn); /* Routines for pipeline mode management */ diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index c745facfec..df91c39fbe 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -706,6 +706,7 @@ extern int PQsendQueryContinue(PGconn *conn, const char *query); extern char *pqBuildStartupPacket3(PGconn *conn, int *packetlen, const PQEnvironmentOption *options); extern void pqParseInput3(PGconn *conn); +extern void pqParseInput3Predescribed(PGconn *conn, PGresult *description); extern int pqGetErrorNotice3(PGconn *conn, bool isError); extern void pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res, PGVerbosity verbosity, PGContextVisibility show_context); ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement @ 2023-11-22 06:46 Andrey M. Borodin <[email protected]> parent: Ivan Trofimov <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Andrey M. Borodin @ 2023-11-22 06:46 UTC (permalink / raw) To: Ivan Trofimov <[email protected]>; +Cc: [email protected] Hi Ivan, thank you for the patch. > On 22 Nov 2023, at 03:58, Ivan Trofimov <[email protected]> wrote: > > Currently libpq sends B(ind), D(escribe), E(execute), S(ync) when executing a prepared statement. > The response for that D message is a RowDescription, which doesn't change during prepared > statement lifetime (with the attributes format being an exception, as they aren't know before execution) . From my POV the idea seems reasonable (though I’m not a real libpq expert). BTW some drivers also send Describe even before Bind. This creates some fuss for routing connection poolers. > In a presumably very common case of repeatedly executing the same statement, this leads to > both client and server parsing/sending exactly the same RowDescritpion data over and over again. > Instead, library user could acquire a statement result RowDescription once (via PQdescribePrepared), > and reuse it in subsequent calls to PQexecPrepared and/or its async friends. But what if query result structure changes? Will we detect this error gracefully and return correct error? Best regards, Andrey Borodin. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement @ 2023-11-26 16:38 Ivan Trofimov <[email protected]> parent: Andrey M. Borodin <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Ivan Trofimov @ 2023-11-26 16:38 UTC (permalink / raw) To: Andrey M. Borodin <[email protected]>; +Cc: [email protected] >> In a presumably very common case of repeatedly executing the same statement, this leads to >> both client and server parsing/sending exactly the same RowDescritpion data over and over again. >> Instead, library user could acquire a statement result RowDescription once (via PQdescribePrepared), >> and reuse it in subsequent calls to PQexecPrepared and/or its async friends. > But what if query result structure changes? Will we detect this error gracefully and return correct error? Afaik changing prepared statement result structure is prohibited by Postgres server-side, and should always lead to "ERROR: cached plan must not change result type", see src/test/regress/sql/plancache.sql. So yes, from the libpq point of view this is just an server error, which would be given to the user, the patch shouldn't change any behavior here. The claim about this always being a server-side error better be reassured from someone from the Postgres team, of course. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement @ 2023-11-26 17:13 Tom Lane <[email protected]> parent: Ivan Trofimov <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2023-11-26 17:13 UTC (permalink / raw) To: Ivan Trofimov <[email protected]>; +Cc: Andrey M. Borodin <[email protected]>; [email protected] Ivan Trofimov <[email protected]> writes: > Afaik changing prepared statement result structure is prohibited by > Postgres server-side, and should always lead to "ERROR: cached plan > must not change result type", see src/test/regress/sql/plancache.sql. Independently of whether we're willing to guarantee that that will never change, I think this patch is basically a bad idea as presented. It adds a whole new set of programmer-error possibilities, and I doubt it saves enough in typical cases to justify creating such a foot-gun. Moreover, it will force us to devote attention to the problem of keeping libpq itself from misbehaving badly in the inevitable situation that somebody passes the wrong tuple descriptor. That is developer effort we could better spend elsewhere. I say this as somebody who deliberately designed the v3 protocol to allow clients to skip Describe steps if they were confident they knew the query result type. I am not disavowing that choice; I just think that successful use of that option requires a client- side coding structure that allows tying a previously-obtained tuple descriptor to the current query with confidence. The proposed API fails badly at that, or at least leaves it up to the end-user programmer while providing no tools to help her get it right. Instead, I'm tempted to suggest having PQprepare/PQexecPrepared maintain a cache that maps statement name to result tupdesc, so that this is all handled internally to libpq. The main hole in that idea is that it's possible to issue PREPARE, DEALLOCATE, etc via PQexec, so that a user could possibly redefine a prepared statement without libpq noticing it. Maybe that's not a big issue. For a little more safety, we could add some extra step that the library user has to take to enable caching of result tupdescs, whereupon it's definitely caller error if she does that and then changes the statement behind our back. BTW, the submitted patch lacks both documentation and tests. For a feature like this, there is much to be said for writing the documentation *first*. Having to explain how to use something often helps you figure out weak spots in your initial design. regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2023-11-26 17:13 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-09-13 17:07 [PATCH v15 03/14] meson: prereq: De-special-case pgevent's rc file handling Andres Freund <[email protected]> 2022-09-13 17:07 [PATCH v13 11/20] meson: prereq: De-special-case pgevent's rc file handling Andres Freund <[email protected]> 2022-09-13 17:07 [PATCH v13 11/20] meson: prereq: De-special-case pgevent's rc file handling Andres Freund <[email protected]> 2023-11-21 22:58 WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement Ivan Trofimov <[email protected]> 2023-11-22 06:46 ` Re: WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement Andrey M. Borodin <[email protected]> 2023-11-26 16:38 ` Re: WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement Ivan Trofimov <[email protected]> 2023-11-26 17:13 ` Re: WIP: libpq: add a possibility to not send D(escribe) when executing a prepared statement Tom Lane <[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