users/gsingh/postgres.git
2 years agoAllow batch insertion during COPY into a foreign table.
Etsuro Fujita [Thu, 13 Oct 2022 09:45:00 +0000 (18:45 +0900)]
Allow batch insertion during COPY into a foreign table.

Commit 3d956d956 allowed the COPY, but it's done by inserting individual
rows to the foreign table, so it can be inefficient due to the overhead
caused by each round-trip to the foreign server.  To improve performance
of the COPY in such a case, this patch allows batch insertion, by
extending the multi-insert machinery in CopyFrom() to the foreign-table
case so that we insert multiple rows to the foreign table at once using
the FDW callback routine added by commit b663a4136.  This patch also
allows this for postgres_fdw.  It is enabled by the "batch_size" option
added by commit b663a4136, which is disabled by default.

When doing batch insertion, we update progress of the COPY command after
performing the FDW callback routine, to count rows not suppressed by the
FDW as well as a BEFORE ROW INSERT trigger.  For consistency, this patch
changes the timing of updating it for plain tables: previously, we
updated it immediately after adding each row to the multi-insert buffer,
but we do so only after writing the rows stored in the buffer out to the
table using table_multi_insert(), which I think would be consistent even
with non-batching mode, because in that mode we update it after writing
each row out to the table using table_tuple_insert().

Andrey Lepikhov, heavily revised by me, with review from Ian Barwick,
Andrey Lepikhov, and Zhihong Yu.

Discussion: https://postgr.es/m/bc489202-9855-7550-d64c-ad2d83c24867%40postgrespro.ru

2 years agoAdd missing isolation test for test_decoding in meson build
Michael Paquier [Thu, 13 Oct 2022 07:03:01 +0000 (16:03 +0900)]
Add missing isolation test for test_decoding in meson build

Oversight in 7f13ac8, where catalog_change_snapshot was missing from the
list in meson.build.

Author: Hayato Kuroda
Discussion: https://postgr.es/m/TYAPR01MB58662C932F45A13C6F9BE352F5259@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agoImprove the WARNING message for CREATE SUBSCRIPTION.
Amit Kapila [Thu, 13 Oct 2022 00:39:43 +0000 (06:09 +0530)]
Improve the WARNING message for CREATE SUBSCRIPTION.

Author: Peter Smith
Reviewed-By: Alvaro Herrera, Tom Lane, Amit Kapila
Discussion: https://postgr.es/m/CAHut+PvqdqOanheWSHDyhQiF+Z-7w=-+k4U+bwbT=b6YQ_hrXQ@mail.gmail.com

2 years agoFix ordering issue with WAL operations in GIN fast insert path
Michael Paquier [Thu, 13 Oct 2022 00:31:57 +0000 (09:31 +0900)]
Fix ordering issue with WAL operations in GIN fast insert path

Contrary to what is documented in src/backend/access/transam/README,
ginHeapTupleFastInsert() had a few ordering issues with the way it does
its WAL operations when inserting items in its fast path.

First, when using a separate list, XLogBeginInsert() was being always
called before START_CRIT_SECTION(), and in this case a second thing was
wrong when merging lists, as an exclusive lock was taken on the tail
page *before* calling XLogBeginInsert().  Finally, when inserting items
into a tail page, the order of XLogBeginInsert() and
START_CRIT_SECTION() was reversed.  This commit addresses all these
issues by moving the calls of XLogBeginInsert() after all the pages
logged are locked and pinned, within a critical section.

Author:  Matthias van de Meent, Zhang Mingli
Discussion: https://postgr.es/m/CAEze2WhL8uLMqynnnCu1LAPwxD5RKEo0nHV+eXGg_N6ELU88HQ@mail.gmail.com

2 years agodoc: Fix description of replication command CREATE_REPLICATION_SLOT
Michael Paquier [Wed, 12 Oct 2022 23:53:42 +0000 (08:53 +0900)]
doc: Fix description of replication command CREATE_REPLICATION_SLOT

The output plugin name is a mandatory option when creating a logical
slot, but the grammar documented was not described as such.  While on
it, fix two comments in repl_gram.y to show that TEMPORARY is an
optional grammar choice.

Author: Ayaki Tachikake
Discussion: https://postgr.es/m/OSAPR01MB2852607B2329FFA27834105AF1229@OSAPR01MB2852.jpnprd01.prod.outlook.com
Backpatch-through: 15

2 years agoDoc: improve recommended systemd unit file.
Tom Lane [Wed, 12 Oct 2022 14:51:11 +0000 (10:51 -0400)]
Doc: improve recommended systemd unit file.

Add
    After=network-online.target
    Wants=network-online.target
to the suggested unit file for starting a Postgres server.
This delays startup until the network interfaces have been
configured; without that, any attempt to bind to a specific
IP address will fail.

If listen_addresses is set to "localhost" or "*", it might be
possible to get away with the less restrictive "network.target",
but I don't think we need to get into such detail here.

Per suggestion from Pablo Federico.

Discussion: https://postgr.es/m/166552157407.591805.10036014441784710940@wrigleys.postgresql.org

2 years agoFix outdated code reference
Alvaro Herrera [Wed, 12 Oct 2022 07:53:00 +0000 (09:53 +0200)]
Fix outdated code reference

ExecCreatePartitionPruneState was renamed by commit 297daa9d4353, but
this test file didn't get the memo.  Repair.

Author: Amit Langote
Discussion: https://postgr.es/m/CA+HiwqFLw=oLX0tP9kcKBmoOExNjDaoAe99dRcxo-GdB9abP9A@mail.gmail.com

2 years agoReduce xlog.h inclusion footprint
Alvaro Herrera [Wed, 12 Oct 2022 07:44:40 +0000 (09:44 +0200)]
Reduce xlog.h inclusion footprint

This file needs xlogreader.h only for the XLogReaderState typedef; but
we can dodge that by forward-declaring it.  Many files use xlog.h for
reasons other than reading WAL, and it's not good to force all those
files to include xlogreader.h, so take it out.

Surprisingly, there is no fallout in core code from making this change.
Perhaps external code will have to start including xlogreader.h.

2 years agoReduce basebackup_sink.h inclusion footprint
Alvaro Herrera [Wed, 12 Oct 2022 07:42:20 +0000 (09:42 +0200)]
Reduce basebackup_sink.h inclusion footprint

This file doesn't need xlog_internal.h, only xlogdefs.h.

2 years agoAdd meson.build to version_stamp.pl
Peter Eisentraut [Wed, 12 Oct 2022 05:03:51 +0000 (07:03 +0200)]
Add meson.build to version_stamp.pl

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/7567dd2d-5e28-c135-79ff-270d7ed83490%40enterprisedb.com

2 years agoRemove Abs()
Peter Eisentraut [Wed, 12 Oct 2022 04:36:12 +0000 (06:36 +0200)]
Remove Abs()

All callers have been replaced by standard C library functions.

Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com

2 years agoFix shadow variable in postgres.c
Michael Paquier [Wed, 12 Oct 2022 04:42:30 +0000 (13:42 +0900)]
Fix shadow variable in postgres.c

-Wshadow=compatible-local is added by default since 0fe954c, and this
warning was detected under -DWRITE_READ_PARSE_PLAN_TREES.

Reviewed-by: David Rowley
Discussion: https://postgr.es/m/Y0Ya5SH0QiaO9kKG@paquier.xyz

2 years agoSimplify some maths in xlogreader.c
Michael Paquier [Wed, 12 Oct 2022 00:59:36 +0000 (09:59 +0900)]
Simplify some maths in xlogreader.c

An LSN was calculated from a segment number, a segment size and a
position offset, matching exactly the LSN given by the caller of
XLogReaderValidatePageHeader().  This change removes the extra LSN
calculation, relying only on the LSN given by the function caller
instead.

Author: Bharath Rupireddy
Reviewed-by: Richard Guo, Álvaro Herrera, Kyotaro Horiguchi
Discussion: https://postgr.es/m/CALj2ACXuh4Ms9j9sxMYdtHEe=5sFcyrs-GAHyADu_A_G71kZTg@mail.gmail.com

2 years agoFix compilation warning in test_copy_callbacks
Michael Paquier [Tue, 11 Oct 2022 23:45:01 +0000 (08:45 +0900)]
Fix compilation warning in test_copy_callbacks

A passed-in parameter value was incorrect, for a warning coming from
MSVC.

Oversight in 9fcdf2c.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20221011224221.dvg5q7e7vhjdtcvv@awork3.anarazel.de

2 years agoHarden pmsignal.c against clobbered shared memory.
Tom Lane [Tue, 11 Oct 2022 22:54:31 +0000 (18:54 -0400)]
Harden pmsignal.c against clobbered shared memory.

The postmaster is not supposed to do anything that depends
fundamentally on shared memory contents, because that creates
the risk that a backend crash that trashes shared memory will
take the postmaster down with it, preventing automatic recovery.
In commit 969d7cd43 I lost sight of this principle and coded
AssignPostmasterChildSlot() in such a way that it could fail
or even crash if the shared PMSignalState structure became
corrupted.  Remarkably, we've not seen field reports of such
crashes; but I managed to induce one while testing the recent
changes around palloc chunk headers.

