Commit Graph

1810 Commits

Author SHA1 Message Date
Thomas Munro bcc8b14ef6 Remove configure probe for sockaddr_in6 and require AF_INET6.
SUSv3 <netinet/in.h> defines struct sockaddr_in6, and all targeted Unix
systems have it.  Windows has it in <ws2ipdef.h>.  Remove the configure
probe, the macro and a small amount of dead code.

Also remove a mention of IPv6-less builds from the documentation, since
there aren't any.

This is similar to commits f5580882 and 077bf2f2 for Unix sockets.  Even
though AF_INET6 is an "optional" component of SUSv3, there are no known
modern operating system without it, and it seems even less likely to be
omitted from future systems than AF_UNIX.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-26 10:18:30 +12:00
Peter Eisentraut 28ec316787 libpq code should use libpq_gettext(), not _()
Fix some wrong use and install a safeguard against future mistakes.
2022-08-25 20:46:58 +02:00
Peter Eisentraut 869e56a399 Message style adjustment 2022-08-23 21:51:07 +02:00
Andres Freund 1d77afefbd Don't define FRONTEND for libpq
Not needed anymore after 7143b3e821.

Discussion: https://postgr.es/m/20220820194550.725755r6fj2ro3rx@awork3.anarazel.de
2022-08-22 20:39:30 -07:00
Peter Eisentraut 1509abe2c5 Remove remaining mentions of UNSAFE_STAT_OK
The last use was removed by bed90759fc.

Discussion: https://www.postgresql.org/message-id/flat/01229f9a-b358-d71e-31ae-4c0855d73cbc%40enterprisedb.com
2022-08-20 13:53:21 +02:00
Thomas Munro f340f97a13 mstcpip.h is not missing on MinGW.
Remove a small difference between MinGW and MSVC builds which isn't
needed for modern MinGW, noticed in passing.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-18 16:31:11 +12:00
Thomas Munro 2492fe49dc Remove configure probe for netinet/tcp.h.
<netinet/tcp.h> is in SUSv3 and all targeted Unix systems have it.
For Windows, we can provide a stub include file, to avoid some #ifdef
noise.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-18 16:31:11 +12:00
Tom Lane bb9237a129 Add missing bad-PGconn guards in libpq entry points.
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
2022-08-15 15:40:07 -04:00
Thomas Munro 52ea29045b Remove configure probe for gethostbyname_r.
It was only used by src/port/getaddrinfo.c, removed by the previous
commit.

Discussion: https://postgr.es/m/CA%2BhUKGJFLPCtAC58EAimF6a6GPw30TU_59FUY%3DGWB_kC%3DJEmVQ%40mail.gmail.com
2022-08-14 09:57:48 +12:00
Thomas Munro 5579388d2d Remove replacement code for getaddrinfo.
SUSv3, all targeted Unixes and modern Windows have getaddrinfo() and
related interfaces.  Drop the replacement implementation, and adjust
some headers slightly to make sure that the APIs are visible everywhere
using standard POSIX headers and names.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 09:53:28 +12:00
Thomas Munro f558088285 Remove HAVE_UNIX_SOCKETS.
Since HAVE_UNIX_SOCKETS is now defined unconditionally, remove the macro
and drop a small amount of dead code.

The last known systems not to have them (as far as I know at least) were
QNX, which we de-supported years ago, and Windows, which now has them.

If a new OS ever shows up with the POSIX sockets API but without working
AF_UNIX, it'll presumably still be able to compile the code, and fail at
runtime with an unsupported address family error.  We might want to
consider adding a HINT that you should turn off the option to use it if
your network stack doesn't support it at that point, but it doesn't seem
worth making the relevant code conditional at compile time.

Also adjust a couple of places in the docs and comments that referred to
builds without Unix-domain sockets, since there aren't any.  Windows
still gets a special mention in those places, though, because we don't
try to use them by default there yet.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 08:46:53 +12:00
Thomas Munro 7e50b4e3c5 Remove configure probe for sys/select.h.
<sys/select.h> is in SUSv3 and every targeted Unix system has it.
Provide an empty header in src/include/port/win32 so that we can
include it unguarded even on Windows.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 00:09:47 +12:00
Alvaro Herrera 0f39b70a6f
Fix [install]check in interfaces/libpq/Makefile
The common recipe when TAP tests are disabled doesn't work, because the
libpq-specific recipe wants to define the PATH environment variable, so
the starting '@' is misinterpreted as part of the command instead of
silencing said command.

