Commit Graph

4739 Commits

Author SHA1 Message Date
Michael Paquier
ba90eac7a9 pg_stat_statements: Expand tests for SET statements
There are many grammar flavors that depend on the parse node
VariableSetStmt.  This closes the gap in pg_stat_statements by providing
test coverage for what should be a large majority of them, improving more
the work begun in de2aca2885.  This will be used to ease the
evaluation of a path towards more normalization of SET queries with
query jumbling.

Note that SET NAMES (grammar from the standard, synonym of SET
client_encoding) is omitted on purpose, this could use UTF8 with a
conditional script where UTF8 is supported, but that does not seem worth
the maintenance cost for the sake of these tests.

The author has submitted most of these in a TAP test (filled in any
holes I could spot), still queries in a SQL file of pg_stat_statements
is able to achieve the same goal while being easier to look at when
testing normalization patterns.

Author: Greg Sabino Mullane, Michael Paquier
Discussion: https://postgr.es/m/CAKAnmmJtJY2jzQN91=2QAD2eAJAA-Per61eyO48-TyxEg-q0Rg@mail.gmail.com
2024-09-25 10:04:44 +09:00
Tom Lane
cd838e2008 Neaten up our choices of SQLSTATEs for XML-related errors.
When our XML-handling modules were first written, the SQL standard
lacked any error codes that were particularly intended for XML
error conditions.  Unsurprisingly, this led to some rather random
choices of errcodes in those modules.  Now the standard has a whole
SQLSTATE class, "Class 10 - XQuery Error", with a reasonably large
selection of relevant-looking errcodes.

In this patch I've chosen one fairly generic code defined by the
standard, 10608 = invalid_argument_for_xquery, and used it where
it seemed appropriate.  I've also made an effort to replace
ERRCODE_INTERNAL_ERROR everywhere it was not clearly reporting
a coding problem; in particular, many of the existing uses look
like they can fairly be reported as ERRCODE_OUT_OF_MEMORY.

It might be interesting to try to map libxml2's error codes into
the standard's new collection, but I've not undertaken that here.

Discussion: https://postgr.es/m/417250.1726341268@sss.pgh.pa.us
2024-09-24 12:59:56 -04:00
Fujii Masao
4f08ab5545 postgres_fdw: Extend postgres_fdw_get_connections to return user name.
This commit adds a "user_name" output column to
the postgres_fdw_get_connections function, returning the name
of the local user mapped to the foreign server for each connection.
If a public mapping is used, it returns "public."

This helps identify postgres_fdw connections more easily,
such as determining which connections are invalid, closed,
or used within the current transaction.

No extension version bump is needed, as commit c297a47c5f
already handled it for v18~.

Author: Hayato Kuroda
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/b492a935-6c7e-8c08-e485-3c1d64d7d10f@oss.nttdata.com
2024-09-18 12:51:48 +09:00
Michael Paquier
933848d16d Add missing query ID reporting in extended query protocol
This commit adds query ID reports for two code paths when processing
extended query protocol messages:
- When receiving a bind message, setting it to the first Query retrieved
from a cached cache.
- When receiving an execute message, setting it to the first PlannedStmt
stored in a portal.

An advantage of this method is that this is able to cover all the types
of portals handled in the extended query protocol, particularly these
two when the report done in ExecutorStart() is not enough (neither is an
addition in ExecutorRun(), actually, for the second point):
- Multiple execute messages, with multiple ExecutorRun().
- Portal with execute/fetch messages, like a query with a RETURNING
clause and a fetch size that stores the tuples in a first execute
message going though ExecutorStart() and ExecuteRun(), followed by one
or more execute messages doing only fetches from the tuplestore created
in the first message.  This corresponds to the case where
execute_is_fetch is set, for example.

Note that the query ID reporting done in ExecutorStart() is still
necessary, as an EXECUTE requires it.  Query ID reporting is optimistic
and more calls to pgstat_report_query_id() don't matter as the first
report takes priority except if the report is forced.  The comment in
ExecutorStart() is adjusted to reflect better the reality with the
extended query protocol.

The test added in pg_stat_statements is a courtesy of Robert Haas.  This
uses psql's \bind metacommand, hence this part is backpatched down to
v16.

Reported-by:  Kaido Vaikla, Erik Wienhold
Author: Sami Imseih
Reviewed-by: Jian He, Andrei Lepikhov, Michael Paquier
Discussion: https://postgr.es/m/CA+427g8DiW3aZ6pOpVgkPbqK97ouBdf18VLiHFesea2jUk3XoQ@mail.gmail.com
Discussion: https://postgr.es/m/CA+TgmoZxtnf_jZ=VqBSyaU8hfUkkwoJCJ6ufy4LGpXaunKrjrg@mail.gmail.com
Discussion: https://postgr.es/m/1391613709.939460.1684777418070@office.mailbox.org
Backpatch-through: 14
2024-09-18 09:59:09 +09:00
Peter Eisentraut
89f908a6d0 Add temporal FOREIGN KEY contraints
Add PERIOD clause to foreign key constraint definitions.  This is
supported for range and multirange types.  Temporal foreign keys check
for range containment instead of equality.

