aboutsummaryrefslogtreecommitdiff
path: root/src/interfaces/libpq/fe-exec.c
Commit message (Collapse)AuthorAge
* With GB18030, prevent SIGSEGV from reading past end of allocation.Noah Misch2025-05-05
| | | | | | | | | | | | | | | | | | | | | With GB18030 as source encoding, applications could crash the server via SQL functions convert() or convert_from(). Applications themselves could crash after passing unterminated GB18030 input to libpq functions PQescapeLiteral(), PQescapeIdentifier(), PQescapeStringConn(), or PQescapeString(). Extension code could crash by passing unterminated GB18030 input to jsonapi.h functions. All those functions have been intended to handle untrusted, unterminated input safely. A crash required allocating the input such that the last byte of the allocation was the last byte of a virtual memory page. Some malloc() implementations take measures against that, making the SIGSEGV hard to reach. Back-patch to v13 (all supported versions). Author: Noah Misch <noah@leadboat.com> Author: Andres Freund <andres@anarazel.de> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Backpatch-through: 13 Security: CVE-2025-4207
* Make escaping functions retain trailing bytes of an invalid character.Tom Lane2025-02-15
| | | | | | | | | | | | | | | | | | | | | | Instead of dropping the trailing byte(s) of an invalid or incomplete multibyte character, replace only the first byte with a known-invalid sequence, and process the rest normally. This seems less likely to confuse incautious callers than the behavior adopted in 5dc1e42b4. While we're at it, adjust PQescapeStringInternal to produce at most one bleat about invalid multibyte characters per string. This matches the behavior of PQescapeInternal, and avoids the risk of producing tons of repetitive junk if a long string is simply given in the wrong encoding. This is a followup to the fixes for CVE-2025-1094, and should be included if cherry-picking those fixes. Author: Andres Freund <andres@anarazel.de> Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/20250215012712.45@rfd.leadboat.com Backpatch-through: 13
* Fix PQescapeLiteral()/PQescapeIdentifier() length handlingAndres Freund2025-02-14
| | | | | | | | | | | | | | | | | | | | | | | | | | In 5dc1e42b4fa I fixed bugs in various escape functions, unfortunately as part of that I introduced a new bug in PQescapeLiteral()/PQescapeIdentifier(). The bug is that I made PQescapeInternal() just use strlen(), rather than taking the specified input length into account. That's bad, because it can lead to including input that wasn't intended to be included (in case len is shorter than null termination of the string) and because it can lead to reading invalid memory if the input string is not null terminated. Expand test_escape to this kind of bug: a) for escape functions with length support, append data that should not be escaped and check that it is not b) add valgrind requests to detect access of bytes that should not be touched Author: Tom Lane <tgl@sss.pgh.pa.us> Author: Andres Freund <andres@anarazel.de Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/Z64jD3u46gObCo1p@pryzbyj2023 Backpatch: 13
* Fix handling of invalidly encoded data in escaping functionsAndres Freund2025-02-10
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously invalidly encoded input to various escaping functions could lead to the escaped string getting incorrectly parsed by psql. To be safe, escaping functions need to ensure that neither invalid nor incomplete multi-byte characters can be used to "escape" from being quoted. Functions which can report errors now return an error in more cases than before. Functions that cannot report errors now replace invalid input bytes with a byte sequence that cannot be used to escape the quotes and that is guaranteed to error out when a query is sent to the server. The following functions are fixed by this commit: - PQescapeLiteral() - PQescapeIdentifier() - PQescapeString() - PQescapeStringConn() - fmtId() - appendStringLiteral() Reported-by: Stephen Fewer <stephen_fewer@rapid7.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Backpatch-through: 13 Security: CVE-2025-1094
* Update copyright for 2025Bruce Momjian2025-01-01
| | | | Backpatch-through: 13
* libpq: Some message style normalizationPeter Eisentraut2024-06-13
|
* Support retrieval of results in chunks with libpq.Tom Lane2024-04-06
| | | | | | | | | | | | | | | | | | | | | | This patch generalizes libpq's existing single-row mode to allow individual partial-result PGresults to contain up to N rows, rather than always one row. This reduces malloc overhead compared to plain single-row mode, and it is very useful for psql's FETCH_COUNT feature, since otherwise we'd have to add code (and cycles) to either merge single-row PGresults into a bigger one or teach psql's results-printing logic to accept arrays of PGresults. To avoid API breakage, PQsetSingleRowMode() remains the same, and we add a new function PQsetChunkedRowsMode() to invoke the more general case. Also, PGresults obtained the old way continue to carry the PGRES_SINGLE_TUPLE status code, while if PQsetChunkedRowsMode() is used then their status code is PGRES_TUPLES_CHUNK. The underlying logic is the same either way, though. Daniel Vérité, reviewed by Laurenz Albe and myself (and whacked around a bit by me, so any remaining bugs are my fault) Discussion: https://postgr.es/m/CAKZiRmxsVTkO928CM+-ADvsMyePmU3L9DQCa9NwqjvLPcEe5QA@mail.gmail.com
* Remove some comments related to pqPipelineSync() and PQsendPipelineSync()Michael Paquier2024-01-17
| | | | | | | | | | | These comments explained how these functions behave internally, and the equivalent is described in the documentation section dedicated to the pipeline mode of libpq. Let's remove these comments, getting rid of the duplication with the docs. Reported-by: Álvaro Herrera Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/202401150949.wq7ynlmqxphy@alvherre.pgsql
* Don't test already-referenced pointer for nullnessAlvaro Herrera2024-01-16
| | | | | | | | | | | | | | | | | | | Commit b8ba7344e9eb added in PQgetResult a derefence to a pointer returned by pqPrepareAsyncResult(), before some other code that was already testing that pointer for nullness. But since commit 618c16707a6d (in Postgres 15), pqPrepareAsyncResult() doesn't ever return NULL (a statically-allocated result is returned if OOM). So in branches 15 and up, we can remove the redundant pointer check with no harm done. However, in branch 14, pqPrepareAsyncResult() can indeed return NULL if it runs out of memory. Fix things there by adding a null pointer check before dereferencing the pointer. This should hint Coverity that the preexisting check is not redundant but necessary. Backpatch to 14, like b8ba7344e9eb. Per Coverity.
* libpq: Add PQsendPipelineSync()Michael Paquier2024-01-16
| | | | | | | | | | | | | | | | This new function is equivalent to PQpipelineSync(), except that it does not flush anything to the server except if the size threshold of the output buffer is reached; the user must subsequently call PQflush() instead. Its purpose is to reduce the system call overhead of pipeline mode, by giving to applications more control over the timing of the flushes when manipulating commands in pipeline mode. Author: Anton Kirilov Reviewed-by: Jelte Fennema-Nio, Robert Haas, Álvaro Herrera, Denis Laxalde, Michael Paquier Discussion: https://postgr.es/m/CACV6eE5arHFZEA717=iKEa_OewpVFfWJOmsOdGrqqsr8CJVfWQ@mail.gmail.com
* Fix a typo and some doc indentation related to libpq pipeline functionsMichael Paquier2024-01-16
| | | | | Noticed while reviewing the area for a different patch. This is cosmetic, so no backpatch is done.
* Update copyright for 2024Bruce Momjian2024-01-03
| | | | | | | | Reported-by: Michael Paquier Discussion: https://postgr.es/m/ZZKTDPxBBMt3C0J9@paquier.xyz Backpatch-through: 12
* Fix handling of errors in libpq pipelinesAlvaro Herrera2023-12-05
| | | | | | | | | | | | | | | | | | The logic to keep the libpq command queue in sync with queries that have been processed had a bug when errors were returned for reasons other than problems in queries -- for example, when a connection is lost. We incorrectly consumed an element from the command queue every time, but this is wrong and can lead to the queue becoming empty ahead of time, leading to later malfunction: PQgetResult would return nothing, potentially causing the calling application to enter a busy loop. Fix by making the SYNC queue element a barrier that can only be consumed when a SYNC message is received. Backpatch to 14. Reported by: Иван Трофимов (Ivan Trofimov) <i.trofimow@yandex.ru> Discussion: https://postgr.es/m/17948-fcace7557e449957@postgresql.org
* Call pqPipelineFlush from PQsendFlushRequestAlvaro Herrera2023-11-08
| | | | | | | | | | | | | | | | | | When PQsendFlushRequest() was added by commit 69cf1d5429d4, we argued against adding a PQflush() call in it[1]. This is still the right decision: if the user wants a flush to occur, they can just call that. However, we failed to realize that the message bytes could still be given to the kernel for transmitting when this can be made without blocking. That's what pqPipelineFlush() does, and it is done for every single other message type sent by libpq, so do that. (When the socket is in blocking mode this may indeed block, but that's what all the other libpq message-sending routines do, too.) [1] https://www.postgresql.org/message-id/202106252352.5ca4byasfun5%40alvherre.pgsql Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQTxZRevRWkKodE-SnJk1Yfm4eKT+8E4Cyq3MJ9YKTnNew@mail.gmail.com
* Fix comment of PQputCopyEnd()Michael Paquier2023-08-30
| | | | | | | | | The comment describing the error codes of this routine mentioned 0 as a possible value, but this error code has never been used. Author: Junwang Zhao Reviewed-by: Aleksander Alekseev Discussion: https://postgr.es/m/CAEG8a3Jt5KwMNr+_S6VN68rog4HeoG6ELvPQO8kZNQTeJeQ=rQ@mail.gmail.com
* Introduce macros for protocol characters.Nathan Bossart2023-08-22
| | | | | | | | | | | This commit introduces descriptively-named macros for the identifiers used in wire protocol messages. These new macros are placed in a new header file so that they can be easily used by third-party code. Author: Dave Cramer Reviewed-by: Alvaro Herrera, Tatsuo Ishii, Peter Smith, Robert Haas, Tom Lane, Peter Eisentraut, Michael Paquier Discussion: https://postgr.es/m/CADK3HHKbBmK-PKf1bPNFoMC%2BoBt%2BpD9PH8h5nvmBQskEHm-Ehw%40mail.gmail.com
* Remove --disable-thread-safety and related code.Thomas Munro2023-07-12
| | | | | | | | | | | | | | | | All supported computers have either POSIX or Windows threads, and we no longer have any automated testing of --disable-thread-safety. We define a vestigial ENABLE_THREAD_SAFETY macro to 1 in ecpg_config.h in case it is useful, but we no longer test it anywhere in PostgreSQL code, and associated dead code paths are removed. The Meson and perl-based Windows build scripts never had an equivalent build option. Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CA%2BhUKGLtmexrpMtxBRLCVePqV_dtWG-ZsEbyPrYc%2BNBB2TkNsw%40mail.gmail.com
* libpq: Add support for Close on portals and statementsMichael Paquier2023-07-04
| | | | | | | | | | | | | | | | | | | | | | | | The following routines are added to libpq: PGresult *PQclosePrepared(PGconn *conn, const char *stmt); PGresult *PQclosePortal(PGconn *conn, const char *portal); int PQsendClosePrepared(PGconn *conn, const char *stmt); int PQsendClosePortal(PGconn *conn, const char *portal); The "send" routines are non-blocking versions of the two others. Close messages are part of the protocol but they did not have a libpq implementation. And, having these routines is for instance useful with connection poolers as these can detect more easily Close messages than DEALLOCATE queries. The implementation takes advantage of what the Describe routines rely on for portals and statements. Some regression tests are added in libpq_pipeline, for the four new routines, by closing portals and statements created already by the tests. Author: Jelte Fennema Reviewed-by: Jian He, Michael Paquier Discussion: https://postgr.es/m/CAGECzQTb4xFAopAVokudB+L62Kt44mNAL4Z9zZ7UTrs1TRFvWA@mail.gmail.com
* Pre-beta mechanical code beautification.Tom Lane2023-05-19
| | | | | | | | | | | | | | | Run pgindent, pgperltidy, and reformat-dat-files. This set of diffs is a bit larger than typical. We've updated to pg_bsd_indent 2.1.2, which properly indents variable declarations that have multi-line initialization expressions (the continuation lines are now indented one tab stop). We've also updated to perltidy version 20230309 and changed some of its settings, which reduces its desire to add whitespace to lines to make assignments etc. line up. Going forward, that should make for fewer random-seeming changes to existing code. Discussion: https://postgr.es/m/20230428092545.qfb3y5wcu4cm75ur@alvherre.pgsql
* Make SCRAM iteration count configurableDaniel Gustafsson2023-03-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace the hardcoded value with a GUC such that the iteration count can be raised in order to increase protection against brute-force attacks. The hardcoded value for SCRAM iteration count was defined to be 4096, which is taken from RFC 7677, so set the default for the GUC to 4096 to match. In RFC 7677 the recommendation is at least 15000 iterations but 4096 is listed as a SHOULD requirement given that it's estimated to yield a 0.5s processing time on a mobile handset of the time of RFC writing (late 2015). Raising the iteration count of SCRAM will make stored passwords more resilient to brute-force attacks at a higher computational cost during connection establishment. Lowering the count will reduce computational overhead during connections at the tradeoff of reducing strength against brute-force attacks. There are however platforms where even a modest iteration count yields a too high computational overhead, with weaker password encryption schemes chosen as a result. In these situations, SCRAM with a very low iteration count still gives benefits over weaker schemes like md5, so we allow the iteration count to be set to one at the low end. The new GUC is intentionally generically named such that it can be made to support future SCRAM standards should they emerge. At that point the value can be made into key:value pairs with an undefined key as a default which will be backwards compatible with this. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Jonathan S. Katz <jkatz@postgresql.org> Discussion: https://postgr.es/m/F72E7BC7-189F-4B17-BF47-9735EB72C364@yesql.se
* Update copyright for 2023Bruce Momjian2023-01-02
| | | | Backpatch-through: 11
* libpq error message refactoring, part 2Peter Eisentraut2022-11-15
| | | | | | | This applies the new APIs to the code. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com
* libpq: Reset singlerow flag correctly in pipeline modeAlvaro Herrera2022-10-14
| | | | | | | | | | | | | | | When a query whose results were requested in single-row mode is the last in the queue by the time those results are being read, the single-row flag was not being reset, because we were returning early from pqPipelineProcessQueue. Move that stanza up so that the flag is always reset at the end of sending that query's results. Add a test for the situation. Backpatch to 14. Author: Denis Laxalde <denis.laxalde@dalibo.com> Discussion: https://postgr.es/m/01af18c5-dacc-a8c8-07ee-aecc7650c3e8@dalibo.com
* Remove PQsendQuery support in pipeline modeAlvaro Herrera2022-09-23
| | | | | | | | | | | | | | | | | | The extended query protocol implementation I added in commit acb7e4eb6b1c has bugs when used in pipeline mode. Rather than spend more time trying to fix it, remove that code and make the function rely on simple query protocol only, meaning it can no longer be used in pipeline mode. Users can easily change their applications to use PQsendQueryParams instead. We leave PQsendQuery in place for Postgres 14, just in case somebody is using it and has not hit the mentioned bugs; but we should recommend that it not be used. Backpatch to 15. Per bug report from Gabriele Varrazzo. Discussion: https://postgr.es/m/CA+mi_8ZGSQNmW6-mk_iSR4JZB_LJ4ww3suOF+1vGNs3MrLsv4g@mail.gmail.com
* Harmonize more parameter names in bulk.Peter Geoghegan2022-09-20
| | | | | | | | | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions in optimizer, parser, utility, libpq, and "commands" code, as well as in remaining library code. Do the same for all code related to frontend programs (with the exception of pg_dump/pg_dumpall related code). Like other recent commits that cleaned up function parameter names, this commit was written with help from clang-tidy. Later commits will handle ecpg and pg_dump/pg_dumpall. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
* Add missing bad-PGconn guards in libpq entry points.Tom Lane2022-08-15
| | | | | | | | | | | | | There's a convention that externally-visible libpq functions should check for a NULL PGconn pointer, and fail gracefully instead of crashing. PQflush() and PQisnonblocking() didn't get that memo though. Also add a similar check to PQdefaultSSLKeyPassHook_OpenSSL; while it's not clear that ordinary usage could reach that with a null conn pointer, it's cheap enough to check, so let's be consistent. Daniele Varrazzo and Tom Lane Discussion: https://postgr.es/m/CA+mi_8Zm_mVVyW1iNFgyMd9Oh0Nv8-F+7Y3-BqwMgTMHuo_h2Q@mail.gmail.com
* Plug memory leakAlvaro Herrera2022-07-13
| | | | | | | Commit 054325c5eeb3 created a memory leak in PQsendQueryInternal in case an error occurs while sending the message. Repair. Backpatch to 14, like that commit. Reported by Coverity.
* libpq: Improve idle state handling in pipeline modeAlvaro Herrera2022-07-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We were going into IDLE state too soon when executing queries via PQsendQuery in pipeline mode, causing several scenarios to misbehave in different ways -- most notably, as reported by Daniele Varrazzo, that a warning message is produced by libpq: message type 0x33 arrived from server while idle But it is also possible, if queries are sent and results consumed not in lockstep, for the expected mediating NULL result values from PQgetResult to be lost (a problem which has not been reported, but which is more serious). Fix this by introducing two new concepts: one is a command queue element PGQUERY_CLOSE to tell libpq to wait for the CloseComplete server response to the Close message that is sent by PQsendQuery. Because the application is not expecting any PGresult from this, the mechanism to consume it is a bit hackish. The other concept, authored by Horiguchi-san, is a PGASYNC_PIPELINE_IDLE state for libpq's state machine to differentiate "really idle" from merely "the idle state that occurs in between reading results from the server for elements in the pipeline". This makes libpq not go fully IDLE when the libpq command queue contains entries; in normal cases, we only go IDLE once at the end of the pipeline, when the server response to the final SYNC message is received. (However, there are corner cases it doesn't fix, such as terminating the query sequence by PQsendFlushRequest instead of PQpipelineSync; this sort of scenario is what requires PGQUERY_CLOSE bit above.) This last bit helps make the libpq state machine clearer; in particular we can get rid of an ugly hack in pqParseInput3 to avoid considering IDLE as such when the command queue contains entries. A new test mode is added to libpq_pipeline.c to tickle some related problematic cases. Reported-by: Daniele Varrazzo <daniele.varrazzo@gmail.com> Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CA+mi_8bvD0_CW3sumgwPvWdNzXY32itoG_16tDYRu_1S2gV2iw@mail.gmail.com
* Remove redundant null pointer checks before PQclear and PQconninfoFreePeter Eisentraut2022-07-03
| | | | | | | | These functions already had the free()-like behavior of handling null pointers as a no-op. But it wasn't documented, so add it explicitly to the documentation, too. Discussion: https://www.postgresql.org/message-id/flat/dac5d2d0-98f5-94d9-8e69-46da2413593d%40enterprisedb.com
* Remove redundant null pointer checks before free()Peter Eisentraut2022-07-03
| | | | | | | | | | Per applicable standards, free() with a null pointer is a no-op. Systems that don't observe that are ancient and no longer relevant. Some PostgreSQL code already required this behavior, so this change does not introduce any new requirements, just makes the code more consistent. Discussion: https://www.postgresql.org/message-id/flat/dac5d2d0-98f5-94d9-8e69-46da2413593d%40enterprisedb.com
* Fix missed cases in libpq's error handling.Tom Lane2022-04-21
| | | | | | | | | | | | | | | | | | | | Commit 618c16707 invented an "error_result" flag in PGconn, which intends to represent the state that we have an error condition and need to build a PGRES_FATAL_ERROR PGresult from the message text in conn->errorMessage, but have not yet done so. (Postponing construction of the error object simplifies dealing with out-of-memory conditions and with concatenation of messages for multiple errors.) For nearly all purposes, this "virtual" PGresult object should act the same as if it were already materialized. But a couple of places in fe-protocol3.c didn't get that memo, and were only testing conn->result as they used to, without also checking conn->error_result. In hopes of reducing the probability of similar mistakes in future, I invented a pgHavePendingResult() macro that includes both tests. Per report from Peter Eisentraut. Discussion: https://postgr.es/m/b52277b9-fa66-b027-4a37-fb8989c73ff8@enterprisedb.com
* Add support for MERGE SQL commandAlvaro Herrera2022-03-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | MERGE performs actions that modify rows in the target table using a source table or query. MERGE provides a single SQL statement that can conditionally INSERT/UPDATE/DELETE rows -- a task that would otherwise require multiple PL statements. For example, MERGE INTO target AS t USING source AS s ON t.tid = s.sid WHEN MATCHED AND t.balance > s.delta THEN UPDATE SET balance = t.balance - s.delta WHEN MATCHED THEN DELETE WHEN NOT MATCHED AND s.delta > 0 THEN INSERT VALUES (s.sid, s.delta) WHEN NOT MATCHED THEN DO NOTHING; MERGE works with regular tables, partitioned tables and inheritance hierarchies, including column and row security enforcement, as well as support for row and statement triggers and transition tables therein. MERGE is optimized for OLTP and is parameterizable, though also useful for large scale ETL/ELT. MERGE is not intended to be used in preference to existing single SQL commands for INSERT, UPDATE or DELETE since there is some overhead. MERGE can be used from PL/pgSQL. MERGE does not support targetting updatable views or foreign tables, and RETURNING clauses are not allowed either. These limitations are likely fixable with sufficient effort. Rewrite rules are also not supported, but it's not clear that we'd want to support them. Author: Pavan Deolasee <pavan.deolasee@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Author: Amit Langote <amitlangote09@gmail.com> Author: Simon Riggs <simon.riggs@enterprisedb.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Andres Freund <andres@anarazel.de> (earlier versions) Reviewed-by: Peter Geoghegan <pg@bowt.ie> (earlier versions) Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier versions) Reviewed-by: Japin Li <japinli@hotmail.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CANP8+jKitBSrB7oTgT9CY2i1ObfOt36z0XMraQc+Xrz8QB0nXA@mail.gmail.com Discussion: https://postgr.es/m/CAH2-WzkJdBuxj9PO=2QaO9-3h3xGbQPZ34kJH=HukRekwM-GZg@mail.gmail.com Discussion: https://postgr.es/m/20201231134736.GA25392@alvherre.pgsql
* Adjust interaction of libpq pipeline mode with errorMessage resets.Tom Lane2022-02-28
| | | | | | | | | | | | | | | | | Since commit ffa2e4670, libpq resets conn->errorMessage only when starting a new query. However, the later introduction of pipelining requires a further refinement: the "start of query" isn't necessarily when it's submitted to PQsendQueryStart. If we clear at that point then we risk dropping text for an error that the application has not noticed yet. Instead, when queuing a query while a previous query is still in flight, leave errorMessage alone; reset it when we begin to process the next query in pqPipelineProcessQueue. Perhaps this should be back-patched to v14 where ffa2e4670 came in. However I'm uncertain about whether it interacts with 618c16707. In the absence of user complaints, leave v14 alone. Discussion: https://postgr.es/m/1421785.1645723238@sss.pgh.pa.us
* Rearrange libpq's error reporting to avoid duplicated error text.Tom Lane2022-02-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit ffa2e4670, libpq accumulates text in conn->errorMessage across a whole query cycle. In some situations, we may report more than one error event within a cycle: the easiest case to reach is where we report a FATAL error message from the server, and then a bit later we detect loss of connection. Since, historically, each error PGresult bears the entire content of conn->errorMessage, this results in duplication of the FATAL message in any output that concatenates the contents of the PGresults. Accumulation in errorMessage still seems like a good idea, especially in view of the number of places that did ad-hoc error concatenation before ffa2e4670. So to fix this, let's track how much of conn->errorMessage has been read out into error PGresults, and only include new text in later PGresults. The tricky part of that is to be sure that we never discard an error PGresult once made (else we'd risk dropping some text, a problem much worse than duplication). While libpq formerly did that in some code paths, a little bit of rearrangement lets us postpone making an error PGresult at all until we are about to return it. A side benefit of that postponement is that it now becomes practical to return a dummy static PGresult in cases where we hit out-of-memory while trying to manufacture an error PGresult. This eliminates the admittedly-very-rare case where we'd return NULL from PQgetResult, indicating successful query completion, even though what actually happened was an OOM failure. Discussion: https://postgr.es/m/ab4288f8-be5c-57fb-2400-e3e857f53e46@enterprisedb.com
* Don't let libpq "event" procs break the state of PGresult objects.Tom Lane2022-02-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | As currently implemented, failure of a PGEVT_RESULTCREATE callback causes the PGresult to be converted to an error result. This is intellectually inconsistent (shouldn't a failing callback likewise prevent creation of the error result? what about side-effects on the behavior seen by other event procs? why does PQfireResultCreateEvents act differently from PQgetResult?), but more importantly it destroys any promises we might wish to make about the behavior of libpq in nontrivial operating modes, such as pipeline mode. For example, it's not possible to promise that PGRES_PIPELINE_SYNC results will be returned if an event callback fails on those. With this definition, expecting applications to behave sanely in the face of possibly-failing callbacks seems like a very big lift. Hence, redefine the result of a callback failure as being simply that that event procedure won't be called any more for this PGresult (which was true already). Event procedures can still signal failure back to the application through out-of-band mechanisms, for example via their passthrough arguments. Similarly, don't let failure of a PGEVT_RESULTCOPY callback prevent PQcopyResult from succeeding. That definition allowed a misbehaving event proc to break single-row mode (our sole internal use of PQcopyResult), and it probably had equally deleterious effects for outside uses. Discussion: https://postgr.es/m/3185105.1644960083@sss.pgh.pa.us
* Fix thinko in PQisBusy().Tom Lane2022-02-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit 1f39a1c06 I made PQisBusy consider conn->write_failed, but that is now looking like complete brain fade. In the first place, the logic is quite wrong: it ought to be like "and not" rather than "or". This meant that once we'd gotten into a write_failed state, PQisBusy would always return true, probably causing the calling application to iterate its loop until PQconsumeInput returns a hard failure thanks to connection loss. That's not what we want: the intended behavior is to return an error PGresult, which the application probably has much cleaner support for. But in the second place, checking write_failed here seems like the wrong thing anyway. The idea of the write_failed mechanism is to postpone handling of a write failure until we've read all we can from the server; so that flag should not interfere with input-processing behavior. (Compare 7247e243a.) What we *should* check for is status = CONNECTION_BAD, ie, socket already closed. (Most places that close the socket don't touch asyncStatus, but they do reset status.) This primarily ensures that if PQisBusy() returns true then there is an open socket, which is assumed by several call sites in our own code, and probably other applications too. While at it, fix a nearby thinko in libpq's my_sock_write: we should only consult errno for res < 0, not res == 0. This is harmless since pqsecure_raw_write would force errno to zero in such a case, but it still could confuse readers. Noted by Andres Freund. Backpatch to v12 where 1f39a1c06 came in. Discussion: https://postgr.es/m/20220211011025.ek7exh6owpzjyudn@alap3.anarazel.de
* Update copyright for 2022Bruce Momjian2022-01-07
| | | | Backpatch-through: 10
* Improve libpq's handling of OOM during error message construction.Tom Lane2021-07-29
| | | | | | | | | | | | | | | | | | | | | | | | | Commit ffa2e4670 changed libpq so that multiple error reports occurring during one operation (a connection attempt or query) are accumulated in conn->errorMessage, where before new ones usually replaced any prior error. At least in theory, that makes us more vulnerable to running out of memory for the errorMessage buffer. If it did happen, the user would be left with just an empty-string error report, which is pretty unhelpful. We can improve this by relying on pqexpbuffer.c's existing "broken buffer" convention to track whether we've hit OOM for the current operation's error string, and then substituting a constant "out of memory" string in the small number of places where the errorMessage is read out. While at it, apply the same method to similar OOM cases in pqInternalNotice and pqGetErrorNotice3. Back-patch to v14 where ffa2e4670 came in. In principle this could go back further; but in view of the lack of field reports, the hazard seems negligible in older branches. Discussion: https://postgr.es/m/530153.1627425648@sss.pgh.pa.us
* libpq: Fix sending queries in pipeline aborted stateAlvaro Herrera2021-07-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When sending queries in pipeline mode, we were careless about leaving the connection in the right state so that PQgetResult would behave correctly; trying to read further results after sending a query after having read a result with an error would sometimes hang. Fix by ensuring internal libpq state is changed properly. All the state changes were being done by the callers of pqAppendCmdQueueEntry(); it would have become too repetitious to have this logic in each of them, so instead put it all in that function and relieve callers of the responsibility. Add a test to verify this case. Without the code fix, this new test hangs sometimes. Also, document that PQisBusy() would return false when no queries are pending result. This is not intuitively obvious, and NULL would be obtained by calling PQgetResult() at that point, which is confusing. Wording by Boris Kolpackov. In passing, fix bogus use of "false" to mean "0", per Ranier Vilela. Backpatch to 14. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Boris Kolpackov <boris@codesynthesis.com> Discussion: https://postgr.es/m/boris.20210624103805@codesynthesis.com
* Fix libpq state machine in pipeline modeAlvaro Herrera2021-06-29
| | | | | | | | | | | | | | | | | The original coding required that PQpipelineSync had been called before the first call to PQgetResult, and failure to do that would result in an unexpected NULL result being returned. Fix by setting the right state when a query is sent, rather than leaving it unchanged and having PQpipelineSync apply the necessary state change. A new test case to verify the behavior is added, which relies on the new PQsendFlushRequest() function added by commit a7192326c74d. Backpatch to 14, where pipeline mode was added. Reported-by: Boris Kolpackov <boris@codesynthesis.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/boris.20210616110321@codesynthesis.com
* Add PQsendFlushRequest to libpqAlvaro Herrera2021-06-29
| | | | | | | | | | | | | | | | | This new libpq function allows the application to send an 'H' message, which instructs the server to flush its outgoing buffer. This hasn't been needed so far because the Sync message already requests a buffer; and I failed to realize that this was needed in pipeline mode because PQpipelineSync also causes the buffer to be flushed. However, sometimes it is useful to request a flush without establishing a synchronization point. Backpatch to 14, where pipeline mode was introduced in libpq. Reported-by: Boris Kolpackov <boris@codesynthesis.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202106252350.t76x73nt643j@alvherre.pgsql
* Add 'Portal Close' message to pipelined PQsendQuery()Alvaro Herrera2021-06-11
| | | | | | | | | | | | Commit acb7e4eb6b1c added a new implementation for PQsendQuery so that it works in pipeline mode (by using extended query protocol), but it behaves differently from the 'Q' message (in simple query protocol) used by regular implementation: the new one doesn't close the unnamed portal. Change the new code to have identical behavior to the old. Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202106072107.d4i55hdscxqj@alvherre.pgsql
* Adjust batch size in postgres_fdw to not use too many parametersTomas Vondra2021-06-08
| | | | | | | | | | | | | | | | The FE/BE protocol identifies parameters with an Int16 index, which limits the maximum number of parameters per query to 65535. With batching added to postges_fdw this limit is much easier to hit, as the whole batch is essentially a single query, making this error much easier to hit. The failures are a bit unpredictable, because it also depends on the number of columns in the query. So instead of just failing, this patch tweaks the batch_size to not exceed the maximum number of parameters. Reported-by: Hou Zhijie <houzj.fnst@cn.fujitsu.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/OS0PR01MB571603973C0AC2874AD6BF2594299%40OS0PR01MB5716.jpnprd01.prod.outlook.com
* Improve PQtrace() output formatAlvaro Herrera2021-03-30
| | | | | | | | | | | | | | | | | | | | | | Transform the PQtrace output format from its ancient (and mostly useless) byte-level output format to a logical-message-level output, making it much more usable. This implementation allows the printing code to be written (as it indeed was) by looking at the protocol documentation, which gives more confidence that the three (docs, trace code and actual code) actually match. Author: 岩田 彩 (Aya Iwata) <iwata.aya@fujitsu.com> Reviewed-by: 綱川 貴之 (Takayuki Tsunakawa) <tsunakawa.takay@fujitsu.com> Reviewed-by: Kirk Jamison <k.jamison@fujitsu.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: 黒田 隼人 (Hayato Kuroda) <kuroda.hayato@fujitsu.com> Reviewed-by: "Nagaura, Ryohei" <nagaura.ryohei@jp.fujitsu.com> Reviewed-by: Ryo Matsumura <matsumura.ryo@fujitsu.com> Reviewed-by: Greg Nancarrow <gregn4422@gmail.com> Reviewed-by: Jim Doty <jdoty@pivotal.io> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/71E660EB361DF14299875B198D4CE5423DE3FBA4@g01jpexmbkw25
* Fix new memory leaks in libpqAlvaro Herrera2021-03-21
| | | | | | My oversight in commit 9aa491abbf07. Per coverity.
* Implement pipeline mode in libpqAlvaro Herrera2021-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | Pipeline mode in libpq lets an application avoid the Sync messages in the FE/BE protocol that are implicit in the old libpq API after each query. The application can then insert Sync at its leisure with a new libpq function PQpipelineSync. This can lead to substantial reductions in query latency. Co-authored-by: Craig Ringer <craig.ringer@enterprisedb.com> Co-authored-by: Matthieu Garrigues <matthieu.garrigues@gmail.com> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Aya Iwata <iwata.aya@jp.fujitsu.com> Reviewed-by: Daniel Vérité <daniel@manitou-mail.org> Reviewed-by: David G. Johnston <david.g.johnston@gmail.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Kirk Jamison <k.jamison@fujitsu.com> Reviewed-by: Michael Paquier <michael.paquier@gmail.com> Reviewed-by: Nikhil Sontakke <nikhils@2ndquadrant.com> Reviewed-by: Vaishnavi Prabakaran <VaishnaviP@fast.au.fujitsu.com> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=B4dMMZw@mail.gmail.com Discussion: https://postgr.es/m/CAJkzx4T5E-2cQe3dtv2R78dYFvz+in8PY7A8MArvLhs_pg75gg@mail.gmail.com
* Remove server and libpq support for old FE/BE protocol version 2.Heikki Linnakangas2021-03-04
| | | | | | | | | | | | | | | | | Protocol version 3 was introduced in PostgreSQL 7.4. There shouldn't be many clients or servers left out there without version 3 support. But as a courtesy, I kept just enough of the old protocol support that we can still send the "unsupported protocol version" error in v2 format, so that old clients can display the message properly. Likewise, libpq still understands v2 ErrorResponse messages when establishing a connection. The impetus to do this now is that I'm working on a patch to COPY FROM, to always prefetch some data. We cannot do that safely with the old protocol, because it requires parsing the input one byte at a time to detect the end-of-copy marker. Reviewed-by: Tom Lane, Alvaro Herrera, John Naylor Discussion: https://www.postgresql.org/message-id/9ec25819-0a8a-d51a-17dc-4150bb3cca3b%40iki.fi
* Extend the abilities of libpq's target_session_attrs parameter.Tom Lane2021-03-02
| | | | | | | | | | | | | | | | | | | | | | In addition to the existing options of "any" and "read-write", we now support "read-only", "primary", "standby", and "prefer-standby". "read-write" retains its previous meaning of "transactions are read-write by default", and "read-only" inverts that. The other three modes test specifically for hot-standby status, which is not quite the same thing. (Setting default_transaction_read_only on a primary server renders it read-only to this logic, but not a standby.) Furthermore, if talking to a v14 or later server, no extra network round trip is needed to detect the session's status; the GUC_REPORT variables delivered by the server are enough. When talking to an older server, a SHOW or SELECT query is issued to detect session read-only-ness or server hot-standby state, as needed. Haribabu Kommi, Greg Nancarrow, Vignesh C, Tom Lane; reviewed at various times by Laurenz Albe, Takayuki Tsunakawa, Peter Smith. Discussion: https://postgr.es/m/CAF3+xM+8-ztOkaV9gHiJ3wfgENTq97QcjXQt+rbFQ6F7oNzt9A@mail.gmail.com
* In libpq, always append new error messages to conn->errorMessage.Tom Lane2021-01-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we had an undisciplined mish-mash of printfPQExpBuffer and appendPQExpBuffer calls to report errors within libpq. This commit establishes a uniform rule that appendPQExpBuffer[Str] should be used. conn->errorMessage is reset only at the start of an application request, and then accumulates messages till we're done. We can remove no less than three different ad-hoc mechanisms that were used to get the effect of concatenation of error messages within a sequence of operations. Although this makes things quite a bit cleaner conceptually, the main reason to do it is to make the world safer for the multiple-target-host feature that was added awhile back. Previously, there were many cases in which an error occurring during an individual host connection attempt would wipe out the record of what had happened during previous attempts. (The reporting is still inadequate, in that it can be hard to tell which host got the failure, but that seems like a matter for a separate commit.) Currently, lo_import and lo_export contain exceptions to the "never use printfPQExpBuffer" rule. If we changed them, we'd risk reporting an incidental lo_close failure before the actual read or write failure, which would be confusing, not least because lo_close happened after the main failure. We could improve this by inventing an internal version of lo_close that doesn't reset the errorMessage; but we'd also need a version of PQfn() that does that, and it didn't quite seem worth the trouble for now. Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
* Update copyright for 2021Bruce Momjian2021-01-02
| | | | Backpatch-through: 9.5