Fix by setting the environment variable in a way that doesn't interfere
with the recipe.

Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220720172321.GL12702@telsasoft.com
2022-07-22 20:15:11 +02:00
Tom Lane 54ba2f0623 Revert "Use wildcards instead of manually-maintained file lists in */nls.mk."
This reverts commit 617d691412.
While I still think the basic idea is attractive, we need to sort
out what happens with built .c files, and there also seem to be
VPATH issues.
2022-07-13 14:29:10 -04:00
Tom Lane 617d691412 Use wildcards instead of manually-maintained file lists in */nls.mk.
The backend already used a mechanically-generated list of *.c files,
but everywhere else we had a manually-written-out list of files in
which to seek translatable messages.  Commit b0a55e432 contains the
latest in a long line of failures to update those lists.  Rather than
manually fix its oversight, let's change to using "$(wildcard *.c)"
in all these nls.mk files.

Many of these files also have manual references to some *.c files
in other directories, most often src/common/.  Perhaps we should try
to improve that situation too; but it's a bit less clear how, so for
now just fix the local file references.

Kyotaro Horiguchi and Tom Lane

Discussion: https://postgr.es/m/20220713.160853.453362706160476128.horikyota.ntt@gmail.com
2022-07-13 12:56:42 -04:00
Alvaro Herrera e1df03b80d
Plug memory leak
Commit 054325c5ee 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.
2022-07-13 12:58:56 +02:00
Peter Eisentraut 88dad06b47 NLS: Put list of available languages into LINGUAS files
This moves the list of available languages from nls.mk into a separate
file called po/LINGUAS.  Advantages:

- It keeps the parts notionally managed by programmers (nls.mk)
  separate from the parts notionally managed by translators (LINGUAS).

- It's the standard practice recommended by the Gettext manual
  nowadays.

- The Meson build system also supports this layout (and of course
  doesn't know anything about our custom nls.mk), so this would enable
  sharing the list of languages between the two build systems.

(The MSVC build system currently finds all po files by globbing, so it
is not affected by this change.)

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/557a9f5c-e871-edc7-2f58-a4140fb65b7b@enterprisedb.com
2022-07-13 08:19:17 +02:00
Alvaro Herrera 054325c5ee
libpq: Improve idle state handling in pipeline mode
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
2022-07-05 14:21:20 +02:00
Noah Misch b6a5158f98 Add Windows file version information to libpq/test programs.
Back-patch to v15, the first version to install these programs.

Discussion: https://postgr.es/m/20220501080706.GA1542365@rfd.leadboat.com
2022-07-03 13:07:17 -07:00
Peter Eisentraut 5faef9d582 Remove redundant null pointer checks before PQclear and PQconninfoFree
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
2022-07-03 20:11:05 +02:00
Peter Eisentraut 02c408e21a Remove redundant null pointer checks before free()
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
2022-07-03 11:47:15 +02:00
Peter Eisentraut 3c0bcdbc66 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 46c120873f1e906cc8dab74d8d756417e1b367f6
2022-06-27 08:19:02 +02:00
Tom Lane 2b65de7fc2 Remove misguided SSL key file ownership check in libpq.
Commits a59c79564 et al. tried to sync libpq's SSL key file
permissions checks with what we've used for years in the backend.
We did not intend to create any new failure cases, but it turns out
we did: restricting the key file's ownership breaks cases where the
client is allowed to read a key file despite not having the identical
UID.  In particular a client running as root used to be able to read
someone else's key file; and having seen that I suspect that there are
other, less-dubious use cases that this restriction breaks on some
platforms.

We don't really need an ownership check, since if we can read the key
file despite its having restricted permissions, it must have the right
ownership --- under normal conditions anyway, and the point of this
patch is that any additional corner cases where that works should be
deemed allowable, as they have been historically.  Hence, just drop
the ownership check, and rearrange the permissions check to get rid
of its faulty assumption that geteuid() can't be zero.  (Note that the
comparable backend-side code doesn't have to cater for geteuid() == 0,
since the server rejects that very early on.)

This does have the end result that the permissions safety check used
for a root user's private key file is weaker than that used for
anyone else's.  While odd, root really ought to know what she's doing
with file permissions, so I think this is acceptable.