This feature matches the behavior of the SQL standard temporal foreign
keys, but it works on PostgreSQL's native ranges instead of SQL's
"periods", which don't exist in PostgreSQL (yet).

Reference actions ON {UPDATE,DELETE} {CASCADE,SET NULL,SET DEFAULT}
are not supported yet.

(previously committed as 34768ee361, reverted by 8aee330af55; this is
essentially unchanged from those)

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
2024-09-17 11:29:30 +02:00
Peter Eisentraut
fc0438b4e8 Add temporal PRIMARY KEY and UNIQUE constraints
Add WITHOUT OVERLAPS clause to PRIMARY KEY and UNIQUE constraints.
These are backed by GiST indexes instead of B-tree indexes, since they
are essentially exclusion constraints with = for the scalar parts of
the key and && for the temporal part.

(previously committed as 46a0cd4cef, reverted by 46a0cd4cefb; the new
part is this:)

Because 'empty' && 'empty' is false, the temporal PK/UQ constraint
allowed duplicates, which is confusing to users and breaks internal
expectations.  For instance, when GROUP BY checks functional
dependencies on the PK, it allows selecting other columns from the
table, but in the presence of duplicate keys you could get the value
from any of their rows.  So we need to forbid empties.

This all means that at the moment we can only support ranges and
multiranges for temporal PK/UQs, unlike the original patch (above).
Documentation and tests for this are added.  But this could
conceivably be extended by introducing some more general support for
the notion of "empty" for other types.

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
2024-09-17 11:29:30 +02:00
Peter Eisentraut
7406ab623f Add stratnum GiST support function
This is support function 12 for the GiST AM and translates
"well-known" RT*StrategyNumber values into whatever strategy number is
used by the opclass (since no particular numbers are actually
required).  We will use this to support temporal PRIMARY
KEY/UNIQUE/FOREIGN KEY/FOR PORTION OF functionality.

This commit adds two implementations, one for internal GiST opclasses
(just an identity function) and another for btree_gist opclasses.  It
updates btree_gist from 1.7 to 1.8, adding the support function for
all its opclasses.

(previously committed as 6db4598fcb, reverted by 8aee330af55; this is
essentially unchanged from those)

Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
2024-09-17 11:29:29 +02:00
Tom Lane
d5622acb32 Replace usages of xmlXPathCompile() with xmlXPathCtxtCompile().
In existing releases of libxml2, xmlXPathCompile can be driven
to stack overflow because it fails to protect itself against
too-deeply-nested input.  While there is an upstream fix as of
yesterday, it will take years for that to propagate into all
shipping versions.  In the meantime, we can protect our own
usages basically for free by calling xmlXPathCtxtCompile instead.

(The actual bug is that libxml2 keeps its nesting counter in the
xmlXPathContext, and its parsing code was willing to just skip
counting nesting levels if it didn't have a context.  So if we supply
a context, all is well.  It seems odd actually that it works at all
to not supply a context, because this means that XPath parsing does
not have access to XML namespace info.  Apparently libxml2 never
checks namespaces until runtime?  Anyway, this seems like good
future-proofing even if its only immediate effect is to dodge a bug.)

Sadly, this hack only offers protection with libxml2 2.9.11 and newer.
Before that there are multiple similar problems, so if you are
processing untrusted XML it behooves you to get a newer version.
But we have some pretty old libxml2 in the buildfarm, so it seems
impractical to add a regression test to verify this fix.

Per bug #18617 from Jingzhou Fu.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/18617-1cee4d2ed1f4e7ae@postgresql.org
Discussion: https://gitlab.gnome.org/GNOME/libxml2/-/issues/799
2024-09-15 13:33:09 -04:00
Tom Lane
94537982ec Remove obsolete comment in pg_stat_statements.
Commit 76db9cb63 removed the use of multiple nesting counters,
but missed one comment describing that arrangement.

Back-patch to v17 where 76db9cb63 came in, just to avoid confusion.

Julien Rouhaud

Discussion: https://postgr.es/m/gfcwh3zjxc2vygltapgo7g6yacdor5s4ynr234b6v2ohhuvt7m@gr42joxalenw
2024-09-14 11:42:31 -04:00
Nathan Bossart
70d1c664f4 Fix contrib/pageinspect's test for sequences.
I managed to break this test in two different ways in commit
05036a3155.

First, the output of the new call to tuple_data_split() on the test
sequence is dependent on endianness.  This is fixed by setting a
special start value for the test sequence that produces the same
output regardless of the endianness of the machine.

Second, on versions older than v15, the new test case fails under
"force_parallel_mode = regress" with the following error:

	ERROR:  cannot access temporary tables during a parallel operation