To fix, make a semi-duplicative state array inside the postmaster
so that we need consult only local state while choosing a "child
slot" for a new backend.  Ensure that other postmaster-executed
routines in pmsignal.c don't have critical dependencies on the
shared state, either.  Corruption of PMSignalState might now
lead ReleasePostmasterChildSlot() to conclude that backend X
failed, when actually backend Y was the one that trashed things.
But that doesn't matter, because we'll force a cluster-wide reset
regardless.

Back-patch to all supported branches, since this is an old bug.

Discussion: https://postgr.es/m/3436789.1665187055@sss.pgh.pa.us

2 years agoYet further fixes for multi-row VALUES lists for updatable views.
Tom Lane [Tue, 11 Oct 2022 22:24:14 +0000 (18:24 -0400)]
Yet further fixes for multi-row VALUES lists for updatable views.

DEFAULT markers appearing in an INSERT on an updatable view
could be mis-processed if they were in a multi-row VALUES clause.
This would lead to strange errors such as "cache lookup failed
for type NNNN", or in older branches even to crashes.

The cause is that commit 41531e42d tried to re-use rewriteValuesRTE()
to remove any SetToDefault nodes (that hadn't previously been replaced
by the view's own default values) appearing in "product" queries,
that is DO ALSO queries.  That's fundamentally wrong because the
DO ALSO queries might not even be INSERTs; and even if they are,
their targetlists don't necessarily match the view's column list,
so that almost all the logic in rewriteValuesRTE() is inapplicable.

What we want is a narrow focus on replacing any such nodes with NULL
constants.  (That is, in this context we are interpreting the defaults
as being strictly those of the view itself; and we already replaced
any that aren't NULL.)  We could add still more !force_nulls tests
to further lobotomize rewriteValuesRTE(); but it seems cleaner to
split out this case to a new function, restoring rewriteValuesRTE()
to the charter it had before.

Per bug #17633 from jiye_sw.  Patch by me, but thanks to
Richard Guo and Japin Li for initial investigation.
Back-patch to all supported branches, as the previous fix was.

Discussion: https://postgr.es/m/17633-98cc85e1fa91e905@postgresql.org

2 years agoDoc: add entry for pg_get_partkeydef().
Tom Lane [Tue, 11 Oct 2022 18:28:38 +0000 (14:28 -0400)]
Doc: add entry for pg_get_partkeydef().

Other pg_get_XXXdef() functions are documented, so it seems reasonable
to include this as well.

Ian Barwick

Discussion: https://postgr.es/m/CAB8KJ=hb2QZXdgyrrRjPCw++DsrRcui4fKArWabQ+oij+2x=_w@mail.gmail.com

2 years agoC comment: explain procArray->pgprocnos[]
Bruce Momjian [Tue, 11 Oct 2022 17:08:17 +0000 (13:08 -0400)]
C comment:  explain procArray->pgprocnos[]

Reported-by: Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TOs9Dh3KNR2kiQJ3Ow0=TBucL_57DAbm--2p8w5x_8YXQ@mail.gmail.com

Author: Aleksander Alekseev

Backpatch-through: master

2 years agoAdd a common function to generate the origin name.
Amit Kapila [Tue, 11 Oct 2022 05:07:52 +0000 (10:37 +0530)]
Add a common function to generate the origin name.

Make a common replication origin name formatting function to replace
multiple snprintf() expressions. This also includes logic previously done
by ReplicationOriginNameForTablesync().

This makes the code to generate the origin name consistent among apply
worker and tablesync worker.

Author: Peter Smith
Reviewed-By: Aleksander Alekseev
Discussion: https://postgr.es/m/CAHut%2BPsa8hhfSE6ozUK-ih7GkQziAVAf4f3bqiXEj2nQiu-43g%40mail.gmail.com

2 years agoAdd TAP tests for role membership in pg_hba.conf
Michael Paquier [Tue, 11 Oct 2022 04:57:07 +0000 (13:57 +0900)]
Add TAP tests for role membership in pg_hba.conf

This commit expands the coverage of pg_hba.conf with checks specific to
role memberships (one "root" role combined with a member and a
non-member).  Coverage is added for the database keywords "samegroup"
and "samerole", where the specified role has to be be a member of the
role with the same name as the requested database, and '+' on the user
entry, where members are allowed.  These tests are plugged in the
authentication test 001_password.pl as of extra connection attempts
combined with resets of pg_hba.conf, making them rather cheap.

Author: Nathan Bossart
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20221009211348.GB900071@nathanxps13

2 years agoAdd support for COPY TO callback functions
Michael Paquier [Tue, 11 Oct 2022 02:45:52 +0000 (11:45 +0900)]
Add support for COPY TO callback functions

This is useful as a way for extensions to process COPY TO rows in the
way they see fit (say auditing, analytics, backend, etc.) without the
need to invoke an external process running as the OS user running the
backend through PROGRAM that requires superuser rights.  COPY FROM
already provides a similar callback for logical replication.  For COPY
TO, the callback is triggered when we are ready to send a row in
CopySendEndOfRow(), which is the same code path as when sending a row
to a frontend or a pipe/file.

A small test module, test_copy_callbacks, is added to provide some
coverage for this facility.

Author: Bilva Sanaba, Nathan Bossart
Discussion: https://postgr.es/m/253C21D1-FCEB-41D9-A2AF-E6517015B7D7@amazon.com

2 years agoHarden memory context allocators against bogus chunk pointers.
Tom Lane [Mon, 10 Oct 2022 22:45:34 +0000 (18:45 -0400)]
Harden memory context allocators against bogus chunk pointers.

Before commit c6e0fe1f2, functions such as AllocSetFree could pretty
safely presume that they were given a valid chunk pointer for their
own type of context, because the indirect call through a memory
context object and method struct would be very unlikely to work
otherwise.  But now, if pfree() is mistakenly invoked on a pointer
to garbage, we have three chances in eight of ending up at one of
these functions.  That means we need to take extra measures to
verify that we are looking at what we're supposed to be looking at,
especially in debug builds.

Hence, add code to verify that the chunk's back-link to a block header
leads to a memory context object that satisfies the right sort of
IsA() check.  This is still a bit weaker than what we did before,
but for the moment assume that an IsA() check is sufficient.

As a compromise between speed and safety, implement these checks
as Asserts when dealing with small chunks but plain test-and-elogs
when dealing with large (external) chunks.  The latter case should
not be too performance-critical, but the former case probably is.
In slab.c, all chunks are small; but nonetheless use a plain test
in SlabRealloc, because that is certainly not performance-critical,
indeed we should be suspicious that it's being called in error.

In aset.c, additionally add some assertions that the "value" field
of the chunk header is within the small range allowed for freelist
indexes.  Without that, we might find ourselves trying to wipe
most of memory when CLOBBER_FREED_MEMORY is enabled, or scribbling
on a "freelist header" that's far away from the context object.

Eventually, field experience might show us that it's smarter for
these tests to be active always, but for now we'll try to get
away with just having them as assertions.

While at it, also be more uniform about asserting that context
objects passed as parameters are of the type we expect.  Some
places missed that altogether, and slab.c was for no very good
reason doing it differently from the other allocators.

Discussion: https://postgr.es/m/3578387.1665244345@sss.pgh.pa.us

2 years agoSimplify our Assert infrastructure a little.
Tom Lane [Mon, 10 Oct 2022 19:16:56 +0000 (15:16 -0400)]
Simplify our Assert infrastructure a little.

Remove the Trap and TrapMacro macros, which were nearly unused
and confusingly had the opposite condition polarity from the
otherwise-functionally-equivalent Assert macros.

Having done that, it's very hard to justify carrying the errorType
argument of ExceptionalCondition, so drop that too, and just
let it assume everything's an Assert.  This saves about 64K
of code space as of current HEAD.

Discussion: https://postgr.es/m/3928703.1665345117@sss.pgh.pa.us

2 years agoRemove unnecessary semicolons after goto labels
John Naylor [Mon, 10 Oct 2022 08:08:38 +0000 (15:08 +0700)]
Remove unnecessary semicolons after goto labels

According to the C standard, a label must followed by a statement.
If there was ever a time we needed an empty statement here, it was
a long time ago.

Japin Li

Reviewed by Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/MEYP282MB16690F40189A4F060B41D56DB65E9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoUse C library functions instead of Abs() for int64
Peter Eisentraut [Mon, 10 Oct 2022 06:51:07 +0000 (08:51 +0200)]
Use C library functions instead of Abs() for int64

Instead of Abs() for int64, use the C standard functions labs() or
llabs() as appropriate.  Define a small wrapper around them that
matches our definition of int64.  (labs() is C90, llabs() is C99.)

Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com

2 years agopgstat: Prevent stats reset from corrupting slotname by removing slotname
Andres Freund [Sat, 8 Oct 2022 16:33:23 +0000 (09:33 -0700)]
pgstat: Prevent stats reset from corrupting slotname by removing slotname

Previously PgStat_StatReplSlotEntry contained the slotname, which was mainly
used when writing out the stats during shutdown, to identify the slot in the
serialized data (at runtime the index in ReplicationSlotCtl->replication_slots
is used, but that can change during a restart). Unfortunately the slotname was
overwritten when the slot's stats were reset.

That turned out to only cause "real" problems if the slot was active during
the reset, triggering an assertion failure at the next
pgstat_report_replslot(). In other paths the stats were re-initialized during
pgstat_acquire_replslot().

Fix this by removing slotname from PgStat_StatReplSlotEntry. Instead we can
get the slot's name from the slot itself. Besides fixing a bug, this also is
architecturally cleaner (a name is not really statistics). This is safe
because stats, for a slot removed while shut down, will not be restored at
startup.

In 15 the slotname is not removed, but renamed, to avoid changing the stats
format. In master, bump PGSTAT_FILE_FORMAT_ID.

This commit does not contain a test for the fix. I think this can only be
tested by a tap test starting pg_recvlogical in the background and checking
pg_recvlogical's output. That type of test is notoriously hard to be reliable,
so committing it shortly before the release is wrapped seems like a bad idea.

Reported-by: Jaime Casanova <jcasanov@systemguards.com.ec>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/YxfagaTXUNa9ggLb@ahch-to
Backpatch: 15-, where the bug was introduced in 5891c7a8ed8f

2 years agoUse fabsf() instead of Abs() or fabs() where appropriate
Peter Eisentraut [Sat, 8 Oct 2022 11:41:18 +0000 (13:41 +0200)]
Use fabsf() instead of Abs() or fabs() where appropriate

This function is new in C99.

Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com

2 years agoautoconf: Rely on ar supporting index creation
Andres Freund [Fri, 7 Oct 2022 18:53:39 +0000 (11:53 -0700)]
autoconf: Rely on ar supporting index creation

This way we don't need RANLIB anymore, making it a bit simpler for the meson
build to generate Makefile.global for PGXS compatibility.

FreeBSD, NetBSD, OpenBSD, the only platforms where we didn't use AROPT=crs,
all have supported the 's' option for a long time.

On macOS we ran ranlib after installing a static library. This was added a
long time ago, in 58ad65ec2def. I cannot reproduce an issue in more recent
macOS versions. This is removed now.

Based on discussion with Tom, I left the 'touch' at the end of static
libraries generation, added in 826eff57c4c, in place. While it looks like
current versions of Apple's ar/ranlib don't need it, it was needed not too
long ago.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de

2 years agoFix self-referencing foreign keys with partitioned tables
Alvaro Herrera [Fri, 7 Oct 2022 17:37:48 +0000 (19:37 +0200)]
Fix self-referencing foreign keys with partitioned tables

There are a number of bugs in this area.  Two of them are fixed here,
namely:
1. get_relation_idx_constraint_oid does not restrict the type of
   constraint that's returned, so with sufficient bad luck it can
   return the OID of a foreign key constraint.  This has the effect that
   a primary key in a partition can end up as a child of a foreign key,
   which makes no sense (it needs to be the child of the equivalent
   primary key.)
   Change the API contract so that only index-backed constraints are
   returned, mimicking get_constraint_index().

2. Both CloneFkReferenced and CloneFkReferencing clone a
   self-referencing foreign key, so the partition ends up with
   a duplicate foreign key.  Change the former function to ignore such
   constraints.

Add some tests to verify that things are better now.  (However, these
new tests show some additional misbehavior that will be fixed later --
namely that there's a constraint marked NOT VALID.)

Backpatch to 12, where these constraints are possible at all.

Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://postgr.es/m/20220603154232.1715b14c@karst

2 years agoConvert macros to static inline functions (rel.h)
Peter Eisentraut [Fri, 7 Oct 2022 14:06:59 +0000 (16:06 +0200)]
Convert macros to static inline functions (rel.h)

Reviewed-by: Amul Sul <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com

2 years agoRemove unnecessary uses of Abs()
Peter Eisentraut [Fri, 7 Oct 2022 11:28:38 +0000 (13:28 +0200)]
Remove unnecessary uses of Abs()

Use C standard abs() or fabs() instead.

Reviewed-by: Zhang Mingli <zmlpostgres@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/4beb42b5-216b-bce8-d452-d924d5794c63%40enterprisedb.com

2 years agoAdd -Wshadow=compatible-local to the standard compilation flags
David Rowley [Fri, 7 Oct 2022 03:50:31 +0000 (16:50 +1300)]
Add -Wshadow=compatible-local to the standard compilation flags

Since cd4e8caaa, we've been able to build the source tree with
-Wshadow=compatible-local without any warnings.  Lots of work was done by
Justin Pryzby and I (David) to get all our code to compile warning free
with that flag.  In that process, 2 bugs (16d69ec29 and af7d270dd) were
discovered and fixed.  Additionally, "git log --grep=shadow" shows that
there is no shortage of other bugs that have been fixed over the years
which were caused by variable shadowing.

In light of the above, it seems very much worthwhile to add at least
-Wshadow=compatible-local to our standard compilation flags.  We *may*
want to go further and take this to -Wshadow=local in the future, but
we're not ready for that today, so let's add -Wshadow=compatible-local now
to help make sure we don't introduce further local variable shadowing.

Author: Andres Freund
Discussion: https://postgr.es/m/20221006003920.6xlqaoccxwisza5k@awork3.anarazel.de

2 years agoImprove our ability to detect bogus pointers passed to pfree et al.
Tom Lane [Fri, 7 Oct 2022 01:23:52 +0000 (21:23 -0400)]
Improve our ability to detect bogus pointers passed to pfree et al.

Commit c6e0fe1f2 was a shade too trusting that any pointer passed
to pfree, repalloc, etc will point at a valid chunk.  Notably,
passing a pointer that was actually obtained from malloc tended
to result in obscure assertion failures, if not worse.  (On FreeBSD
I've seen such mistakes take down the entire cluster, seemingly as
a result of clobbering shared memory.)

To improve matters, extend the mcxt_methods[] array so that it
has entries for every possible MemoryContextMethodID bit-pattern,
with the currently unassigned ID codes pointing to error-reporting
functions.  Then, fiddle with the ID assignments so that patterns
likely to be associated with bad pointers aren't valid ID codes.
In particular, we should avoid assigning bit patterns 000 (zeroed
memory) and 111 (wipe_mem'd memory).

It turns out that on glibc (Linux), malloc uses chunk headers that
have flag bits in the same place we keep MemoryContextMethodID,
and that the bit patterns 000, 001, 010 are the only ones we'll
see as long as the backend isn't threaded.  So we can have very
robust detection of pfree'ing a malloc-assigned block on that
platform, at least so long as we can refrain from using up those
ID codes.  On other platforms, we don't have such a good guarantee,
but keeping 000 reserved will be enough to catch many such cases.

While here, make GetMemoryChunkMethodID() local to mcxt.c, as there
seems no need for it to be exposed even in memutils_internal.h.

Patch by me, with suggestions from Andres Freund and David Rowley.

Discussion: https://postgr.es/m/2910981.1665080361@sss.pgh.pa.us

2 years agomeson: Add support for building with precompiled headers
Andres Freund [Fri, 7 Oct 2022 00:19:30 +0000 (17:19 -0700)]
meson: Add support for building with precompiled headers

This substantially speeds up building for windows, due to the vast amount of
headers included via windows.h. A cross build from linux targetting mingw goes
from

994.11user 136.43system 0:31.58elapsed 3579%CPU
to
422.41user 89.05system 0:14.35elapsed 3562%CPU

The wins on windows are similar-ish (but I don't have a system at hand just
now for actual numbers). Targetting other operating systems the wins are far
smaller (tested linux, macOS, FreeBSD).

For now precompiled headers are disabled by default, it's not clear how well
they work on all platforms. E.g. on FreeBSD gcc doesn't seem to have working
support, but clang does.

When doing a full build precompiled headers are only beneficial for targets
with multiple .c files, as meson builds a separate precompiled header for each
target (so that different compilation options take effect). This commit
therefore only changes target with at least two .c files to use precompiled
headers.

Because this commit adds b_pch=false to the default_options new build
directories will have precompiled headers disabled by default, however
existing build directories will continue use the default value of b_pch, which
is true.

Note that using precompiled headers with ccache requires setting
CCACHE_SLOPPINESS=pch_defines,time_macros to get hits.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CA+hUKG+50eOUbN++ocDc0Qnp9Pvmou23DSXu=ZA6fepOcftKqA@mail.gmail.com
Discussion: https://postgr.es/m/c5736f70-bb6d-8d25-e35c-e3d886e4e905@enterprisedb.com
Discussion: https://postgr.es/m/20190826054000.GE7005%40paquier.xyz

2 years agoCreate subscription stats entry at CREATE SUBSCRIPTION time
Andres Freund [Thu, 6 Oct 2022 23:45:20 +0000 (16:45 -0700)]
Create subscription stats entry at CREATE SUBSCRIPTION time

Previously, the subscription stats entry was created when the first
stats, i.e., an error on apply worker or tablesync worker,  were
reported. Therefore, the stats_reset field was not updated by
pg_stat_reset_subscription_stats() if the stats entry was not
populated yet, which was different behavior than other statistics.

This change creates the subscription stats entry and initializes it at
CREATE SUBSCRIPTION time.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_Zqd-e5imT_3-ZiQv1cfsWuy16OJTiUaCvqpq4V7GVdSg@mail.gmail.com

2 years agoFix final warnings produced by -Wshadow=compatible-local
David Rowley [Fri, 7 Oct 2022 00:13:27 +0000 (13:13 +1300)]
Fix final warnings produced by -Wshadow=compatible-local

I thought I had these in d8df67bb1, but per report from Andres Freund, I
missed some.

Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20221005214052.c4tkudawyp5wxt3c@awork3.anarazel.de

2 years agowindows: Adjust FD_SETSIZE via commandline define
Andres Freund [Thu, 6 Oct 2022 20:03:31 +0000 (13:03 -0700)]
windows: Adjust FD_SETSIZE via commandline define

When using precompiled headers, we cannot pre-define macros for the system
headers from within .c files, as headers are already processed before
the #define in the C file is reached. But we can pre-define using
-DFD_SETSIZE, as long as that's also used when building the precompiled header.

A few files #define FD_SETSIZE 1024 on windows, as the default is only 64. I
am hesitant to change FD_SETSIZE globally on windows, due to
src/backend/port/win32/socket.c using it to size on-stack arrays. Instead add
-DFD_SETSIZE=1024 when building the specific targets needing it.

We likely should move away from using select() in those places, but that's a
larger change.

Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20221005190829.lda7ttalh4mzrvf4@awork3.anarazel.de
Discussion: https://postgr.es/m/CA+hUKG+50eOUbN++ocDc0Qnp9Pvmou23DSXu=ZA6fepOcftKqA@mail.gmail.com
Discussion: https://postgr.es/m/20190826054000.GE7005%40paquier.xyz

2 years agomeson: Fix two comments
Andres Freund [Thu, 6 Oct 2022 19:22:36 +0000 (12:22 -0700)]
meson: Fix two comments

Author: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/CAEG8a3KxObc9g8NTzx1kX0Auf=J7FNiubYZXSK6G5wv5ShmP6A@mail.gmail.com

2 years agoRemove MemoryContextContains().
Tom Lane [Thu, 6 Oct 2022 17:35:31 +0000 (13:35 -0400)]
Remove MemoryContextContains().

MemoryContextContains is no longer reliable in the wake of c6e0fe1f2,
because there's no longer very much redundancy in chunk headers.
(It wasn't *completely* reliable even before that, as there was a
chance of a false positive if you passed it something that didn't
point to an mcxt chunk at all.  But it was generally good enough.)

Hence, remove it.  There is no remaining core code that requires it.
Extensions that have been using it might be able to substitute a
test like "GetMemoryChunkContext(ptr) == context", recognizing that
this explicitly requires that the pointer point to some chunk.

Tom Lane and David Rowley

Discussion: https://postgr.es/m/1913788.1664898906@sss.pgh.pa.us

2 years agoRemove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.
Tom Lane [Thu, 6 Oct 2022 17:27:34 +0000 (13:27 -0400)]
Remove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.

MemoryContextContains is no longer reliable in the wake of c6e0fe1f2,
so we need to get rid of these uses.

It appears that there's no really good reason to force the result of
an aggregate's finalfn or serialfn to be allocated in the per-tuple
context.  The only other plausible case is that the result points to
or into the aggregate's transition value, and that's fine because it
will last as long as we need it to.  (This conclusion depends on the
assumption that finalfns are not allowed to scribble on the transition
value, but we've long required that.)  So we can just drop the
MemoryContextContains plus datumCopy business, although we do need
to take care to not return a read-write pointer when the transition
value is an expanded datum.

Likewise, we don't really need to force the result of a window
function to be in the output context.  In this case, the plausible
alternative is that it's pointing into the temporary tuple slot used
by WinGetFuncArgInPartition or WinGetFuncArgInFrame (since those
functions could return such a pointer, which might become the window
function's result).  That will hold still for long enough, unless
there is another window function using the same WindowObject.
I'm content to always perform a datumCopy when there's more than one
such function.

On net, these changes should provide small speed improvements as well
as removing problematic code.

Tom Lane and David Rowley

Discussion: https://postgr.es/m/1913788.1664898906@sss.pgh.pa.us

2 years agoTake care to de-duplicate entries in standby.c's table of locks.
Tom Lane [Thu, 6 Oct 2022 16:27:36 +0000 (12:27 -0400)]
Take care to de-duplicate entries in standby.c's table of locks.

The RecoveryLockLists data structure, which tracks all exclusive
locks that the startup process is holding on behalf of transactions
being replayed, did not have any provision for avoiding duplicate
entries for the same lock.  Maybe that was okay when the code was
first written.  However, modern practice is for checkpoints to
write fresh lists of all active exclusive locks into the WAL.
Thus, an exclusive lock that survives across multiple checkpoints
causes bloat in standbys' startup processes.  If there are a lot
of such locks this can look like a memory leak, and it's even
possible to drive the startup process into a palloc failure from
an over-length List.

To fix, use a hash table instead of simple lists to track the
locks being held.  Allowing for dynahash overhead, this requires
a little more space per lock than the old way (although it's the
same size as what we were allocating prior to c6e0fe1f2).  It's
probably a shade slower too.  However, testing indicates that the
penalty is negligible on ordinary workloads, so let's make this
change to improve robustness in extreme cases.

Patch by me, per report from Dmitriy Kuzmin.  No back-patch
(for now anyway), since it seems that a significant improvement
would only occur in corner cases.

Discussion: https://postgr.es/m/CAHLDt=_ts0A7Agn=hCpUh+RCFkxd+G6uuT=kcTfqFtGur0dp=A@mail.gmail.com

2 years agoRemove useless character-length checks in contrib/ltree.
Tom Lane [Thu, 6 Oct 2022 15:18:32 +0000 (11:18 -0400)]
Remove useless character-length checks in contrib/ltree.

The t_iseq() macro does not need to be guarded by a character
length check (at least when the comparison value is an ASCII
character, as its documentation requires).  Some portions of
contrib/ltree hadn't read that memo, so simplify them.

The last change in gettoken_query,

-                else if (charlen == 1 && !t_iseq(state->buf, ' '))
+                else if (!t_iseq(state->buf, ' '))

looks like it's actually a bug fix: I doubt that the intention
was to silently ignore multibyte characters as if they were
whitespace.  I'm not tempted to back-patch though, because this
will have the effect of tightening what is allowed in ltxtquery
strings.

Discussion: https://postgr.es/m/2548310.1664999615@sss.pgh.pa.us

2 years agoIntroduce t_isalnum() to replace t_isalpha() || t_isdigit() tests.
Tom Lane [Thu, 6 Oct 2022 15:08:56 +0000 (11:08 -0400)]
Introduce t_isalnum() to replace t_isalpha() || t_isdigit() tests.

ts_locale.c omitted support for "isalnum" tests, perhaps on the
grounds that there were initially no use-cases for that.  However,
both ltree and pg_trgm need such tests, and we do also have one
use-case now in the core backend.  The workaround of testing
isalpha and isdigit separately seems quite inefficient, especially
when dealing with multibyte characters; so let's fill in the
missing support.

Discussion: https://postgr.es/m/2548310.1664999615@sss.pgh.pa.us

2 years agoFix comment in xlogprefetcher.c
Michael Paquier [Thu, 6 Oct 2022 11:25:02 +0000 (20:25 +0900)]
Fix comment in xlogprefetcher.c

Author: Sho Kato
Discussion: https://postgr.es/m/TYCPR01MB684954052EC534A3261B29249F5C9@TYCPR01MB6849.jpnprd01.prod.outlook.com

2 years agoRefactor TAP test authentication/001_password.pl
Michael Paquier [Thu, 6 Oct 2022 00:40:16 +0000 (09:40 +0900)]
Refactor TAP test authentication/001_password.pl

The test is changed to test for connection strings rather than specific
roles, and the reset logic of pg_hba.conf is extended so as the database
and user name entries can be directly specified.  This is aimed at being
used as a base for more test scenarios of pg_hba.conf and authentication
paths.

Author: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/Yz0xO0emJ+mxtj2a@paquier.xyz

2 years agoFix final compiler warning produced by -Wshadow=compatible-local
David Rowley [Wed, 5 Oct 2022 21:19:36 +0000 (10:19 +1300)]
Fix final compiler warning produced by -Wshadow=compatible-local

We're now able to compile the entire tree with -Wshadow=compatible-local
without any compiler warnings.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.com

2 years agoAdd optional parameter to PG_TRY() macros
David Rowley [Wed, 5 Oct 2022 21:08:31 +0000 (10:08 +1300)]
Add optional parameter to PG_TRY() macros

This optional parameter can be specified in cases where there are nested
PG_TRY() statements within a function in order to stop the compiler from
issuing warnings about shadowed local variables when compiling with
-Wshadow.  The optional parameter is used as a suffix on the variable
names declared within the PG_TRY(), PG_CATCH(), PG_FINALLY() and
PG_END_TRY() macros.  The parameter, if specified, must be the same in
each component macro of the given PG_TRY() block.

This also adjusts the single case where we have nested PG_TRY() statements
to add a parameter to the inner-most PG_TRY().

This reduces the number of compiler warnings when compiling with
-Wshadow=compatible-local from 5 down to 1.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.com

2 years agodoc: clarify description for log_startup_progress_interval
Bruce Momjian [Wed, 5 Oct 2022 19:53:40 +0000 (15:53 -0400)]
doc:  clarify description for log_startup_progress_interval

Reported-by: Elena Indrupskaya
Discussion: https://postgr.es/m/0af43b49-1646-93d0-ccf1-bb3c635c8c6f@postgrespro.ru

Author: Elena Indrupskaya

Backpatch-through: 15

2 years agotests: Restrict pg_locks queries in advisory_locks.sql to current database
Andres Freund [Wed, 5 Oct 2022 17:44:38 +0000 (10:44 -0700)]
tests: Restrict pg_locks queries in advisory_locks.sql to current database

Otherwise testing an existing installation can fail, if there are other locks,
e.g. from one of the isolation tests.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20221003234111.4ob7yph6r4g4ywhu@awork3.anarazel.de

2 years agotests: Rename conflicting role names
Andres Freund [Wed, 5 Oct 2022 17:43:13 +0000 (10:43 -0700)]
tests: Rename conflicting role names

These cause problems when running installcheck-world USE_MODULE_DB=1 with -j.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20221003234111.4ob7yph6r4g4ywhu@awork3.anarazel.de

2 years agomeson: Add windows resource files
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: Add windows resource files

The generated resource files aren't exactly the same ones as the old
buildsystems generate. Previously "InternalName" and "OriginalFileName" were
mostly wrong / not set (despite being required), but that was hard to fix in
at least the make build. Additionally, the meson build falls back to a
"auto-generated" description when not set, and doesn't set it in a few cases -
unlikely that anybody looks at these descriptions in detail.

Author: Andres Freund <andres@anarazel.de>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2 years agomeson: ecpg: Split definition of static and shared libraries
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: ecpg: Split definition of static and shared libraries

Required for correct resource file generation, as the resource files should
only be added to the shared library.

This also fixes a bunch of issues in the .pc files.

Previously I tried to avoid building sources twice, once for the static and
once for the shared libraries. We could still do so, but it's not clear that
it's worth the complication.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20220927011951.j3h4o7n6bhf7dwau@awork3.anarazel.de

2 years agomeson: libpq: Revise static / shared library setup
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: libpq: Revise static / shared library setup

Improvements:
- we don't need -DFRONTEND for libpq anymore since 1d77afefbd1
- the .pc file contents for a static libpq were wrong (referencing
  {pgport, common}_shlib)
- incidentally fixes meson not supporting link_whole on AIX yet
- added explanatory comments

Previously I tried to avoid building libpq's sources twice, once for the
static and once for the shared library. We could still do so, but it's not
clear that it's worth the complication.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2 years agomeson: docs: Add xml{lint,proc} wrapper to collect dependencies
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: docs: Add xml{lint,proc} wrapper to collect dependencies

meson/ninja do not support specifying dependencies via globs (as those make it
significantly more expensive to check the current build state). Instead
targets should emit dependency information when running that then can be
cheaply re-checked during future builds.

To handle xmllint and xsltproc invocations in the docs, add and use a wrapper
that uses --load-trace to collect dependency information.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/c5736f70-bb6d-8d25-e35c-e3d886e4e905@enterprisedb.com

2 years agoFix whitespace
Peter Eisentraut [Wed, 5 Oct 2022 13:06:43 +0000 (15:06 +0200)]
Fix whitespace

2 years agoRename shadowed local variables
David Rowley [Wed, 5 Oct 2022 08:01:41 +0000 (21:01 +1300)]
Rename shadowed local variables

In a similar effort to f01592f91, here we mostly rename shadowed local
variables to remove the warnings produced when compiling with
-Wshadow=compatible-local.

This fixes 63 warnings and leaves just 5.

Author: Justin Pryzby, David Rowley
Reviewed-by: Justin Pryzby
Discussion https://postgr.es/m/20220817145434.GC26426%40telsasoft.com

2 years agoRemove definition of JUMBLE_SIZE from queryjumble.h
Michael Paquier [Wed, 5 Oct 2022 05:27:50 +0000 (14:27 +0900)]
Remove definition of JUMBLE_SIZE from queryjumble.h

The same exists in queryjumble.c, and it is used only locally in this
file so let's remove the definition in the header.

Author: Tatsu Nakamori
Reviewed-by: Tom Lane, Julien Rouhaud
Discussion: https://postgr.es/m/bb4ebd0412da9b1ac87a5eb2a3646bf1@oss.nttdata.com

2 years agoUse macros from xlog_internal.h for WAL segment logic in pg_resetwal
Michael Paquier [Wed, 5 Oct 2022 05:10:13 +0000 (14:10 +0900)]
Use macros from xlog_internal.h for WAL segment logic in pg_resetwal

When scanning for the end of WAL, pg_resetwal has been maintaining its
own internal logic to calculate segment numbers and to parse a WAL
segment name for its timeline and segment number.  The code claimed for
example that XLogFromFileName() cannot be used because it lacks the
possibility of specifying a WAL segment size, which is not the case
since fc49e24, that has made the WAL segment size configurable at
initialization time, extending this routine to do so.

Similarly, this switches one segment number calculation to use
XLByteToSeg() rather than the same logic present in xlog_internal.h.
While on it, switch to TimeLineID in pg_resetwal.c for TLI numbers
parsed from segment names, to be more consistent with
XLogFromFileName().  The maths are exactly the same, but the code gets
simplified.

Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CALj2ACX+E_jnwqH_jmjhNG8BczJTNRTOLpw8K1CB1OcB48MJ8w@mail.gmail.com

2 years agoAdd a few new patterns to the tab completion of psql
Michael Paquier [Wed, 5 Oct 2022 02:46:10 +0000 (11:46 +0900)]
Add a few new patterns to the tab completion of psql

This improves the tab completion of psql on a few points:
- Provide a list of subscriptions on \dRs.
- Provide a list of publications on \dRp.
- Add CURRENT_ROLE, CURRENT_USER, SESSION_USER when OWNER TO is provided
at the end of a query (as defined by RoleSpec in gram.y).

Author: Vignesh C
Reviewed-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/CALDaNm3toRBt6c6saY3174f7CsGztXRvVvfWbikkJEXY7x5WAA@mail.gmail.com

2 years agoFix comment in guc_tables.c
Michael Paquier [Tue, 4 Oct 2022 06:39:41 +0000 (15:39 +0900)]
Fix comment in guc_tables.c

s/ERROR_HANDLING/ERROR_HANDLING_OPTIONS/.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+PtDj3CV+f0pVisc0XYMi2LHGBpQxQWtF0FjiSVN_nV17Q@mail.gmail.com

2 years agoCleanup useless assignments and checks
Michael Paquier [Tue, 4 Oct 2022 04:16:23 +0000 (13:16 +0900)]
Cleanup useless assignments and checks

This cleans up a couple of areas:
- Remove XLogSegNo calculation for the last WAL segment in backup in
xlog.c (7d70809 has moved this logic entirely to xlogbackup.c when
building the contents of the backup history file).
- Remove check on log_min_duration in analyze.c, as it is already true
where this code path is reached.
- Simplify call to find_option() in guc.c.

Author: Ranier Vilela
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAEudQArCDQQiPiFR16=yu9k5s2tp4tgEe1U1ZbkW4ofx81AWWQ@mail.gmail.com

2 years agoAdd filtering capability for cross-version pg_upgrade tests
Michael Paquier [Tue, 4 Oct 2022 01:11:08 +0000 (10:11 +0900)]
Add filtering capability for cross-version pg_upgrade tests

This commit expands the TAP tests of pg_upgrade when running these with
different major versions for the "old" cluster (to-be-upgraded) and the
"new" cluster (upgraded-to), by backporting some of the buildfarm
facilities directory into the script:
- Remove comments from the dump files, avoiding version-dependent
information.
- Remove empty lines from the dump files.
- Use --extra-float-digits=0 in the pg_dump command, when using an "old"
cluster with version equal to or lower than v11.
- Use --wal-segsize and --allow-group-access in initdb only when the
"old" cluster is equal to or higher than v11.

This allows the tests to pass down to v14 with the main regression test
suite, while v9.5~13 still generate some diffs, but these are minimal
compared to what happened before this commit.  Much more could be done,
especially around dump differences with function and procedures (these
can also be avoided with direct manipulation of the dumps loaded, for
example, in a way similar to the buildfarm), but at least the basics are
in place now.

Reviewed-by: Justin Pryzby, Anton A. Melnikov
Discussion: https://postgr.es/m/Yox1ME99GhAemMq1@paquier.xyz

2 years agomeson: llvm: Use llvm-config's --cxxflags when building llvmjit
Andres Freund [Mon, 3 Oct 2022 21:52:51 +0000 (14:52 -0700)]
meson: llvm: Use llvm-config's --cxxflags when building llvmjit

Otherwise we don't use LLVM's flags when building llvmjit_wrap.cpp and
llvmjit_inline.cpp. That can cause compile time failures if the C++ compiler
doesn't default to a new enough C++ standards version and link time failures
due to ABI influencing flags like -fno-rtti.

2 years agoFix psql's behavior with \g for a multiple-command string.
Tom Lane [Mon, 3 Oct 2022 19:07:10 +0000 (15:07 -0400)]
Fix psql's behavior with \g for a multiple-command string.

The pre-v15 behavior was to discard all but the last result,
but with the new behavior of printing all results by default,
we will send each such result to the \g file.  However,
we're still opening and closing the \g file for each result,
so you lose all but the last result anyway.  Move the output-file
state up to ExecQueryAndProcessResults so that we open/close the
\g file only once per command string.

To support this without changing other behavior, we must
adjust PrintQueryResult to have separate FILE * arguments
for query and status output (since status output has never
gone to the \g file).  That in turn makes it a good idea
to push the responsibility for fflush'ing output down to
PrintQueryTuples and PrintQueryStatus.

Also fix an infinite loop if COPY IN/OUT is attempted in \watch.
We used to reject that, but that error exit path got broken
somewhere along the line in v15.  There seems no real reason
to reject it anyway as the code now stands, so just remove
the error exit and make sure that COPY OUT data goes to the
right place.

Also remove PrintQueryResult's unused is_watch parameter,
and make some other cosmetic cleanups (adjust obsolete
comments, break some overly-long lines).

Daniel Vérité and Tom Lane

Discussion: https://postgr.es/m/4333844c-2244-4d6e-a49a-1d483fbe304f@manitou-mail.org

2 years agomeson: respect -Dldap=disabled
Andres Freund [Mon, 3 Oct 2022 17:13:12 +0000 (10:13 -0700)]
meson: respect -Dldap=disabled

I noticed during some manual testing that -Dldap=disabled (or
--auto-features=disabled) doesn't disable ldap if available - that's obviously
wrong.

2 years agoRevert "Optimize order of GROUP BY keys".
Tom Lane [Mon, 3 Oct 2022 14:56:16 +0000 (10:56 -0400)]
Revert "Optimize order of GROUP BY keys".

This reverts commit db0d67db2401eb6238ccc04c6407a4fd4f985832 and
several follow-on fixes.  The idea of making a cost-based choice
of the order of the sorting columns is not fundamentally unsound,
but it requires cost information and data statistics that we don't
really have.  For example, relying on procost to distinguish the
relative costs of different sort comparators is pretty pointless
so long as most such comparator functions are labeled with cost 1.0.
Moreover, estimating the number of comparisons done by Quicksort
requires more than just an estimate of the number of distinct values
in the input: you also need some idea of the sizes of the larger
groups, if you want an estimate that's good to better than a factor of
three or so.  That's data that's often unknown or not very reliable.
Worse, to arrive at estimates of the number of calls made to the
lower-order-column comparison functions, the code needs to make
estimates of the numbers of distinct values of multiple columns,
which are necessarily even less trustworthy than per-column stats.
Even if all the inputs are perfectly reliable, the cost algorithm
as-implemented cannot offer useful information about how to order
sorting columns beyond the point at which the average group size
is estimated to drop to 1.

Close inspection of the code added by db0d67db2 shows that there
are also multiple small bugs.  These could have been fixed, but
there's not much point if we don't trust the estimates to be
accurate in-principle.

Finally, the changes in cost_sort's behavior made for very large
changes (often a factor of 2 or so) in the cost estimates for all
sorting operations, not only those for multi-column GROUP BY.
That naturally changes plan choices in many situations, and there's
precious little evidence to show that the changes are for the better.
Given the above doubts about whether the new estimates are really
trustworthy, it's hard to summon much confidence that these changes
are better on the average.

Since we're hard up against the release deadline for v15, let's
revert these changes for now.  We can always try again later.

Note: in v15, I left T_PathKeyInfo in place in nodes.h even though
it's unreferenced.  Removing it would be an ABI break, and it seems
a bit late in the release cycle for that.

Discussion: https://postgr.es/m/TYAPR01MB586665EB5FB2C3807E893941F5579@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agoAdd authentication TAP test for peer authentication
Michael Paquier [Mon, 3 Oct 2022 07:42:25 +0000 (16:42 +0900)]
Add authentication TAP test for peer authentication

This commit introduces an authentication test for the peer method, as of
a set of scenarios with and without a user name map.  The script is
automatically skipped if peer is not supported in the environment where
this test is run, checking this behavior by attempting a connection
first on a cluster up and running.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/aa60994b-1c66-ca7a-dab9-9a200dbac3d2@amazon.com

2 years agoci: enable various runtime checks on FreeBSD and macOS
Andres Freund [Sun, 2 Oct 2022 00:04:13 +0000 (17:04 -0700)]
ci: enable various runtime checks on FreeBSD and macOS

Increase likelihood of discovering problems during CI rather the buildfarm by
defining -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES
-DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST on FreeBSD, and
-DRANDOMIZE_ALLOCATED_MEMORY on macOS.

FreeBSD and macoS are currently not CI bottlenecks, so we can afford to
increase their test times a bit.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20220910200542.GX31833@telsasoft.com

2 years agoci: macos: Reduce test concurrency
Andres Freund [Sat, 1 Oct 2022 23:55:16 +0000 (16:55 -0700)]
ci: macos: Reduce test concurrency

Test performance regresses noticably when using all cores. This is more
pronounced with meson than with autoconf, presumably because meson will
schedule the "full number" of tests more consistently.  8 seems to work
OK.

Discussion: https://postgr.es/m/20220927040208.l3shfcidovpzqxfh@awork3.anarazel.de
Backpatch: 15-, where CI was introduced

2 years agomeson: Add prefix=/usr/local/pgsql to default_options
Andres Freund [Sat, 1 Oct 2022 19:39:12 +0000 (12:39 -0700)]
meson: Add prefix=/usr/local/pgsql to default_options

autoconf set PREFIX to /usr/local/pgsql, so using the same default for meson
makes sense. The effect on windows is that installation defaults to installing
to C:/usr/local/pgsql rather than meson's default of C:/, which doesn't seem
perfect, but OK enough.

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
Author: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/CAEG8a3LGWE-gG6vuddmH91RORhi8gWs0mMB-hcTmP3_NVgM7dg@mail.gmail.com

2 years agomeson: windows: Determine path to tmp_install + prefix using meson
Andres Freund [Sat, 1 Oct 2022 19:39:12 +0000 (12:39 -0700)]
meson: windows: Determine path to tmp_install + prefix using meson

Previously some paths (like c:\ or d:/) worked, but plenty others (like
/path/to or //computer/share/path) didn't. As we'd like to change the default
prefix to /usr/local/pgsql, that's a problem.

Instead of trying to do this in meson.build, call out to the implementation
meson install uses. This isn't pretty, but it's more reliable than what we had
before.

Discussion: https://postgr.es/CAEG8a3LGWE-gG6vuddmH91RORhi8gWs0mMB-hcTmP3_NVgM7dg@mail.gmail.com

2 years agoFix tiny memory leaks
Peter Eisentraut [Sat, 1 Oct 2022 10:48:24 +0000 (12:48 +0200)]
Fix tiny memory leaks

Both check_application_name() and check_cluster_name() use
pg_clean_ascii() but didn't release the memory.  Depending on when the
GUC is set, this might be cleaned up at some later time or it would
leak postmaster memory once.  In any case, it seems better not to have
to rely on such analysis and make the code locally robust.  Also, this
makes Valgrind happier.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Jacob Champion <jchampion@timescale.com>
Discussion: https://www.postgresql.org/message-id/CAD21AoBmFNy9MPfA0UUbMubQqH3AaK5U3mrv6pSeWrwCk3LJ8g@mail.gmail.com

2 years agodoc: Fix some grammar and typos
Michael Paquier [Sat, 1 Oct 2022 06:28:02 +0000 (15:28 +0900)]
doc: Fix some grammar and typos

This fixes some areas related to logical replication and custom RMGRs.

Author: Ekaterina Kiryanova
Discussion: https://postgr.es/m/fa4773f1-1396-384a-bcd7-85b5e013f399@postgrespro.ru
Backpatch-through: 15

2 years agomeson: mingw: Add -Wl,--disable-auto-import, enable when linking with readline
Andres Freund [Wed, 28 Sep 2022 17:19:00 +0000 (10:19 -0700)]
meson: mingw: Add -Wl,--disable-auto-import, enable when linking with readline

I hadn't ported -Wl,--disable-auto-import over from the win32 template as I
had focused on msvc for windows. The flag is desirable as it makes it easier
to find problems one would have with msvc, particularly useful during cross
compilation.

This turned out to be a somewhat happy accident, as it allowed me to realize
that readline actually works on windows these days, as long as auto imports to
enable. Therefore enable auto-import again as part of linking to readline.

We perhaps can come up with a better solution for the readline issue, but this
seems good enough for now.

Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de

2 years agoAvoid improbable PANIC during heap_update, redux.
Tom Lane [Fri, 30 Sep 2022 23:36:46 +0000 (19:36 -0400)]
Avoid improbable PANIC during heap_update, redux.

Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the relation, VM concerns
were dealt with only in the relatively-less-likely case that we
fail to conditionally lock the otherBuffer.  I think I'd believed
that we couldn't need to worry about it if the conditional lock
succeeds, which is true for the target buffer; but the otherBuffer
was unlocked for awhile so its bit might be set anyway.  So we need
to do the GetVisibilityMapPins dance, and then also recheck the
page's free space, in both cases.

Per report from Jaime Casanova.  Back-patch to v12 as the previous
patch was (although there's still no evidence that the bug is
reachable pre-v14).

Discussion: https://postgr.es/m/E1lWLjP-00006Y-Ml@gemulon.postgresql.org

2 years agomingw: Define PGDLLEXPORT as __declspec (dllexport) as done for msvc
Andres Freund [Wed, 28 Sep 2022 17:16:49 +0000 (10:16 -0700)]
mingw: Define PGDLLEXPORT as __declspec (dllexport) as done for msvc

While mingw would otherwise fall back to
__attribute__((visibility("default"))), that appears to only work as long as
no symbols are declared with __declspec(dllexport). But we can end up with
some, e.g. plpython's Py_Init.

It's quite possible we should do the same for cygwin, but I don't have a test
environment for that...

Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de
Discussion: http://postgr.es/m/20220928025242.ugf7t5ugxxgmkraa@awork3.anarazel.de

2 years agoAdjust PQsslAttributeNames() to match PQsslAttribute().
Tom Lane [Fri, 30 Sep 2022 14:26:47 +0000 (10:26 -0400)]
Adjust PQsslAttributeNames() to match PQsslAttribute().

Currently, PQsslAttributeNames() returns the same list of attribute
names regardless of its conn parameter.  This patch changes it to
have behavior parallel to what 80a05679d installed for PQsslAttribute:
you get OpenSSL's attributes if conn is NULL or is an SSL-encrypted
connection, or an empty list if conn is a non-encrypted connection.
The point of this is to have sensible connection-dependent behavior
in case we ever support multiple SSL libraries.  The behavior for
NULL can be defined as "the attributes for the default SSL library",
parallel to what PQsslAttribute(NULL, "library") does.

Since this is mostly just future-proofing, no back-patch.

Discussion: https://postgr.es/m/17625-fc47c78b7d71b534@postgresql.org

2 years agoFix tab-completion after commit 790bf615ddba
Alvaro Herrera [Fri, 30 Sep 2022 10:53:31 +0000 (12:53 +0200)]
Fix tab-completion after commit 790bf615ddba

I (Álvaro) broke tab-completion for GRANT .. ALL TABLES IN SCHEMA while
removing ALL from the publication syntax for schemas in the
aforementioned commit.  I also missed to update a bunch of
tab-completion rules for ALTER/CREATE PUBLICATION that match each
individual piece of ALL TABLES IN SCHEMA.  Repair those bugs.

While fixing up that commit, update a couple of outdated comments
related to the same change.

Backpatch to 15.

Author: Shi yu <shiy.fnst@fujitsu.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/OSZPR01MB6310FCE8609185A56344EED2FD559@OSZPR01MB6310.jpnprd01.prod.outlook.com

2 years agodoc: Fix PQsslAttribute docs for compression
Daniel Gustafsson [Fri, 30 Sep 2022 10:03:48 +0000 (12:03 +0200)]
doc: Fix PQsslAttribute docs for compression

The compression parameter to PQsslAttribute has never returned the
compression method used, it has always returned "on" or "off since
it was added in commit 91fa7b4719ac. Backpatch through v10.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/B9EC60EC-F665-47E8-A221-398C76E382C9@yesql.se
Backpatch-through: v10

2 years agoRemove useless argument from UnpinBuffer()
Michael Paquier [Fri, 30 Sep 2022 06:57:47 +0000 (15:57 +0900)]
Remove useless argument from UnpinBuffer()

The last caller of UnpinBuffer() that did not want to adjust
CurrentResourceOwner was removed in 2d115e4, and nothing has been
introduced in bufmgr.c to do the same thing since.  This simplifies 10
code paths.

Author: Aleksander Alekseev
Reviewed-by: Nathan Bossart, Zhang Mingli, Bharath Rupireddy
Discussion: https://postgr.es/m/CAJ7c6TOmmFpb6ohurLhTC7hKNJWGzdwf8s4EAtAZxD48g-e6Jw@mail.gmail.com

2 years agoci: Add 32bit build and test
Andres Freund [Thu, 29 Sep 2022 23:09:09 +0000 (16:09 -0700)]
ci: Add 32bit build and test

It's easy enough to make changes that break on 32bit platforms and few people
test that locally. Add a test for that to CI. LLVM is disabled on 32bit
because installing it would bloat the image size further.

The debian w/ meson task is fast enough that we can afford to test both.

Use the occasion of a separate run of the tests to run them under LANG=C,
we've recently discovered there's not a lot of testing in the buildfarm for
the case.

Discussion: https://postgr.es/m/4033181.1664395633@sss.pgh.pa.us

2 years agoFix bogus behavior of PQsslAttribute(conn, "library").
Tom Lane [Thu, 29 Sep 2022 21:28:09 +0000 (17:28 -0400)]
Fix bogus behavior of PQsslAttribute(conn, "library").

Commit ebc8b7d44 intended to change the behavior of
PQsslAttribute(NULL, "library"), but accidentally also changed
what happens with a non-NULL conn pointer.  Undo that so that
only the intended behavior change happens.  Clarify some
associated documentation.

Per bug #17625 from Heath Lord.  Back-patch to v15.

Discussion: https://postgr.es/m/17625-fc47c78b7d71b534@postgresql.org

2 years agoImprove wording of log messages triggered by max_slot_wal_keep_size.
Tom Lane [Thu, 29 Sep 2022 17:27:48 +0000 (13:27 -0400)]
Improve wording of log messages triggered by max_slot_wal_keep_size.

The one about "terminating process to release replication slot" told
you nothing about why that was happening.  The one about "invalidating
slot because its restart_lsn exceeds max_slot_wal_keep_size" told you
what was happening, but violated our message style guideline about
keeping the primary message short.  Add DETAIL/HINT lines to carry
the appropriate detail and make the two cases more uniform.

While here, fix bogus test logic in 019_replslot_limit.pl: if it timed
out without seeing the expected log message, no test failure would be
reported.  This is flat broken since commit 549ec201d removed the test
counts; even before that it was horribly bad style, since you'd only
get told that not all tests had been run.

Kyotaro Horiguchi, reviewed by Bertrand Drouvot; test fixes by me

Discussion: https://postgr.es/m/20211214.130456.2233153190058148084.horikyota.ntt@gmail.com

2 years agoUse actual backend IDs in pg_stat_get_backend_idset() and friends.
Tom Lane [Thu, 29 Sep 2022 16:14:39 +0000 (12:14 -0400)]
Use actual backend IDs in pg_stat_get_backend_idset() and friends.

Up to now, the ID values returned by pg_stat_get_backend_idset() and
used by pg_stat_get_backend_activity() and allied functions were just
indexes into a local array of sessions seen by the last stats refresh.
This is problematic for a few reasons.  The "ID" of a session can vary
over its existence, which is surprising.  Also, while these numbers
often match the "backend ID" used for purposes like temp schema
assignment, that isn't reliably true.  We can fairly cheaply switch
things around to make these numbers actually be the sessions' backend
IDs.  The added test case illustrates that with this definition, the
temp schema used by a given session can be obtained given its PID.

While here, delete some dead code that guarded against getting
a NULL return from pgstat_fetch_stat_local_beentry().  That can't
happen as long as the caller is careful to pass an in-range array
index, as all the callers are.  (This code may not have been dead
when written, but it surely is now.)

Nathan Bossart

Discussion: https://postgr.es/m/20220815205811.GA250990@nathanxps13

2 years agoUpdate comment in ExecInsert() regarding batch insertion.
Etsuro Fujita [Thu, 29 Sep 2022 07:55:00 +0000 (16:55 +0900)]
Update comment in ExecInsert() regarding batch insertion.

Remove the stale text that is a leftover from an earlier version of the
patch to add support for batch insertion, and adjust the wording in the
remaining text.

Back-patch to v14 where batch insertion came in.

Review and wording adjustment by Tom Lane.

Discussion: https://postgr.es/m/CAPmGK14goatHPHQv2Aeu_UTKqZ%2BBO%2BP%2Bzd3HKv5D%2BdyyfWKDSw%40mail.gmail.com

2 years agoIntroduce SYSTEM_USER
Michael Paquier [Thu, 29 Sep 2022 06:05:40 +0000 (15:05 +0900)]
Introduce SYSTEM_USER

SYSTEM_USER is a reserved keyword of the SQL specification that,
roughly described, is aimed at reporting some information about the
system user who has connected to the database server.  It may include
implementation-specific information about the means by the user
connected, like an authentication method.

This commit implements SYSTEM_USER as of auth_method:identity, where
"auth_method" is a keyword about the authentication method used to log
into the server (like peer, md5, scram-sha-256, gss, etc.) and
"identity" is the authentication identity as introduced by 9afffcb (peer
sets authn to the OS user name, gss to the user principal, etc.).  This
format has been suggested by Tom Lane.

Note that thanks to d951052, SYSTEM_USER is available to parallel
workers.

Bump catalog version.

Author: Bertrand Drouvot
Reviewed-by: Jacob Champion, Joe Conway, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/7e692b8c-0b11-45db-1cad-3afc5b57409f@amazon.com

2 years agoMark sigint_interrupt_enabled as sig_atomic_t
Michael Paquier [Thu, 29 Sep 2022 05:28:13 +0000 (14:28 +0900)]
Mark sigint_interrupt_enabled as sig_atomic_t

This is a continuation of 78fdb1e, where this flag is set in the psql
callback handler used for SIGINT.  This was previously a boolean but the
C standard recommends the use of sig_atomic_t.  Note that this
influences PromptInterruptContext in string.h, where the same flag is
tracked.

Author: Hayato Kuroda
Discussion: https://postgr.es/m/TYAPR01MB58669A9EC96AA3078C2CD938F5549@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agowindows: Set UMDF_USING_NTSTATUS globally, include ntstatus.h
Andres Freund [Thu, 29 Sep 2022 04:59:15 +0000 (21:59 -0700)]
windows: Set UMDF_USING_NTSTATUS globally, include ntstatus.h

We'd like to use precompiled headers on windows to reduce compile times. Right
now we rely on defining UMDF_USING_NTSTATUS before including postgres.h in a few
select places - which doesn't work with precompiled headers.  Instead define
it globally.

When UMDF_USING_NTSTATUS is defined we need to explicitly include ntstatus.h,
winternl.h to get a comparable set of symbols. Right now these includes would
be required in a number of non-platform-specific .c files - to avoid that,
include them in win32_port.h. Based on my measurements that doesn't increase
compile times measurably.

Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/20220927011951.j3h4o7n6bhf7dwau@awork3.anarazel.de

2 years agomeson: Implement getopt logic from autoconf
Andres Freund [Wed, 28 Sep 2022 21:21:43 +0000 (14:21 -0700)]
meson: Implement getopt logic from autoconf

Not replacing getopt/getopt_long definitely causes issues on mingw. It's not
as clear whether the solaris & openbsd aspect is still needed, but if not, we
should remove it from both autoconf and meson.

Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de

2 years agomeson: mingw: Allow multiple definitions
Andres Freund [Wed, 28 Sep 2022 16:48:09 +0000 (09:48 -0700)]
meson: mingw: Allow multiple definitions

I didn't carry this forward from the win32 template. It's not needed anymore
for the reason stated therein, but it turns out to be required to
e.g. override getopt. Possibly a better solution exists, but that's for later.

Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de

2 years agomeson: pg_regress: Define a HOST_TUPLE sufficient to make resultmap work
Andres Freund [Tue, 27 Sep 2022 19:01:35 +0000 (12:01 -0700)]
meson: pg_regress: Define a HOST_TUPLE sufficient to make resultmap work

This doesn't end up with a triple that's exactly the same as config.guess -
it'd be hard to achieve that and it doesn't seem required. We can't rely on
config.guess as we don't necessarily have a /bin/sh on windows, e.g., when
building on windows with msvc.

This isn't perfect, e.g., clang works on windows as well.  But I suspect we'd
need a bunch of other changes to make clang on windows work, and we haven't
supported it historically.

Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de

2 years agomeson: windows: Normalize slashes in prefix
Andres Freund [Tue, 27 Sep 2022 18:55:00 +0000 (11:55 -0700)]
meson: windows: Normalize slashes in prefix

This fixes a build issue on windows, when the prefix is set to a path with
forward slashes. Windows python defaults to a path with a backslash, but mingw
ucrt python defaults to a forward slash. This in turn lead to a wrong PATH set
during tests, causing tests to fail.

Reported-by: Melih Mutlu <m.melihmutlu@gmail.com>
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de

2 years agoMap ERROR_INVALID_NAME to ENOENT in mapping table of win32error.c
Michael Paquier [Thu, 29 Sep 2022 01:14:47 +0000 (10:14 +0900)]
Map ERROR_INVALID_NAME to ENOENT in mapping table of win32error.c

This error can be reached when sending an incorrect file name to open()
on Windows, resulting in a confusing errno reported.  This has been seen
in the development of a different patch by the same author.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACWet-b8Juba0DiXwfGCyyOcohzwksahE5ebB9rcbLZKCQ@mail.gmail.com

2 years agoRestore pg_pread and friends.
Thomas Munro [Thu, 29 Sep 2022 00:12:11 +0000 (13:12 +1300)]
Restore pg_pread and friends.

Commits cf112c12 and a0dc8271 were a little too hasty in getting rid of
the pg_ prefixes where we use pread(), pwrite() and vectored variants.

We dropped support for ancient Unixes where we needed to use lseek() to
implement replacements for those, but it turns out that Windows also
changes the current position even when you pass in an offset to
ReadFile() and WriteFile() if the file handle is synchronous, despite
its documentation saying otherwise.

Switching to asynchronous file handles would fix that, but have other
complications.  For now let's just put back the pg_ prefix and add some
comments to highlight the non-standard side-effect, which we can now
describe as Windows-only.

Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/20220923202439.GA1156054%40nathanxps13

2 years agoRestrict Datum sort optimization to byval types only
David Rowley [Wed, 28 Sep 2022 22:43:00 +0000 (11:43 +1300)]
Restrict Datum sort optimization to byval types only

91e9e89dc modified nodeSort.c so that it used datum sorts when the
targetlist of the outer node contained only a single column.  That commit
failed to recognise that the Datum returned by tuplesort_getdatum() must
be pfree'd when the type is a byref type.  Ronan Dunklau did originally
propose the patch with that restriction, but that, probably through my own
fault, got lost during further development work.

Due to the timing of this report (PG15 RC1 is almost out the door), let's
just restrict the datum sort optimization to apply for byval types only.
We might want to look harder into making this work for byref types in
PG16.

Reported-by: Önder Kalacı
Diagnosis-by: Tom Lane
Discussion: https://postgr.es/m/CACawEhVxe0ufR26UcqtU7GYGRuubq3p6ZWPGXL4cxy_uexpAAQ@mail.gmail.com
Backpatch-through: 15, where 91e9e89dc was introduced.

2 years agodoc: clarify internal behavior of RECURSIVE CTE queries
Bruce Momjian [Wed, 28 Sep 2022 17:14:38 +0000 (13:14 -0400)]
doc: clarify internal behavior of RECURSIVE CTE queries

Reported-by: Tom Lane
Discussion: https://postgr.es/m/3976627.1662651004@sss.pgh.pa.us

Backpatch-through: 10

2 years agorevert "warn of SECURITY DEFINER schemas for non-sql_body funcs"
Bruce Momjian [Wed, 28 Sep 2022 17:05:21 +0000 (13:05 -0400)]
revert "warn of SECURITY DEFINER schemas for non-sql_body funcs"

doc revert of commit 1703726488.  Change was applied to irrelevant
branches, and was not detailed enough to be helpful in relevant
branches.

Reported-by: Peter Eisentraut, Noah Misch
Discussion: https://postgr.es/m/a2dc9de4-24fc-3222-87d3-0def8057d7d8@enterprisedb.com

Backpatch-through: 10

2 years agoDoc: document bpchar, clarify relationship of text and varchar.
Tom Lane [Wed, 28 Sep 2022 16:31:36 +0000 (12:31 -0400)]
Doc: document bpchar, clarify relationship of text and varchar.

For some reason the "bpchar" type name was defined nowhere in
our SGML docs, although several places refer to it in passing.
Give it a proper mention under Character Types.

While here, also provide an explanation of how the text and varchar
types relate.  The previous wording seemed to be doing its best
to sweep text under the rug, which doesn't seem very appropriate
given its prominence in other parts of the docs.

Minor rearrangements and word-smithing for clarity, too.

Laurenz Albe and Tom Lane, per gripe from Yanliang Lei

Discussion: https://postgr.es/m/120b3084.56b6.1833b5ffe4b.Coremail.msdnchina@163.com

2 years agoAllow callback functions to deregister themselves during a call.
Tom Lane [Wed, 28 Sep 2022 15:23:14 +0000 (11:23 -0400)]
Allow callback functions to deregister themselves during a call.

Fetch the next-item pointer before the call not after, so that
we aren't dereferencing a dangling pointer if the callback
deregistered itself during the call.  The risky coding pattern
appears in CallXactCallbacks, CallSubXactCallbacks, and
ResourceOwnerReleaseInternal.  (There are some other places that
might be at hazard if they offered deregistration functionality,
but they don't.)

I (tgl) considered back-patching this, but desisted because it
wouldn't be very safe for extensions to rely on this working in
pre-v16 branches.

Hao Wu

Discussion: https://postgr.es/m/CAH+9SWXTiERkmhRke+QCcc+jRH8d5fFHTxh8ZK0-Yn4BSpyaAg@mail.gmail.com

2 years agoChange some errdetail() to errdetail_internal()
Alvaro Herrera [Wed, 28 Sep 2022 15:14:53 +0000 (17:14 +0200)]
Change some errdetail() to errdetail_internal()

This prevents marking the argument string for translation for gettext,
and it also prevents the given string (which is already translated) from
being translated at runtime.

Also, mark the strings used as arguments to check_rolespec_name for
translation.

Backpatch all the way back as appropriate.  None of this is caught by
any tests (necessarily so), so I verified it manually.