Per report from Yogendra Suralkar.  Like the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/MW3PR15MB3931DF96896DC36D21AFD47CA3D39@MW3PR15MB3931.namprd15.prod.outlook.com
2022-05-26 14:14:05 -04:00
Peter Eisentraut 6a8a7b1ccb Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: dde45df385dab9032155c1f867b677d55695310c
2022-05-16 11:12:42 +02:00
Tom Lane 23e7b38bfe Pre-beta mechanical code beautification.
Run pgindent, pgperltidy, and reformat-dat-files.
I manually fixed a couple of comments that pgindent uglified.
2022-05-12 15:17:30 -04:00
Tom Lane 93909599cd libpq: drop pending pipelined commands in pqDropConnection().
The original coding did this in pqDropServerData(), which seems
fairly backwards.  Pending commands are more like already-queued
output data, which is dropped in pqDropConnection().  Moving the
operation means that we clear the command queue immediately upon
detecting connection drop, which improves the sanity of subsequent
behavior.  In particular this eliminates duplicated error message
text as a consequence of code added in b15f25446, which supposed
that a nonempty command queue must mean the prior operation is
still active.

There might be an argument for backpatching this to v14; but as with
b15f25446, I'm unsure about interactions with 618c16707.  For now,
given the lack of complaints about v14's behavior, leave it alone.

Per report from Peter Eisentraut.

Discussion: https://postgr.es/m/de57761c-b99b-3435-b0a6-474c72b1149a@enterprisedb.com
2022-05-12 13:08:31 -04:00
Peter Eisentraut 3aa7a3d2a3 Add missing source files to nls.mk 2022-05-11 06:16:56 +02:00
Daniel Gustafsson 0432490d29 Rename libpq test programs with libpq_ prefix
The testclient and uri-regress programs in the libpq test suite had
quite generic names which didn't convey much meaning. Since they are
installed as part of the MSVC test runs, ensure that their purpose
is a little bit clearer by renaming with a libpq_ prefix. While at
it rename uri-regress to uri_regress to avoid mixing dash and under-
score in the same filename.

Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20220501080706.GA1542365@rfd.leadboat.com
2022-05-04 14:15:25 +02:00
Tom Lane 914611ea73 Fix missed cases in libpq's error handling.
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
2022-04-21 17:12:49 -04:00
Alvaro Herrera 24d2b2680a
Remove extraneous blank lines before block-closing braces
These are useless and distracting.  We wouldn't have written the code
with them to begin with, so there's no reason to keep them.

Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com
Discussion: https://postgr.es/m/attachment/133167/0016-Extraneous-blank-lines.patch
2022-04-13 19:16:02 +02:00
Peter Eisentraut 465ab24296 libpq: Fix pkg-config without OpenSSL
Do not add OpenSSL dependencies to libpq pkg-config file if OpenSSL is
not enabled.  Oversight in beff361bc1.

Author: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/20220331163759.32665-1-fontaine.fabrice%40gmail.com
2022-04-01 17:15:24 +02:00
Peter Eisentraut c1932e5428 libpq: Allow IP address SANs in server certificates
The current implementation supports exactly one IP address in a server
certificate's Common Name, which is brittle (the strings must match
exactly).  This patch adds support for IPv4 and IPv6 addresses in a
server's Subject Alternative Names.

Per discussion on-list:

- If the client's expected host is an IP address, we allow fallback to
  the Subject Common Name if an iPAddress SAN is not present, even if
  a dNSName is present.  This matches the behavior of NSS, in
  violation of the relevant RFCs.

- We also, counter-intuitively, match IP addresses embedded in dNSName
  SANs.  From inspection this appears to have been the behavior since
  the SAN matching feature was introduced in acd08d76.

- Unlike NSS, we don't map IPv4 to IPv6 addresses, or vice-versa.

Author: Jacob Champion <pchampion@vmware.com>
Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/9f5f20974cd3a4091a788cf7f00ab663d5fcdffe.camel@vmware.com
2022-04-01 15:51:23 +02:00
Tom Lane 878e64d0f8 Add missing newline in one libpq error message.
Oversight in commit a59c79564.  Back-patch, as that was.
Noted by Peter Eisentraut.

Discussion: https://postgr.es/m/7f85ef6d-250b-f5ec-9867-89f0b16d019f@enterprisedb.com
2022-03-31 11:24:26 -04:00
Daniel Gustafsson ebc8b7d441 Enable SSL library detection via PQsslAttribute()
Currently, libpq client code must have a connection handle
before it can query the "library" SSL attribute.  This poses
problems if the client needs to know what SSL library is in
use before constructing a connection string.

Allow PQsslAttribute(NULL, "library") to return the library
in use -- currently, just "OpenSSL" or NULL. The new behavior
is announced with the LIBPQ_HAS_SSL_LIBRARY_DETECTION feature
macro, allowing clients to differentiate between a libpq that
was compiled without SSL support and a libpq that's just too
old to tell.