This is because pageinspect's disk-accessing functions are
incorrectly marked PARALLEL SAFE on versions older than v15 (see
commit aeaaf520f4 for details).  This one is fixed by changing the
test sequence to be permanent.  The only reason it was previously
marked temporary was to avoid needing a DROP SEQUENCE command at
the end of the test.  Unlike some other tests in this file, the use
of a permanent sequence here shouldn't result in any test
instability like what was fixed by commit e2933a6e11.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/ZuOKOut5hhDlf_bP%40nathan
Backpatch-through: 12
2024-09-13 10:16:40 -05:00
Michael Paquier
7b1ddbae36 pg_stat_statements: Add tests with extended query protocol
There are currently no tests in the tree checking that queries using the
extended query protocol are able to map with their query ID.

This can be achieved for some paths of the extended query protocol with
the psql meta-commands \bind or \bind_named, so let's add some tests
based on both.

I have found that to be a useful addition while working on a different
issue.

Discussion: https://postgr.es/m/ZuEt6MOEBSlifBfn@paquier.xyz
2024-09-13 09:41:06 +09:00
Nathan Bossart
05036a3155 Reintroduce support for sequences in pgstattuple and pageinspect.
Commit 4b82664156 restricted a number of functions provided by
contrib modules to only relations that use the "heap" table access
method.  Sequences always use this table access method, but they do
not advertise as such in the pg_class system catalog, so the
aforementioned commit also (presumably unintentionally) removed
support for sequences from some of these functions.  This commit
reintroduces said support for sequences to these functions and adds
a couple of relevant tests.

Co-authored-by: Ayush Vatsa
Reviewed-by: Robert Haas, Michael Paquier, Matthias van de Meent
Discussion: https://postgr.es/m/CACX%2BKaP3i%2Bi9tdPLjF5JCHVv93xobEdcd_eB%2B638VDvZ3i%3DcQA%40mail.gmail.com
Backpatch-through: 12
2024-09-12 16:31:29 -05:00
Peter Eisentraut
8b5c6a54c4 Replace gratuitous memmove() with memcpy()
The index access methods all had similar code that copied the
passed-in scan keys to local storage.  They all used memmove() for
that, which is not wrong, but it seems confusing not to use memcpy()
when that would work.  Presumably, this was all once copied from
ancient code and never adjusted.

Discussion: https://www.postgresql.org/message-id/flat/f8c739d9-f48d-4187-b214-df3391ba41ab@eisentraut.org
2024-09-11 15:21:36 +02:00
Noah Misch
65c310b310 Optimize pg_visibility with read streams.
We've measured 5% performance improvement, and this arranges to benefit
automatically from future optimizations to the read_stream subsystem.
The area lacked test coverage, so close that gap.

Nazir Bilal Yavuz

Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com
Discussion: https://postgr.es/m/CAEudQAozv3wTY5TV2t29JcwPydbmKbiWQkZD42S2OgzdixPMDQ@mail.gmail.com
2024-09-10 15:21:33 -07:00
Peter Eisentraut
56fead44dc Add amgettreeheight index AM API routine
The only current implementation is for btree where it calls
_bt_getrootheight().  Other index types can now also use this to pass
information to their amcostestimate routine.  Previously, btree was
hardcoded and other index types could not hook into the optimizer at
this point.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com
2024-09-10 10:03:23 +02:00
Tom Lane
de239d01e7 Consistently use PageGetExactFreeSpace() in pgstattuple.
Previously this code used PageGetHeapFreeSpace on heap pages,
and usually used PageGetFreeSpace on index pages (though for some
reason GetHashPageStats used PageGetExactFreeSpace instead).
The difference is that those functions subtract off the size of
a line pointer, and PageGetHeapFreeSpace has some additional
rules about returning zero if adding another line pointer would
require exceeding MaxHeapTuplesPerPage.  Those things make sense
when testing to see if a new tuple can be put on the page, but
they seem pretty strange for pure statistics collection.

Additionally, statapprox_heap had a special rule about counting
a "new" page as being fully available space.  This also seems
strange, because it's not actually usable until VACUUM or some
such process initializes the page.  Moreover, it's inconsistent
with what pgstat_heap does, which is to count such a page as
having zero free space.  So make it work like pgstat_heap, which
as of this patch unconditionally calls PageGetExactFreeSpace.

This is more of a definitional change than a bug fix, so no
back-patch.  The module's documentation doesn't define exactly
what "free space" means either, so we left that as-is.

Frédéric Yhuel, reviewed by Rafia Sabih and Andreas Karlsson.