Author: Jacob Champion <pchampion@vmware.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/4c8b76ef434a96627170a31c3acd33cbfd6e41f1.camel@vmware.com
2022-03-29 14:02:45 +02:00
Alvaro Herrera 7103ebb7aa
Add support for MERGE SQL command
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
2022-03-28 16:47:48 +02:00
Michael Paquier 6bdf1a1400 Fix collection of typos in the code and the documentation
Some words were duplicated while other places were grammatically
incorrect, including one variable name in the code.

Author: Otto Kekalainen, Justin Pryzby
Discussion: https://postgr.es/m/7DDBEFC5-09B6-4325-B942-B563D1A24BDC@amazon.com
2022-03-15 11:29:35 +09:00
Tom Lane 9240589798 Fix pg_regress to print the correct postmaster address on Windows.
pg_regress reported "Unix socket" as the default location whenever
HAVE_UNIX_SOCKETS is defined.  However, that's not been accurate
on Windows since 8f3ec75de.  Update this logic to match what libpq
actually does now.

This is just cosmetic, but still it's potentially misleading.
Back-patch to v13 where 8f3ec75de came in.

Discussion: https://postgr.es/m/3894060.1646415641@sss.pgh.pa.us
2022-03-04 13:23:58 -05:00
Tom Lane a59c79564b Allow root-owned SSL private keys in libpq, not only the backend.
This change makes libpq apply the same private-key-file ownership
and permissions checks that we have used in the backend since commit
9a83564c5.  Namely, that the private key can be owned by either the
current user or root (with different file permissions allowed in the
two cases).  This allows system-wide management of key files, which
is just as sensible on the client side as the server, particularly
when the client is itself some application daemon.

Sync the comments about this between libpq and the backend, too.

David Steele

Discussion: https://postgr.es/m/f4b7bc55-97ac-9e69-7398-335e212f7743@pgmasters.net
2022-02-28 14:12:52 -05:00
Tom Lane b15f254466 Adjust interaction of libpq pipeline mode with errorMessage resets.
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
2022-02-28 11:31:30 -05:00
Andres Freund 6b04abdfc5 Run tap tests in src/interfaces/libpq.
To be able to run binaries in the test/ directory, prove_[install]check need
to be executable in a single shell invocation, so that test/ can be added to
PATH.

Discussion: https://postgr.es/m/20220223203031.ezrd73ohvjgfksow@alap3.anarazel.de
2022-02-26 16:51:47 -08:00
Andres Freund ac25173cdb Convert src/interfaces/libpq/test to a tap test.
The old form of the test needed a bunch of custom infrastructure. These days
tap tests provide the necessary infrastructure to do better.

We discussed whether to move this test to src/test/modules, alongside
libpq_pipeline, but concluded that the opposite direction would be
better. libpq_pipeline will be moved at a later date, once the buildfarm and
msvc build infrastructure is ready for it.

The invocation of the tap test will be added in the next commit. It involves
just enough buildsystem changes to be worth commiting separately. Can't happen
the other way round because prove errors out when invoked without tests.

Discussion: https://postgr.es/m/20220223203031.ezrd73ohvjgfksow@alap3.anarazel.de
2022-02-26 16:51:47 -08:00
Tom Lane 83a7637e2c Reset conn->errorReported when PQrequestCancel sets errorMessage.
Oversight in commit 618c16707.  This is mainly neatnik-ism, since
if PQrequestCancel is used per its API contract, we should perform
pqClearConnErrorState before reaching any place that would consult
errorReported.  But still, it seems like a bad idea to potentially
leave errorReported pointing past errorMessage.len.
2022-02-20 15:02:41 -05:00
Tom Lane 618c16707a Rearrange libpq's error reporting to avoid duplicated error text.
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
2022-02-18 15:35:21 -05:00
Tom Lane 2e372869aa Don't let libpq PGEVT_CONNRESET callbacks break a PGconn.
As currently implemented, failure of a PGEVT_CONNRESET callback
forces the PGconn into the CONNECTION_BAD state (without closing
the socket, which is inconsistent with other failure paths), and
prevents later callbacks from being called.  This seems highly
questionable, and indeed is questioned by comments in the source.

Instead, let's just ignore the result value of PGEVT_CONNRESET
calls.  Like the preceding commit, this converts event callbacks
into "pure observers" that cannot affect libpq's processing logic.