Discussion: https://postgr.es/m/3a18f843-76f6-4a84-8cca-49537fefa15d@dalibo.com
2024-09-09 14:34:10 -04:00
Tom Lane
218527d014 Don't bother checking the result of SPI_connect[_ext] anymore.
SPI_connect/SPI_connect_ext have not returned any value other than
SPI_OK_CONNECT since commit 1833f1a1c in v10; any errors are thrown
via ereport.  (The most likely failure is out-of-memory, which has
always been thrown that way, so callers had better be prepared for
such errors.)  This makes it somewhat pointless to check these
functions' result, and some callers within our code haven't been
bothering; indeed, the only usage example within spi.sgml doesn't
bother.  So it's likely that the omission has propagated into
extensions too.

Hence, let's standardize on not checking, and document the return
value as historical, while not actually changing these functions'
behavior.  (The original proposal was to change their return type
to "void", but that would needlessly break extensions that are
conforming to the old practice.)  This saves a small amount of
boilerplate code in a lot of places.

Stepan Neretin

Discussion: https://postgr.es/m/CAMaYL5Z9Uk8cD9qGz9QaZ2UBJFOu7jFx5Mwbznz-1tBbPDQZow@mail.gmail.com
2024-09-09 12:18:34 -04:00
Noah Misch
ddfc556a64 Revert "Optimize pg_visibility with read streams."
This reverts commit ed1b1ee59f and its
followup 1c61fd8b52.  They rendered
collect_corrupt_items() unable to detect corruption.

Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com
Discussion: https://postgr.es/m/CAEudQAozv3wTY5TV2t29JcwPydbmKbiWQkZD42S2OgzdixPMDQ@mail.gmail.com
2024-09-04 11:36:40 -07:00
Michael Paquier
b4db64270e Apply more quoting to GUC names in messages
This is a continuation of 17974ec259.  More quotes are applied to
GUC names in error messages and hints, taking care of what seems to be
all the remaining holes currently in the tree for the GUCs.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
2024-09-04 13:50:44 +09:00
Noah Misch
1c61fd8b52 Fix stack variable scope from previous commit.
The defect came from me, not from that commit's credited author.  Per
buildfarm members olingo and grassquit.

Discussion: https://postgr.es/m/20240903192030.1e@rfd.leadboat.com
2024-09-03 12:44:54 -07:00
Noah Misch
ed1b1ee59f Optimize pg_visibility with read streams.
We've measured 5% performance improvement, and this arranges to benefit
automatically from future optimizations to the read_stream subsystem.

Nazir Bilal Yavuz

Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com
2024-09-03 10:46:20 -07:00
Noah Misch
c582b75851 Add block_range_read_stream_cb(), to deduplicate code.
This replaces two functions for iterating over all blocks in a range.  A
pending patch will use this instead of adding a third.

Nazir Bilal Yavuz

Discussion: https://postgr.es/m/20240820184742.f2.nmisch@google.com
2024-09-03 10:46:20 -07:00
Michael Paquier
4236825197 Fix typos and grammar in code comments and docs
Author: Alexander Lakhin
Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com
2024-09-03 14:49:04 +09:00
Daniel Gustafsson
a70e01d430 Remove support for OpenSSL older than 1.1.0
OpenSSL 1.0.2 has been EOL from the upstream OpenSSL project for
some time, and is no longer the default OpenSSL version with any
vendor which package PostgreSQL. By retiring support for OpenSSL
1.0.2 we can remove a lot of no longer required complexity for
managing state within libcrypto which is now handled by OpenSSL.

Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/ZG3JNursG69dz1lr@paquier.xyz
Discussion: https://postgr.es/m/CA+hUKGKh7QrYzu=8yWEUJvXtMVm_CNWH1L_TLWCbZMwbi1XP2Q@mail.gmail.com
2024-09-02 13:51:48 +02:00
Tom Lane
0e5c823806 Make postgres_fdw's query_cancel test less flaky.
This test occasionally shows

+WARNING:  could not get result of cancel request due to timeout

which appears to be because the cancel request is sometimes unluckily
sent to the remote session between queries, and then it's ignored.

This patch tries to make that less probable in three ways:

1. Use a test query that does not involve remote estimates, so that
no EXPLAINs are sent.
2. Make sure that the remote session is ready-to-go (transaction
started, SET commands sent) before we start the timer.
3. Increase the statement_timeout to 100ms, to give the local
session enough time to plan and issue the query.

We might have to go higher than 100ms to make this adequately
stable in the buildfarm, but let's see how it goes.

Back-patch to v17 where this test was introduced.

Jelte Fennema-Nio and Tom Lane

Discussion: https://postgr.es/m/578934.1725045685@sss.pgh.pa.us
2024-08-30 16:47:39 -04:00
Masahiko Sawada
52f1d6730b Fix memory counter update in ReorderBuffer.
Commit 5bec1d6bc5 changed the memory usage updates of the
ReorderBufferTXN to zero all at once by subtracting txn->size, rather
than updating it for each change. However, if TOAST reconstruction
data remained in the transaction when freeing it, there were cases
where it further subtracted the memory counter from zero, resulting in
an assertion failure.

This change calculates the memory size for each change and updates the
memory usage to precisely the amount that has been freed.

Backpatch to v17, where this was introducd.