Discussion: https://postgr.es/m/3185105.1644960083@sss.pgh.pa.us
2022-02-18 11:43:04 -05:00
Tom Lane ce1e7a2f71 Don't let libpq "event" procs break the state of PGresult objects.
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
2022-02-18 11:37:27 -05:00
Peter Eisentraut 797129e591 Remove IS_AF_UNIX macro
The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS,
apparently since 2008.  So the redirection through IS_AF_UNIX() is
apparently no longer necessary.  (More generally, all supported
platforms are now HAVE_UNIX_SOCKETS, but even if there were a new
platform in the future, it seems plausible that it would define the
AF_UNIX symbol even without kernel support.)  So remove the
IS_AF_UNIX() macro and make the code a bit more consistent.

Discussion: https://www.postgresql.org/message-id/flat/f2d26815-9832-e333-d52d-72fbc0ade896%40enterprisedb.com
2022-02-15 10:16:34 +01:00
Tom Lane faa189c932 Move libpq's write_failed mechanism down to pqsecure_raw_write().
Commit 1f39a1c06 implemented write-failure postponement in pqSendSome,
which is above SSL/GSS processing.  However, we've now seen failures
indicating that (some versions of?) OpenSSL have a tendency to report
write failures prematurely too.  Hence, move the primary responsibility
for postponing write failures down to pqsecure_raw_write(), below
SSL/GSS processing.  pqSendSome now sets write_failed only in corner
cases where we'd lost the connection already.

A side-effect of this change is that errors detected in the SSL/GSS
layer itself will be reported immediately (as if they were read
errors) rather than being postponed like write errors.  That's
reverting an effect of 1f39a1c06, and I think it's fine: if there's
not a socket-level error, it's hard to be sure whether an OpenSSL
error ought to be considered a read or write failure anyway.

Another important point is that write-failure postponement is now
effective during connection setup.  OpenSSL's misbehavior of this
sort occurs during SSL_connect(), so that's a change we want.

Per bug #17391 from Nazir Bilal Yavuz.  Possibly this should be
back-patched, but I think it prudent to let it age awhile in HEAD
first.

Discussion: https://postgr.es/m/17391-304f81bcf724b58b@postgresql.org
2022-02-12 14:00:09 -05:00
Tom Lane 335fa5a260 Fix thinko in PQisBusy().
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
2022-02-12 13:23:20 -05:00
Tom Lane 1f655fdc39 Fix race condition in gettext() initialization in libpq and ecpglib.
In libpq and ecpglib, multiple threads can concurrently enter the
initialization logic for message localization.  Since we set the
its-done flag before actually doing the work, it'd be possible
for some threads to reach gettext() before anyone has called
bindtextdomain().  Barring bugs in libintl itself, this would not
result in anything worse than failure to localize some early
messages.  Nonetheless, it's a bug, and an easy one to fix.

Noted while investigating bug #17299 from Clemens Zeidler
(much thanks to Liam Bowen for followup investigation on that).
It currently appears that that actually *is* a bug in libintl itself,
but that doesn't let us off the hook for this bit.

Back-patch to all supported versions.

Discussion: https://postgr.es/m/17299-7270741958c0b1ab@postgresql.org
Discussion: https://postgr.es/m/CAE7q7Eit4Eq2=bxce=Fm8HAStECjaXUE=WBQc-sDDcgJQ7s7eg@mail.gmail.com
2022-01-21 15:36:27 -05:00
Tom Lane 5987feb70b Make PQcancel use the PGconn's tcp_user_timeout and keepalives settings.
If connectivity to the server has been lost or become flaky, the
user might well try to send a query cancel.  It's highly annoying
if PQcancel hangs up in such a case, but that's exactly what's likely
to happen.  To ameliorate this problem, apply the PGconn's
tcp_user_timeout and keepalives settings to the TCP connection used
to send the cancel.  This should be safe on Unix machines, since POSIX
specifies that setsockopt() is async-signal-safe.  We are guessing
that WSAIoctl(SIO_KEEPALIVE_VALS) is similarly safe on Windows.
(Note that at least in psql and our other frontend programs, there's
no safety issue involved anyway, since we run PQcancel in its own
thread rather than in a signal handler.)

Most of the value here comes from the expectation that tcp_user_timeout
will be applied as a connection timeout.  That appears to happen on
Linux, even though its tcp(7) man page claims differently.  The
keepalive options probably won't help much, but as long as we can
apply them for not much code, we might as well.

Jelte Fennema, reviewed by Fujii Masao and myself

Discussion: https://postgr.es/m/AM5PR83MB017870DE81FC84D5E21E9D1EF7AA9@AM5PR83MB0178.EURPRD83.prod.outlook.com
2022-01-18 14:13:13 -05:00