Reviewed-by: Amit Kapila, Shlok Kyal
Discussion: https://postgr.es/m/CAD21AoAqkNUvicgKPT_dXzNoOwpPkVTg0QPPxEcWmzT0moCJ1g%40mail.gmail.com
Backpatch-through: 17
2024-08-26 11:00:07 -07:00
Robert Haas
e222534679 Treat number of disabled nodes in a path as a separate cost metric.
Previously, when a path type was disabled by e.g. enable_seqscan=false,
we either avoided generating that path type in the first place, or
more commonly, we added a large constant, called disable_cost, to the
estimated startup cost of that path. This latter approach can distort
planning. For instance, an extremely expensive non-disabled path
could seem to be worse than a disabled path, especially if the full
cost of that path node need not be paid (e.g. due to a Limit).
Or, as in the regression test whose expected output changes with this
commit, the addition of disable_cost can make two paths that would
normally be distinguishible in cost seem to have fuzzily the same cost.

To fix that, we now count the number of disabled path nodes and
consider that a high-order component of both the startup cost and the
total cost. Hence, the path list is now sorted by disabled_nodes and
then by total_cost, instead of just by the latter, and likewise for
the partial path list.  It is important that this number is a count
and not simply a Boolean; else, as soon as we're unable to respect
disabled path types in all portions of the path, we stop trying to
avoid them where we can.

Because the path list is now sorted by the number of disabled nodes,
the join prechecks must compute the count of disabled nodes during
the initial cost phase instead of postponing it to final cost time.

Counts of disabled nodes do not cross subquery levels; at present,
there is no reason for them to do so, since the we do not postpone
path selection across subquery boundaries (see make_subplan).

Reviewed by Andres Freund, Heikki Linnakangas, and David Rowley.

Discussion: http://postgr.es/m/CA+TgmoZ_+MS+o6NeGK2xyBv-xM+w1AfFVuHE4f_aq6ekHv7YSQ@mail.gmail.com
2024-08-21 10:12:30 -04:00
Peter Eisentraut
95b856de23 Remove incidental md5() function use from test
To allow test to pass in OpenSSL FIPS mode, similar to 657f5f223e, for
a new test that has been added since.

Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://www.postgresql.org/message-id/86763810-70a1-4872-8ba7-1676f788e5a2@eisentraut.org
2024-08-16 17:14:32 +02:00
Alexander Korotkov
e3ec9dc1bf Add missing wait_for_catchup() to pg_visibility tap test
e2ed7e3227 introduced check of pg_visibility on standby.  This commit adds
missing wait_for_catchup() to synchronize standby before querying it.
2024-08-16 00:58:32 +03:00
Alexander Korotkov
e2ed7e3227 Fix GetStrictOldestNonRemovableTransactionId() on standby
e85662df44 implemented GetStrictOldestNonRemovableTransactionId() function
for computation of xid horizon that avoid reporting of false errors.
However, GetStrictOldestNonRemovableTransactionId() uses
GetRunningTransactionData() even on standby leading to an assertion failure.

Given that we decided to ignore KnownAssignedXids and standby can't have
own running xids, we switch to use TransamVariables->nextXid as a xid horizon.

Also, revise the comment regarding ignoring KnownAssignedXids with more
detailed reasoning provided by Heikki.

Reported-by: Heikki Linnakangas
Discussion: https://postgr.es/m/42218c4f-2c8d-40a3-8743-4d34dd0e4cce%40iki.fi
Reviewed-by: Heikki Linnakangas
2024-08-16 00:18:55 +03:00
Heikki Linnakangas
fe8dd65bf2 Mark misc static global variables as const
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/54c29fb0-edf2-48ea-9814-44e918bbd6e8@iki.fi
2024-08-06 23:04:48 +03:00
Nathan Bossart
8928817769 Remove volatile qualifiers from pg_stat_statements.c.
Prior to commit 0709b7ee72, which changed the spinlock primitives
to function as compiler barriers, access to variables within a
spinlock-protected section required using a volatile pointer, but
that is no longer necessary.

Reviewed-by: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/Zqkv9iK7MkNS0KaN%40nathan
2024-08-06 10:56:37 -05:00
Masahiko Sawada
66e94448ab Restrict accesses to non-system views and foreign tables during pg_dump.
When pg_dump retrieves the list of database objects and performs the
data dump, there was possibility that objects are replaced with others
of the same name, such as views, and access them. This vulnerability
could result in code execution with superuser privileges during the
pg_dump process.

This issue can arise when dumping data of sequences, foreign
tables (only 13 or later), or tables registered with a WHERE clause in
the extension configuration table.

To address this, pg_dump now utilizes the newly introduced
restrict_nonsystem_relation_kind GUC parameter to restrict the
accesses to non-system views and foreign tables during the dump
process. This new GUC parameter is added to back branches too, but
these changes do not require cluster recreation.

Back-patch to all supported branches.

Reviewed-by: Noah Misch
Security: CVE-2024-7348
Backpatch-through: 12
2024-08-05 06:05:33 -07:00
Peter Eisentraut
9fb855fe1a Include bison header files into implementation files
Before Bison 3.4, the generated parser implementation files run afoul
of -Wmissing-variable-declarations (in spite of commit ab61c40bfa)
because declarations for yylval and possibly yylloc are missing.  The
generated header files contain an extern declaration, but the
implementation files don't include the header files.  Since Bison 3.4,
the generated implementation files automatically include the generated
header files, so then it works.

To make this work with older Bison versions as well, include the
generated header file from the .y file.

(With older Bison versions, the generated implementation file contains
effectively a copy of the header file pasted in, so including the
header file is redundant.  But we know this works anyway because the
core grammar uses this arrangement already.)

Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
2024-08-02 10:25:11 +02:00
Heikki Linnakangas
01e51ed780 Replace static buf with a stack-allocated one in 'seg' extension
The buffer is used only locally within the function. Also, the
initialization to '0' characters was unnecessary, the initial content
were always overwritten with sprintf(). I don't understand why it was
done that way, but it's been like that since forever.

In the passing, change from sprintf() to snprintf(). The buffer was
long enough so sprintf() was fine, but this makes it more obvious that
there's no risk of a buffer overflow.

Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/7f86e06a-98c5-4ce3-8ec9-3885c8de0358@iki.fi
2024-07-30 22:06:03 +03:00
Alexander Korotkov
cdd6ab9d1f amcheck: Optimize speed of checking for unique constraint violation
Currently, when amcheck validates a unique constraint, it visits the heap for
each index tuple.  This commit implements skipping keys, which have only one
non-dedeuplicated index tuple (quite common case for unique indexes). That
gives substantial economy on index checking time.

Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240325020323.fd.nmisch%40google.com
Author: Alexander Korotkov, Pavel Borisov
2024-07-28 13:50:57 +03:00
David Rowley
17a5871d9d Optimize escaping of JSON strings
There were quite a few places where we either had a non-NUL-terminated
string or a text Datum which we needed to call escape_json() on.  Many of
these places required that a temporary string was created due to the fact
that escape_json() needs a NUL-terminated cstring.  For text types, those
first had to be converted to cstring before calling escape_json() on them.

Here we introduce two new functions to make escaping JSON more optimal:

escape_json_text() can be given a text Datum to append onto the given
buffer.  This is more optimal as it foregoes the need to convert the text
Datum into a cstring.  A temporary allocation is only required if the text
Datum needs to be detoasted.

escape_json_with_len() can be used when the length of the cstring is
already known or the given string isn't NUL-terminated.  Having this
allows various places which were creating a temporary NUL-terminated
string to just call escape_json_with_len() without any temporary memory
allocations.

Discussion: https://postgr.es/m/CAApHDvpLXwMZvbCKcdGfU9XQjGCDm7tFpRdTXuB9PVgpNUYfEQ@mail.gmail.com
Reviewed-by: Melih Mutlu, Heikki Linnakangas
2024-07-27 23:46:07 +12:00
Fujii Masao
454aab4b73 postgres_fdw: Fix bug in connection status check.
The buildfarm member "hake" reported a failure in the regression test
added by commit 857df3cef7, where postgres_fdw_get_connections(true)
returned unexpected results.

The function postgres_fdw_get_connections(true) checks
if a connection is closed by using POLLRDHUP in the requested events
and calling poll(). Previously, the function only considered
POLLRDHUP or 0 as valid returned events. However, poll() can also
return POLLHUP, POLLERR, and/or POLLNVAL. So if any of these events
were returned, postgres_fdw_get_connections(true) would report
incorrect results. postgres_fdw_get_connections(true) failed to
account for these return events.

This commit updates postgres_fdw_get_connections(true) to correctly
report a closed connection when poll() returns not only POLLRDHUP
but also POLLHUP, POLLERR, or POLLNVAL.

Discussion: https://postgr.es/m/fd8f6186-9e1e-4b9a-92c5-e71e3697d381@oss.nttdata.com
2024-07-27 03:58:48 +09:00
Fujii Masao
857df3cef7 postgres_fdw: Add connection status check to postgres_fdw_get_connections().
This commit extends the postgres_fdw_get_connections() function
to check if connections are closed. This is useful for detecting closed
postgres_fdw connections that could prevent successful transaction
commits. Users can roll back transactions immediately upon detecting
closed connections, avoiding unnecessary processing of failed
transactions.

This feature is available only on systems supporting the non-standard
POLLRDHUP extension to the poll system call, including Linux.

Author: Hayato Kuroda
Reviewed-by: Shinya Kato, Zhihong Yu, Kyotaro Horiguchi, Andres Freund
Reviewed-by: Onder Kalaci, Takamichi Osumi, Vignesh C, Tom Lane, Ted Yu
Reviewed-by: Katsuragi Yuta, Peter Smith, Shubham Khanna, Fujii Masao
Discussion: https://postgr.es/m/TYAPR01MB58662809E678253B90E82CE5F5889@TYAPR01MB5866.jpnprd01.prod.outlook.com
2024-07-26 22:16:39 +09:00
Fujii Masao
c297a47c5f postgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().
This commit extends the postgres_fdw_get_connections() function to
include a new used_in_xact column, indicating whether each connection
is used in the current transaction.

This addition is particularly useful for the upcoming feature that
will check if connections are closed. By using those information,
users can verify if postgres_fdw connections used in a transaction
remain open. If any connection is closed, the transaction cannot
be committed successfully. In this case users can roll back it
immediately without waiting for transaction end.

The SQL API for postgres_fdw_get_connections() is updated by
this commit and may change in the future. To handle compatibility
with older SQL declarations, an API versioning system is introduced,
allowing the function to behave differently based on the API version.

Author: Hayato Kuroda
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/be9382f7-5072-4760-8b3f-31d6dffa8d62@oss.nttdata.com
2024-07-26 22:15:51 +09:00
Tom Lane
580f8727ca Add argument names to the regexp_XXX functions.
This change allows these functions to be called using named-argument
notation, which can be helpful for readability, particularly for
the ones with many arguments.

There was considerable debate about exactly which names to use,
but in the end we settled on the names already shown in our
documentation table 9.10.

The citext extension provides citext-aware versions of some of
these functions, so add argument names to those too.

In passing, fix table 9.10's syntax synopses for regexp_match,
which were slightly wrong about which combinations of arguments
are allowed.

Jian He, reviewed by Dian Fay and others

Discussion: https://postgr.es/m/CACJufxG3NFKKsh6x4fRLv8h3V-HvN4W5dA=zNKMxsNcDwOKang@mail.gmail.com
2024-07-25 14:51:46 -04:00
Daniel Gustafsson
cc59f9d0ff pgcrypto: Remove unused binary from clean target
Generation of the gen-rtab binary was removed in db7d1a7b0 but it
was accidentally left in the cleaning target.  Remove since it is
no longer built.

Author: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/c1d63754-cb85-2d8a-8409-bde2c4d2d04b@gmail.com
2024-07-25 14:27:01 +02:00
Peter Eisentraut
ab61c40bfa Add extern declarations for Bison global variables
This adds extern declarations for some global variables produced by
Bison that are not already declared in its generated header file.
This is a workaround to be able to add -Wmissing-variable-declarations
to the global set of warning options in the near future.

Another longer-term solution would be to convert these grammars to
"pure" parsers in Bison, to avoid global variables altogether.  Note
that the core grammar is already pure, so this patch did not need to
touch it.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
2024-07-25 09:26:08 +02:00
Fujii Masao
97f2bc5aa5 pg_stat_statements: Add regression test for privilege handling.
This commit adds a regression test to verify that pg_stat_statements
correctly handles privileges, improving its test coverage.

Author: Keisuke Kuroda
Reviewed-by: Michael Paquier, Fujii Masao
Discussion: https://postgr.es/m/2224ccf2e12c41ccb81702ef3303d5ac@nttcom.co.jp
2024-07-24 20:54:51 +09:00
Alvaro Herrera
90c1ba52e0
postgres_fdw: Split out the query_cancel test to its own file
This allows us to skip it in Cygwin, where it's reportedly flaky because
of platform bugs or something.

Backpatch to 17, where the test was introduced by commit 2466d6654f.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/e4d0cb33-6be5-e4d5-ae49-9eac3ff2b005@gmail.com
2024-07-22 12:49:57 +02:00
Etsuro Fujita
5c571a34d0 postgres_fdw: Avoid "cursor can only scan forward" error.
Commit d844cd75a disallowed rewind in a non-scrollable cursor to resolve
anomalies arising from such a cursor operation.  However, this failed to
take into account the assumption in postgres_fdw that when rescanning a
foreign relation, it can rewind the cursor created for scanning the
foreign relation without specifying the SCROLL option, regardless of its
scrollability, causing this error when it tried to do such a rewind in a
non-scrollable cursor.  Fix by modifying postgres_fdw to instead
recreate the cursor, regardless of its scrollability, when rescanning
the foreign relation.  (If we had a way to check its scrollability, we
could improve this by rewinding it if it is scrollable and recreating it
if not, but we do not have it, so this commit modifies it to recreate it
in any case.)

Per bug #17889 from Eric Cyr.  Devrim Gunduz also reported this problem.
Back-patch to v15 where that commit enforced the prohibition.

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/17889-e8c39a251d258dda%40postgresql.org
Discussion: https://postgr.es/m/b415ac3255f8352d1ea921cf3b7ba39e0587768a.camel%40gunduz.org
2024-07-19 13:15:00 +09:00
Michael Paquier
c145f321b6 Propagate query IDs of utility statements in functions
For utility statements defined within a function, the query tree is
copied to a PlannedStmt as utility commands do not require planning.
However, the query ID was missing from the information passed down.

This leads to plugins relying on the query ID like pg_stat_statements to
not be able to track utility statements within function calls.  Tests
are added to check this behavior, depending on pg_stat_statements.track.

This is an old bug.  Now, query IDs for utilities are compiled using
their parsed trees rather than the query string since v16
(3db72ebcbe), leading to less bloat with utilities, so backpatch down
only to this version.

Author: Anthonin Bonnefoy
Discussion: https://postgr.es/m/CAO6_XqrGp-uwBqi3vBPLuRULKkddjC7R5QZCgsFren=8E+m2Sg@mail.gmail.com
Backpatch-through: 16
2024-07-19 10:21:01 +09:00
Richard Guo
069d0ff022 Check lateral references within PHVs for memoize cache keys
If we intend to generate a Memoize node on top of a path, we need
cache keys of some sort.  Currently we search for the cache keys in
the parameterized clauses of the path as well as the lateral_vars of
its parent.  However, it turns out that this is not sufficient because
there might be lateral references derived from PlaceHolderVars, which
we fail to take into consideration.

This oversight can cause us to miss opportunities to utilize the
Memoize node.  Moreover, in some plans, failing to recognize all the
cache keys could result in performance regressions.  This is because
without identifying all the cache keys, we would need to purge the
entire cache every time we get a new outer tuple during execution.

This patch fixes this issue by extracting lateral Vars from within
PlaceHolderVars and subsequently including them in the cache keys.

In passing, this patch also includes a comment clarifying that Memoize
nodes are currently not added on top of join relation paths.  This
explains why this patch only considers PlaceHolderVars that are due to
be evaluated at baserels.

Author: Richard Guo
Reviewed-by: Tom Lane, David Rowley, Andrei Lepikhov
Discussion: https://postgr.es/m/CAMbWs48jLxn0pAPZpJ50EThZ569Xrw+=4Ac3QvkpQvNszbeoNg@mail.gmail.com
2024-07-15 10:26:33 +09:00
Masahiko Sawada
bb19b70081 Fix possibility of logical decoding partial transaction changes.
When creating and initializing a logical slot, the restart_lsn is set
to the latest WAL insertion point (or the latest replay point on
standbys). Subsequently, WAL records are decoded from that point to
find the start point for extracting changes in the
DecodingContextFindStartpoint() function. Since the initial
restart_lsn could be in the middle of a transaction, the start point
must be a consistent point where we won't see the data for partial
transactions.

Previously, when not building a full snapshot, serialized snapshots
were restored, and the SnapBuild jumps to the consistent state even
while finding the start point. Consequently, the slot's restart_lsn
and confirmed_flush could be set to the middle of a transaction. This
could lead to various unexpected consequences. Specifically, there
were reports of logical decoding decoding partial transactions, and
assertion failures occurred because only subtransactions were decoded
without decoding their top-level transaction until decoding the commit
record.

To resolve this issue, the changes prevent restoring the serialized
snapshot and jumping to the consistent state while finding the start
point.

On v17 and HEAD, a flag indicating whether snapshot restores should be
skipped has been added to the SnapBuild struct, and SNAPBUILD_VERSION
has been bumpded.

On backbranches, the flag is stored in the LogicalDecodingContext
instead, preserving on-disk compatibility.

Backpatch to all supported versions.

Reported-by: Drew Callahan
Reviewed-by: Amit Kapila, Hayato Kuroda
Discussion: https://postgr.es/m/2444AA15-D21B-4CCE-8052-52C7C2DAFE5C%40amazon.com
Backpatch-through: 12
2024-07-11 22:48:23 +09:00
Thomas Munro
18501841bc Add simple codepoint redirections to unaccent.rules.
Previously we searched for code points where the Unicode data file
listed an equivalent combining character sequence that added accents.
Some codepoints redirect to a single other codepoint, instead of doing
any combining.  We can follow those references recursively to get the
answer.

Per bug report #18362, which reported missing Ancient Greek characters.
Specifically, precomposed characters with oxia (from the polytonic
accent system used for old Greek) just point to precomposed characters
with tonos (from the monotonic accent system for modern Greek), and we
have to follow the extra hop to find out that they are composed with
an acute accent.

Besides those, the new rule also:

* pulls in a lot of 'Mathematical Alphanumeric Symbols', which are
  copies of the Latin and Greek alphabets and numbers rendered
  in different typefaces, and

* corrects a single mathematical letter that previously came from the
  CLDR transliteration file, but the new rule extracts from the main
  Unicode database file, where clearly the latter is right and the
  former is a wrong (reported to CLDR).

Reported-by: Cees van Zeeland <cees.van.zeeland@freedom.nl>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/18362-be6d0cfe122b6354%40postgresql.org
2024-07-05 15:25:31 +12:00