users/rhaas/postgres.git
5 years agoAvoid -Wconversion warnings when using checksum_impl.h
Michael Paquier [Thu, 5 Mar 2020 05:12:37 +0000 (14:12 +0900)]
Avoid -Wconversion warnings when using checksum_impl.h

This does not matter much when compiling Postgres proper as many
warnings exist when enabling this compilation flag, but it can be
annoying for external modules willing to use both.

Author: David Steele
Discussion: https://postgr.es/m/91d86c8a-11fc-7b88-43eb-5ca3f6fb8bd3@pgmasters.net

5 years agoFix issues around .pgpass file.
Fujii Masao [Thu, 5 Mar 2020 04:00:38 +0000 (13:00 +0900)]
Fix issues around .pgpass file.

This commit fixes the following two issues around .pgpass file.

(1) If the length of a line in .pgpass file was larger than 319B,
        libpq silently treated each 319B in the line as a separate
        setting line.

(2) The document explains that a line beginning with # is treated
        as a comment in .pgpass. But there was no code doing such
        special handling. Whether a line begins with # or not, libpq
        just checked that the first token in the line match with the host.

For (1), this commit makes libpq warn if the length of a line
is larger than 319B, and throw away the remaining part beginning
from 320B position.

For (2), this commit changes libpq so that it treats any lines
beginning with # as comments.

Author: Fujii Masao
Reviewed-by: Hamid Akhtar
Discussion: https://postgr.es/m/c0f0c01c-fa74-9749-2084-b73882fd5465@oss.nttdata.com

5 years agoFix more issues with dependency handling at swap phase of REINDEX CONCURRENTLY
Michael Paquier [Thu, 5 Mar 2020 03:50:15 +0000 (12:50 +0900)]
Fix more issues with dependency handling at swap phase of REINDEX CONCURRENTLY

When canceling a REINDEX CONCURRENTLY operation after swapping is done,
a drop of the parent table would leave behind old indexes.  This is a
consequence of 68ac9cf, which fixed the case of pg_depend bloat when
repeating REINDEX CONCURRENTLY on the same relation.

In order to take care of the problem without breaking the previous fix,
this uses a different strategy, possible even with the exiting set of
routines to handle dependency changes.  The dependencies of/on the
new index are additionally switched to the old one, allowing an old
invalid index remaining around because of a cancellation or a failure to
use the dependency links of the concurrently-created index.  This
ensures that dropping any objects the old invalid index depends on also
drops the old index automatically.

Reported-by: Julien Rouhaud
Author: Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/20200227080735.l32fqcauy73lon7o@nol
Backpatch-through: 12

5 years agoExtend ExecBuildAggTrans() to support a NULL pointer check.
Jeff Davis [Thu, 5 Mar 2020 01:20:20 +0000 (17:20 -0800)]
Extend ExecBuildAggTrans() to support a NULL pointer check.

Optionally push a step to check for a NULL pointer to the pergroup
state.

This will be important for disk-based hash aggregation in combination
with grouping sets. When memory limits are reached, a given tuple may
find its per-group state for some grouping sets but not others. For
the former, it advances the per-group state as normal; for the latter,
it skips evaluation and the calling code will have to spill the tuple
and reprocess it in a later batch.

Add the NULL check as a separate expression step because in some
common cases it's not needed.

Discussion: https://postgr.es/m/20200221202212.ssb2qpmdgrnx52sj%40alap3.anarazel.de

5 years agoIntroduce macros for typalign and typstorage constants.
Tom Lane [Wed, 4 Mar 2020 15:34:25 +0000 (10:34 -0500)]
Introduce macros for typalign and typstorage constants.

Our usual practice for "poor man's enum" catalog columns is to define
macros for the possible values and use those, not literal constants,
in C code.  But for some reason lost in the mists of time, this was
never done for typalign/attalign or typstorage/attstorage.  It's never
too late to make it better though, so let's do that.

The reason I got interested in this right now is the need to duplicate
some uses of the TYPSTORAGE constants in an upcoming ALTER TYPE patch.
But in general, this sort of change aids greppability and readability,
so it's a good idea even without any specific motivation.

I may have missed a few places that could be converted, and it's even
more likely that pending patches will re-introduce some hard-coded
references.  But that's not fatal --- there's no expectation that
we'd actually change any of these values.  We can clean up stragglers
over time.

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

5 years agoMove pg_upgrade's Windows link() implementation to AC_REPLACE_FUNCS
Peter Eisentraut [Wed, 4 Mar 2020 07:05:33 +0000 (08:05 +0100)]
Move pg_upgrade's Windows link() implementation to AC_REPLACE_FUNCS

This way we can make use of it in other components as well, and it
fits better with the rest of the build system.

Discussion: https://www.postgresql.org/message-id/flat/72fff73f-dc9c-4ef4-83e8-d2e60c98df48%402ndquadrant.com

5 years agoAllow to_date/to_timestamp to recognize non-English month/day names.
Tom Lane [Tue, 3 Mar 2020 16:06:47 +0000 (11:06 -0500)]
Allow to_date/to_timestamp to recognize non-English month/day names.

to_char() has long allowed the TM (translation mode) prefix to
specify output of translated month or day names; but that prefix
had no effect in input format strings.  Now it does.  to_date()
and to_timestamp() will now recognize the same month or day names
that to_char() would output for the same format code.  Matching is
case-insensitive (per the active collation's notion of what that
means), just as it has always been for English month/day names
without the TM prefix.

(As per the discussion thread, there are lots of cases that this
feature will not handle, such as alternate day names.  But being
able to accept what to_char() will output seems useful enough.)

In passing, fix some shaky English and violations of message
style guidelines in jsonpath errors for the .datetime() method,
which depends on this code.

Juan José Santamaría Flecha, reviewed and modified by me,
with other commentary from Alvaro Herrera, Tomas Vondra,
Arthur Zakirov, Peter Eisentraut, Mark Dilger.

Discussion: https://postgr.es/m/CAC+AXB3u1jTngJcoC1nAHBf=M3v-jrEfo86UFtCqCjzbWS9QhA@mail.gmail.com

5 years agopg_standby: Don't use HAVE_WORKING_LINK
Peter Eisentraut [Thu, 27 Feb 2020 15:33:05 +0000 (16:33 +0100)]
pg_standby: Don't use HAVE_WORKING_LINK

HAVE_WORKING_LINK is meant to indicate support for hard links, mainly
for Windows.  Here it is used for soft links on Unix, and the
functionality is optional anyway, so we can just make it error out
normally if needed.

Discussion: https://www.postgresql.org/message-id/flat/72fff73f-dc9c-4ef4-83e8-d2e60c98df48%402ndquadrant.com

5 years agoFix typo in monitoring.sgml.
Fujii Masao [Tue, 3 Mar 2020 06:01:47 +0000 (15:01 +0900)]
Fix typo in monitoring.sgml.

Author: Noriyoshi Shinoda

5 years agoRemove overzealous _bt_split() assertions.
Peter Geoghegan [Tue, 3 Mar 2020 05:40:11 +0000 (21:40 -0800)]
Remove overzealous _bt_split() assertions.

_bt_split() is passed NULL as its insertion scankey for internal page
splits.  Two recently added Assert() statements failed to consider this,
leading to a crash with pg_upgrade'd BREE_VERSION < 4 indexes.  Remove
the assertions.

The assertions in question were added by commit 0d861bbb, which added
nbtree deduplication.  It would be possible to fix the assertions
directly instead, but they weren't adding much anyway.

5 years agoFix assertion failure with ALTER TABLE ATTACH PARTITION and indexes
Michael Paquier [Tue, 3 Mar 2020 04:55:41 +0000 (13:55 +0900)]
Fix assertion failure with ALTER TABLE ATTACH PARTITION and indexes

Using ALTER TABLE ATTACH PARTITION causes an assertion failure when
attempting to work on a partitioned index, because partitioned indexes
cannot have partition bounds.

The grammar of ALTER TABLE ATTACH PARTITION requires partition bounds,
but not ALTER INDEX, so mixing ALTER TABLE with partitioned indexes is
confusing.  Hence, on HEAD, prevent ALTER TABLE to attach a partition if
the relation involved is a partitioned index.  On back-branches, as
applications may rely on the existing behavior, just remove the
culprit assertion.

Reported-by: Alexander Lakhin
Author: Amit Langote, Michael Paquier
Discussion: https://postgr.es/m/16276-5cd1dcc8fb8be7b5@postgresql.org
Backpatch-through: 11

5 years agoFix the name of the first WAL segment file, in docs.
Fujii Masao [Tue, 3 Mar 2020 03:22:04 +0000 (12:22 +0900)]
Fix the name of the first WAL segment file, in docs.

Previously the documentation explains that WAL segment files
start at 000000010000000000000000. But the first WAL segment file
that initdb creates is 000000010000000000000001 not
000000010000000000000000. This change was caused by old
commit 8c843fff2d, but the documentation had not been updated
a long time.

Back-patch to all supported branches.

Author: Fujii Masao
Reviewed-by: David Zhang
Discussion: https://postgr.es/m/CAHGQGwHOmGe2OqGOmp8cOfNVDivq7dbV74L5nUGr+3eVd2CU2Q@mail.gmail.com

5 years agoReport progress of streaming base backup.
Fujii Masao [Tue, 3 Mar 2020 03:03:43 +0000 (12:03 +0900)]
Report progress of streaming base backup.

This commit adds pg_stat_progress_basebackup view that reports
the progress while an application like pg_basebackup is taking
a base backup. This uses the progress reporting infrastructure
added by c16dc1aca5e0, adding support for streaming base backup.

Bump catversion.

Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Amit Langote, Sergei Kornilov
Discussion: https://postgr.es/m/9ed8b801-8215-1f3d-62d7-65bff53f6e94@oss.nttdata.com

5 years agoPreserve pg_index.indisclustered across REINDEX CONCURRENTLY
Michael Paquier [Tue, 3 Mar 2020 01:12:28 +0000 (10:12 +0900)]
Preserve pg_index.indisclustered across REINDEX CONCURRENTLY

If the flag value is lost, a CLUSTER query following REINDEX
CONCURRENTLY could fail.  Non-concurrent REINDEX is already handling
this case consistently.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20200229024202.GH29456@telsasoft.com
Backpatch-through: 12

5 years agoFix possibly-uninitialized variable.
Tom Lane [Mon, 2 Mar 2020 23:41:50 +0000 (18:41 -0500)]
Fix possibly-uninitialized variable.

Thinko in 2f9661311.  Per buildfarm, as well as warning seen locally.

5 years agoRepresent command completion tags as structs
Alvaro Herrera [Mon, 2 Mar 2020 21:19:51 +0000 (18:19 -0300)]
Represent command completion tags as structs

The backend was using strings to represent command tags and doing string
comparisons in multiple places, but that's slow and unhelpful.  Create a
new command list with a supporting structure to use instead; this is
stored in a tag-list-file that can be tailored to specific purposes with
a caller-definable C macro, similar to what we do for WAL resource
managers.  The first first such uses are a new CommandTag enum and a
CommandTagBehavior struct.

Replace numerous occurrences of char *completionTag with a
QueryCompletion struct so that the code no longer stores information
about completed queries in a cstring.  Only at the last moment, in
EndCommand(), does this get converted to a string.

EventTriggerCacheItem no longer holds an array of palloc’d tag strings
in sorted order, but rather just a Bitmapset over the CommandTags.

Author: Mark Dilger, with unsolicited help from Álvaro Herrera
Reviewed-by: John Naylor, Tom Lane
Discussion: https://postgr.es/m/981A9DB4-3F0C-4DA5-88AD-CB9CFF4D6CAD@enterprisedb.com

5 years agoBlacklist port/win32_msvc/utime.h in cpluspluscheck and headerscheck.
Tom Lane [Mon, 2 Mar 2020 19:35:22 +0000 (14:35 -0500)]
Blacklist port/win32_msvc/utime.h in cpluspluscheck and headerscheck.

Since commit 481c8e923 it tends to produce "error: sys/utime.h: No such
file or directory" on non-Windows platforms.

5 years agoSilence nbtree.h cpluspluscheck warning.
Peter Geoghegan [Mon, 2 Mar 2020 18:29:30 +0000 (10:29 -0800)]
Silence nbtree.h cpluspluscheck warning.

Add a cast to size_t to silence "comparison between signed and unsigned
integer expressions" cpluspluscheck warning.

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

5 years agoAdd assertions to _bt_update_posting().
Peter Geoghegan [Mon, 2 Mar 2020 16:07:16 +0000 (08:07 -0800)]
Add assertions to _bt_update_posting().

Copy some assertions from _bt_form_posting() to its sibling function,
_bt_update_posting().

Discussion: https://postgr.es/m/CAH2-WzkPR8KMwkL0ap976kmXwBCeukTeHz6fB-U__wvuP1S9Zg@mail.gmail.com

5 years agoUpdate Microsoft documentation link
Peter Eisentraut [Mon, 2 Mar 2020 14:21:42 +0000 (15:21 +0100)]
Update Microsoft documentation link

Reported-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAC%2BAXB1EDXiRPmiVfh%2BWX79x5vXJDU17k0GkDjfyPgOWO4Y5og%40mail.gmail.com

5 years agoRemove long unused code behind a #if 0
Peter Eisentraut [Mon, 2 Mar 2020 07:55:31 +0000 (08:55 +0100)]
Remove long unused code behind a #if 0

Author: Vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CALDaNm3sn4yOq-4rogb-CfE0EYw6b3mVzz8+DnS9BNRwPnhngw@mail.gmail.com

5 years agoFix command-line colorization on Windows with VT100-compatible environments
Michael Paquier [Mon, 2 Mar 2020 06:45:34 +0000 (15:45 +0900)]
Fix command-line colorization on Windows with VT100-compatible environments

When setting PG_COLOR to "always" or "auto" in a Windows terminal
VT100-compatible, the colorization output was not showing up correctly
because it is necessary to update the console's output handling mode.
This fix allows to detect automatically if the environment is compatible
with VT100.  Hence, PG_COLOR=auto is able to detect and handle both
compatible and non-compatible environments.  The behavior of
PG_COLOR=always remains unchanged, as it enforces the use of colorized
output even if the environment does not allow it.

This fix is based on an initial suggestion from Thomas Munro.

Reported-by: Haiying Tang
Author: Juan José Santamaría Flecha
Reviewed-by: Michail Nikolaev, Michael Paquier, Haiying Tang
Discussion: https://postgr.es/m/16108-134692e97146b7bc@postgresql.org
Backpatch-through: 12

5 years agoHandle logical decoding in multi-insert for catalog tuples
Michael Paquier [Mon, 2 Mar 2020 01:00:37 +0000 (10:00 +0900)]
Handle logical decoding in multi-insert for catalog tuples

The code path for multi-insert decoding is not stressed yet for
catalogs (a future patch may introduce this capability), so no
back-patch is needed.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/9690D72F-5C4F-4016-9572-6D16684E1D87@yesql.se

5 years agoRemove dead code from _bt_update_posting().
Peter Geoghegan [Sun, 1 Mar 2020 20:11:26 +0000 (12:11 -0800)]
Remove dead code from _bt_update_posting().

Discussion: https://postgr.es/m/CAH2-WzmAufHiOku6AGiFD=81VQs5nYJ1L2YkhW1t+BH4CMsgRw@mail.gmail.com

5 years agoFix corner-case loss of precision in numeric ln().
Dean Rasheed [Sun, 1 Mar 2020 14:49:25 +0000 (14:49 +0000)]
Fix corner-case loss of precision in numeric ln().

When deciding on the local rscale to use for the Taylor series
expansion, ln_var() neglected to account for the fact that the result
is subsequently multiplied by a factor of 2^(nsqrt+1), where nsqrt is
the number of square root operations performed in the range reduction
step, which can be as high as 22 for very large inputs. This could
result in a loss of precision, particularly when combined with large
rscale values, for which a large number of Taylor series terms is
required (up to around 400).

Fix by computing a few extra digits in the Taylor series, based on the
weight of the multiplicative factor log10(2^(nsqrt+1)). It remains to
be proven whether or not the other 8 extra digits used for the Taylor
series is appropriate, but this at least deals with the obvious
oversight of failing to account for the effects of the final
multiplication.

Per report from Justin AnyhowStep. Reviewed by Tom Lane.

Discussion: https://postgr.es/m/16280-279f299d9c06e56f@postgresql.org

5 years agoDoc: Fix pageinspect bt_page_items() example.
Peter Geoghegan [Sat, 29 Feb 2020 23:10:13 +0000 (15:10 -0800)]
Doc: Fix pageinspect bt_page_items() example.

Oversight in commit 93ee38ea.

5 years agoTeach pageinspect about nbtree deduplication.
Peter Geoghegan [Sat, 29 Feb 2020 20:10:17 +0000 (12:10 -0800)]
Teach pageinspect about nbtree deduplication.

Add a new bt_metap() column to display the metapage's allequalimage
field.  Also add three new columns to contrib/pageinspect's
bt_page_items() function:

* Add a boolean column ("dead") that displays the LP_DEAD bit value for
each non-pivot tuple.

* Add a TID column ("htid") that displays a single heap TID value for
each tuple.  This is the TID that is returned by BTreeTupleGetHeapTID(),
so comparable values are shown for pivot tuples, plain non-pivot tuples,
and posting list tuples.

* Add a TID array column ("tids") that displays TIDs from each tuple's
posting list, if any.  This works just like the "tids" column from
pageinspect's gin_leafpage_items() function.

No version bump for the pageinspect extension, since there hasn't been a
stable Postgres release since the last version bump (the last bump was
part of commit 58b4cb30).

Author: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-WzmSMmU2eNvY9+a4MNP+z02h6sa-uxZvN3un6jY02ZVBSw@mail.gmail.com

5 years agoCorrectly re-use hash tables in buildSubPlanHash().
Tom Lane [Sat, 29 Feb 2020 18:48:09 +0000 (13:48 -0500)]
Correctly re-use hash tables in buildSubPlanHash().

Commit 356687bd8 omitted to remove leftover code for destroying
a hashed subplan's hash tables, with the result that the tables
were always rebuilt not reused; this leads to severe memory
leakage if a hashed subplan is re-executed enough times.
Moreover, the code for reusing the hashnulls table had a typo
that would have made it do the wrong thing if it were reached.

Looking at the code coverage report shows severe under-coverage
of the potential callers of ResetTupleHashTable, so add some test
cases that exercise them.

Andreas Karlsson and Tom Lane, per reports from Ranier Vilela
and Justin Pryzby.

Backpatch to v11, as the faulty commit was.

Discussion: https://postgr.es/m/edb62547-c453-c35b-3ed6-a069e4d6b937@proxel.se
Discussion: https://postgr.es/m/CAEudQAo=DCebm1RXtig9OH+QivpS97sMkikt0A9qHmMUs+g6ZA@mail.gmail.com
Discussion: https://postgr.es/m/20200210032547.GA1412@telsasoft.com

5 years agoRemove obsolete comment.
Tom Lane [Sat, 29 Feb 2020 18:23:12 +0000 (13:23 -0500)]
Remove obsolete comment.

Noted while studying subplan hash issue.

5 years agoAvoid failure if autovacuum tries to access a just-dropped temp namespace.
Tom Lane [Sat, 29 Feb 2020 01:28:34 +0000 (20:28 -0500)]
Avoid failure if autovacuum tries to access a just-dropped temp namespace.

Such an access became possible when commit 246a6c8f7 added more
aggressive cleanup of orphaned temp relations by autovacuum.
Since autovacuum's snapshot might be slightly stale, it could
attempt to access an already-dropped temp namespace, resulting in
an assertion failure or null-pointer dereference.  (In practice,
since we don't drop temp namespaces automatically but merely
recycle them, this situation could only arise if a superuser does
a manual drop of a temp namespace.  Still, that should be allowed.)

The core of the bug, IMO, is that isTempNamespaceInUse and its callers
failed to think hard about whether to treat "temp namespace isn't there"
differently from "temp namespace isn't in use".  In hopes of forestalling
future mistakes of the same ilk, replace that function with a new one
checkTempNamespaceStatus, which makes the same tests but returns a
three-way enum rather than just a bool.  isTempNamespaceInUse is gone
entirely in HEAD; but just in case some external code is relying on it,
keep it in the back branches, as a bug-compatible wrapper around the
new function.

Per report originally from Prabhat Kumar Sahu, investigated by Mahendra
Singh and Michael Paquier; the final form of the patch is my fault.
This replaces the failed fix attempt in a052f6cbb.

Backpatch as far as v11, as 246a6c8f7 was.

Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com

5 years agoFix commit c11cb17d.
Jeff Davis [Fri, 28 Feb 2020 17:32:35 +0000 (09:32 -0800)]
Fix commit c11cb17d.

I neglected to update copyfuncs/outfuncs/readfuncs.

Discussion: https://postgr.es/m/12491.1582833409%40sss.pgh.pa.us

5 years agoDoc: correct thinko in pg_buffercache documentation.
Tom Lane [Fri, 28 Feb 2020 16:29:58 +0000 (11:29 -0500)]
Doc: correct thinko in pg_buffercache documentation.

Access to this module is granted to the pg_monitor role, not
pg_read_all_stats.  (Given the view's performance impact,
it seems wise to be restrictive, so I think this was the
correct decision --- and anyway it was clearly intentional.)

Per bug #16279 from Philip Semanchuk.

Discussion: https://postgr.es/m/16279-fcaac33c68aab0ab@postgresql.org

5 years agoAdd comments on avoid reuse of parse-time snapshot
Alvaro Herrera [Fri, 28 Feb 2020 16:13:54 +0000 (13:13 -0300)]
Add comments on avoid reuse of parse-time snapshot

Apparently, reusing the parse-time query snapshot for later steps
(execution) is a frequently considered optimization ... but it doesn't
work, for reasons discovered in thread [1].  Adding some comments about
why it doesn't really work can relieve some future hackers from wasting
time reimplementing it again.

[1] https://postgr.es/m/flat/5075D8DF.6050500@fuzzy.cz

Author: Michail Nikolaev
Discussion: https://postgr.es/m/CANtu0ogp6cTvMJObXP8n=k+JtqxY1iT9UV5MbGCpjjPa5crCiw@mail.gmail.com

5 years agoAdd PostgreSQL home page to --help output
Peter Eisentraut [Fri, 28 Feb 2020 07:54:49 +0000 (08:54 +0100)]
Add PostgreSQL home page to --help output

Per emerging standard in GNU programs and elsewhere.  Autoconf already
has support for specifying a home page, so we can just that.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/8d389c5f-7fb5-8e48-9a4a-68cec44786fa%402ndquadrant.com

5 years agoRefer to bug report address by symbol rather than hardcoding
Peter Eisentraut [Fri, 28 Feb 2020 07:54:49 +0000 (08:54 +0100)]
Refer to bug report address by symbol rather than hardcoding

Use the PACKAGE_BUGREPORT macro that is created by Autoconf for
referring to the bug reporting address rather than hardcoding it
everywhere.  This makes it easier to change the address and it reduces
translation work.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/8d389c5f-7fb5-8e48-9a4a-68cec44786fa%402ndquadrant.com

5 years agoCatversion bump for b9b408c48724
Alvaro Herrera [Thu, 27 Feb 2020 20:25:47 +0000 (17:25 -0300)]
Catversion bump for b9b408c48724

Per Tom Lane.

5 years agoSave calculated transitionSpace in Agg node.
Jeff Davis [Thu, 27 Feb 2020 18:46:58 +0000 (10:46 -0800)]
Save calculated transitionSpace in Agg node.

This will be useful in the upcoming Hash Aggregation work to improve
estimates for hash table sizing.

Discussion: https://postgr.es/m/37091115219dd522fd9ed67333ee8ed1b7e09443.camel%40j-davis.com

5 years agoDoc: Fix deduplicate_items index term.
Peter Geoghegan [Thu, 27 Feb 2020 17:32:34 +0000 (09:32 -0800)]
Doc: Fix deduplicate_items index term.

Reported-By: Fujii Masao
Discussion: https://postgr.es/m/18f07ae8-7d89-537c-b0a9-54100a1b46da@oss.nttdata.com

5 years agoRecord parents of triggers
Alvaro Herrera [Thu, 27 Feb 2020 16:23:33 +0000 (13:23 -0300)]
Record parents of triggers

This let us get rid of a recently introduced ugly hack (commit
1fa846f1c9af).

Author: Álvaro Herrera
Reviewed-by: Amit Langote, Tom Lane
Discussion: https://postgr.es/m/20200217215641.GA29784@alvherre.pgsql

5 years agoRemove TAP test for createdb --lc-ctype
Michael Paquier [Thu, 27 Feb 2020 12:58:37 +0000 (21:58 +0900)]
Remove TAP test for createdb --lc-ctype

OpenBSD falls back to "C" when using an incorrect input with setlocale()
and LC_CTYPE, causing this test, introduced by 008cf04, to fail.  This
removes the culprit test to avoid the portability issue.

Per report from Robert Haas, via buildfarm member curculio.

Discussion: https://postgr.es/m/CA+TgmoZ6ddh3mHD9gU8DvNYoFmuJaYYn1+4AvZNp25vTdRwCAQ@mail.gmail.com
Backpatch-through: 11

5 years agoSkip foreign tablespaces when running pg_checksums/pg_verify_checksums
Michael Paquier [Thu, 27 Feb 2020 06:31:27 +0000 (15:31 +0900)]
Skip foreign tablespaces when running pg_checksums/pg_verify_checksums

Attempting to use pg_checksums (pg_verify_checksums in 11) on a data
folder which includes tablespace paths used across multiple major
versions would cause pg_checksums to scan all directories present in
pg_tblspc, and not only marked with TABLESPACE_VERSION_DIRECTORY.  This
could lead to failures when for example running sanity checks on an
upgraded instance with --check.  Even worse, it was possible to rewrite
on-disk pages with --enable for a cluster potentially online.

This commit makes pg_checksums skip any directories not named
TABLESPACE_VERSION_DIRECTORY, similarly to what is done for base
backups.

Reported-by: Michael Banck
Author: Michael Banck, Bernd Helmle
Discussion: https://postgr.es/m/62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de
backpatch-through: 11

5 years agoMove src/backend/utils/hash/hashfn.c to src/common
Robert Haas [Thu, 27 Feb 2020 03:55:41 +0000 (09:25 +0530)]
Move src/backend/utils/hash/hashfn.c to src/common

This also involves renaming src/include/utils/hashutils.h, which
becomes src/include/common/hashfn.h. Perhaps an argument can be
made for keeping the hashutils.h name, but it seemed more
consistent to make it match the name of the file, and also more
descriptive of what is actually going on here.

Patch by me, reviewed by Suraj Kharage and Mark Dilger. Off-list
advice on how not to break the Windows build from Davinder Singh
and Amit Kapila.

Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com

5 years agocreatedb: Fix quoting of --encoding, --lc-ctype and --lc-collate
Michael Paquier [Thu, 27 Feb 2020 02:20:46 +0000 (11:20 +0900)]
createdb: Fix quoting of --encoding, --lc-ctype and --lc-collate

The original coding failed to properly quote those arguments, leading to
failures when using quotes in the values used.  As the quoting can be
encoding-sensitive, the connection to the backend needs to be taken
before applying the correct quoting.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200214041004.GB1998@paquier.xyz
Backpatch-through: 9.5

5 years agoSilence another compiler warning in nbtinsert.c.
Peter Geoghegan [Wed, 26 Feb 2020 23:15:45 +0000 (15:15 -0800)]
Silence another compiler warning in nbtinsert.c.

Per complaint from Álvaro Herrera.

5 years agoSuppress unnecessary RelabelType nodes in more cases.
Tom Lane [Wed, 26 Feb 2020 23:13:58 +0000 (18:13 -0500)]
Suppress unnecessary RelabelType nodes in more cases.

eval_const_expressions sometimes produced RelabelType nodes that
were useless because they just relabeled an expression to the same
exposed type it already had.  This is worth avoiding because it can
cause two equivalent expressions to not be equal(), preventing
recognition of useful optimizations.  In the test case added here,
an unpatched planner fails to notice that the "sqli = constant" clause
renders a sort step unnecessary, because one code path produces an
extra RelabelType and another doesn't.

Fix by ensuring that eval_const_expressions_mutator's T_RelabelType
case will not add in an unnecessary RelabelType.  Also save some
code by sharing a subroutine with the effectively-equivalent cases
for CollateExpr and CoerceToDomain.  (CollateExpr had no bug, and
I think that the case couldn't arise with CoerceToDomain, but
it seems prudent to do the same check for all three cases.)

Back-patch to v12.  In principle this has been wrong all along,
but I haven't seen a case where it causes visible misbehavior
before v12, so refrain from changing stable branches unnecessarily.

Per investigation of a report from Eric Gillum.

Discussion: https://postgr.es/m/CAMmjdmvAZsUEskHYj=KT9sTukVVCiCSoe_PBKOXsncFeAUDPCQ@mail.gmail.com

5 years agoFix docs regarding AFTER triggers on partitioned tables
Alvaro Herrera [Wed, 26 Feb 2020 22:57:14 +0000 (19:57 -0300)]
Fix docs regarding AFTER triggers on partitioned tables

In commit 86f575948c77 I forgot to update the trigger.sgml paragraph
that needs to explain that AFTER triggers are allowed in partitioned
tables.  Do so now.

Discussion: https://postgr.es/m/20200224185850.GA30899@alvherre.pgsql

5 years agoSilence compiler warning in nbtinsert.c.
Peter Geoghegan [Wed, 26 Feb 2020 21:17:36 +0000 (13:17 -0800)]
Silence compiler warning in nbtinsert.c.

Per buildfarm member longfin.

5 years agoAdd deduplication to nbtree.
Peter Geoghegan [Wed, 26 Feb 2020 21:05:30 +0000 (13:05 -0800)]
Add deduplication to nbtree.

Deduplication reduces the storage overhead of duplicates in indexes that
use the standard nbtree index access method.  The deduplication process
is applied lazily, after the point where opportunistic deletion of
LP_DEAD-marked index tuples occurs.  Deduplication is only applied at
the point where a leaf page split would otherwise be required.  New
posting list tuples are formed by merging together existing duplicate
tuples.  The physical representation of the items on an nbtree leaf page
is made more space efficient by deduplication, but the logical contents
of the page are not changed.  Even unique indexes make use of
deduplication as a way of controlling bloat from duplicates whose TIDs
point to different versions of the same logical table row.

The lazy approach taken by nbtree has significant advantages over a GIN
style eager approach.  Most individual inserts of index tuples have
exactly the same overhead as before.  The extra overhead of
deduplication is amortized across insertions, just like the overhead of
page splits.  The key space of indexes works in the same way as it has
since commit dd299df8 (the commit that made heap TID a tiebreaker
column).

Testing has shown that nbtree deduplication can generally make indexes
with about 10 or 15 tuples for each distinct key value about 2.5X - 4X
smaller, even with single column integer indexes (e.g., an index on a
referencing column that accompanies a foreign key).  The final size of
single column nbtree indexes comes close to the final size of a similar
contrib/btree_gin index, at least in cases where GIN's posting list
compression isn't very effective.  This can significantly improve
transaction throughput, and significantly reduce the cost of vacuuming
indexes.

A new index storage parameter (deduplicate_items) controls the use of
deduplication.  The default setting is 'on', so all new B-Tree indexes
automatically use deduplication where possible.  This decision will be
reviewed at the end of the Postgres 13 beta period.

There is a regression of approximately 2% of transaction throughput with
synthetic workloads that consist of append-only inserts into a table
with several non-unique indexes, where all indexes have few or no
repeated values.  The underlying issue is that cycles are wasted on
unsuccessful attempts at deduplicating items in non-unique indexes.
There doesn't seem to be a way around it short of disabling
deduplication entirely.  Note that deduplication of items in unique
indexes is fairly well targeted in general, which avoids the problem
there (we can use a special heuristic to trigger deduplication passes in
unique indexes, since we're specifically targeting "version bloat").

Bump XLOG_PAGE_MAGIC because xl_btree_vacuum changed.

No bump in BTREE_VERSION, since the representation of posting list
tuples works in a way that's backwards compatible with version 4 indexes
(i.e. indexes built on PostgreSQL 12).  However, users must still
REINDEX a pg_upgrade'd index to use deduplication, regardless of the
Postgres version they've upgraded from.  This is the only way to set the
new nbtree metapage flag indicating that deduplication is generally
safe.

Author: Anastasia Lubennikova, Peter Geoghegan
Reviewed-By: Peter Geoghegan, Heikki Linnakangas
Discussion:
    https://postgr.es/m/55E4051B.7020209@postgrespro.ru
    https://postgr.es/m/4ab6e2db-bcee-f4cf-0916-3a06e6ccbb55@postgrespro.ru

5 years agoAdd equalimage B-Tree support functions.
Peter Geoghegan [Wed, 26 Feb 2020 19:28:25 +0000 (11:28 -0800)]
Add equalimage B-Tree support functions.

Invent the concept of a B-Tree equalimage ("equality implies image
equality") support function, registered as support function 4.  This
indicates whether it is safe (or not safe) to apply optimizations that
assume that any two datums considered equal by an operator class's order
method must be interchangeable without any loss of semantic information.
This is static information about an operator class and a collation.

Register an equalimage routine for almost all of the existing B-Tree
opclasses.  We only need two trivial routines for all of the opclasses
that are included with the core distribution.  There is one routine for
opclasses that index non-collatable types (which returns 'true'
unconditionally), plus another routine for collatable types (which
returns 'true' when the collation is a deterministic collation).

This patch is infrastructure for an upcoming patch that adds B-Tree
deduplication.

Author: Peter Geoghegan, Anastasia Lubennikova
Discussion: https://postgr.es/m/CAH2-Wzn3Ee49Gmxb7V1VJ3-AC8fWn-Fr8pfWQebHe8rYRxt5OQ@mail.gmail.com

5 years agoInclude error code in message from pg_upgrade
Magnus Hagander [Wed, 26 Feb 2020 09:03:11 +0000 (10:03 +0100)]
Include error code in message from pg_upgrade

In passing, also quote the filename in one message where it wasn't.

Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87pne2w98h.fsf@wibble.ilmari.org

5 years agoFix build failure on header generation with repetitive builds of MSVC
Michael Paquier [Tue, 25 Feb 2020 04:57:40 +0000 (13:57 +0900)]
Fix build failure on header generation with repetitive builds of MSVC

GenerateConfigHeader() in Solution.pm was complaining about unused
define symbols even if a newer config header was not generated, causing
successive build attempts with MSVC to fail.

Oversight in commit 8f4fb4c.

Author: Kyotaro Horiguchi
Reviewed-by: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/20200218.160500.44393633318853097.horikyota.ntt@gmail.com

5 years agoFix compile failure.
Tom Lane [Mon, 24 Feb 2020 23:43:23 +0000 (18:43 -0500)]
Fix compile failure.

I forgot that some compilers won't handle #if constructs within
ereport() calls.  Duplicating most of the call is annoying but simple.
Per buildfarm.

5 years agoexpression eval: Reduce number of steps for agg transition invocations.
Andres Freund [Mon, 24 Feb 2020 22:39:22 +0000 (14:39 -0800)]
expression eval: Reduce number of steps for agg transition invocations.

Do so by combining the various steps that are part of aggregate
transition function invocation into one larger step. As some of the
current steps are only necessary for some aggregates, have one variant
of the aggregate transition step for each possible combination.

To avoid further manual copies of code in the different transition
step implementations, move most of the code into helper functions
marked as "always inline".

The benefit of this change is an increase in performance when
aggregating lots of rows. This comes in part due to the reduced number
of indirect jumps due to the reduced number of steps, and in part by
reducing redundant setup code across steps. This mainly benefits
interpreted execution, but the code generated by JIT is also improved
a bit.

As a nice side-effect it also ends up making the code a bit simpler.

A small additional optimization is removing the need to set
aggstate->curaggcontext before calling ExecAggInitGroup, choosing to
instead passign curaggcontext as an argument. It was, in contrast to
other aggregate related functions, only needed to fetch a memory
context to copy the transition value into.

Author: Andres Freund
Discussion:
   https://postgr.es/m/20191023163849.sosqbfs5yenocez3@alap3.anarazel.de
   https://postgr.es/m/5c371df7cee903e8cd4c685f90c6c72086d3a2dc.camel@j-davis.com

5 years agoIssue properly WAL record for CID of first catalog tuple in multi-insert
Michael Paquier [Mon, 24 Feb 2020 22:55:22 +0000 (07:55 +0900)]
Issue properly WAL record for CID of first catalog tuple in multi-insert

Multi-insert for heap is not yet used actively for catalogs, but the
code to support this case is in place for logical decoding.  The
existing code forgot to issue a XLOG_HEAP2_NEW_CID record for the first
tuple inserted, leading to failures when attempting to use multiple
inserts for catalogs at decoding time.  This commit fixes the problem by
WAL-logging the needed CID.

This is not an active bug, so no back-patch is done.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/E0D4CC67-A1CF-4DF4-991D-B3AC2EB5FAE9@yesql.se

5 years agoAccount explicitly for long-lived FDs that are allocated outside fd.c.
Tom Lane [Mon, 24 Feb 2020 22:28:33 +0000 (17:28 -0500)]
Account explicitly for long-lived FDs that are allocated outside fd.c.

The comments in fd.c have long claimed that all file allocations should
go through that module, but in reality that's not always practical.
fd.c doesn't supply APIs for invoking some FD-producing syscalls like
pipe() or epoll_create(); and the APIs it does supply for non-virtual
FDs are mostly insistent on releasing those FDs at transaction end;
and in some cases the actual open() call is in code that can't be made
to use fd.c, such as libpq.

This has led to a situation where, in a modern server, there are likely
to be seven or so long-lived FDs per backend process that are not known
to fd.c.  Since NUM_RESERVED_FDS is only 10, that meant we had *very*
few spare FDs if max_files_per_process is >= the system ulimit and
fd.c had opened all the files it thought it safely could.  The
contrib/postgres_fdw regression test, in particular, could easily be
made to fall over by running it under a restrictive ulimit.

To improve matters, invent functions Acquire/Reserve/ReleaseExternalFD
that allow outside callers to tell fd.c that they have or want to allocate
a FD that's not directly managed by fd.c.  Add calls to track all the
fixed FDs in a standard backend session, so that we are honestly
guaranteeing that NUM_RESERVED_FDS FDs remain unused below the EMFILE
limit in a backend's idle state.  The coding rules for these functions say
that there's no need to call them in code that just allocates one FD over
a fairly short interval; we can dip into NUM_RESERVED_FDS for such cases.
That means that there aren't all that many places where we need to worry.
But postgres_fdw and dblink must use this facility to account for
long-lived FDs consumed by libpq connections.  There may be other places
where it's worth doing such accounting, too, but this seems like enough
to solve the immediate problem.

Internally to fd.c, "external" FDs are limited to max_safe_fds/3 FDs.
(Callers can choose to ignore this limit, but of course it's unwise
to do so except for fixed file allocations.)  I also reduced the limit
on "allocated" files to max_safe_fds/3 FDs (it had been max_safe_fds/2).
Conceivably a smarter rule could be used here --- but in practice,
on reasonable systems, max_safe_fds should be large enough that this
isn't much of an issue, so KISS for now.  To avoid possible regression
in the number of external or allocated files that can be opened,
increase FD_MINFREE and the lower limit on max_files_per_process a
little bit; we now insist that the effective "ulimit -n" be at least 64.

This seems like pretty clearly a bug fix, but in view of the lack of
field complaints, I'll refrain from risking a back-patch.

Discussion: https://postgr.es/m/E1izCmM-0005pV-Co@gemulon.postgresql.org

5 years agoChange client-side fsync_fname() to report errors fatally
Peter Eisentraut [Mon, 24 Feb 2020 15:32:34 +0000 (16:32 +0100)]
Change client-side fsync_fname() to report errors fatally

Given all we have learned about fsync() error handling in the last few
years, reporting an fsync() error non-fatally is not useful,
unless you don't care much about the file, in which case you probably
don't need to use fsync() in the first place.

Change fsync_fname() and durable_rename() to exit(1) on fsync() errors
other than those that we specifically chose to ignore.

This affects initdb, pg_basebackup, pg_checksums, pg_dump, pg_dumpall,
and pg_rewind.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/d239d1bd-aef0-ca7c-dc0a-da14bdcf0392%402ndquadrant.com

5 years agoAdapt hashfn.c and hashutils.h for frontend use.
Robert Haas [Mon, 24 Feb 2020 11:57:15 +0000 (17:27 +0530)]
Adapt hashfn.c and hashutils.h for frontend use.

hash_any() and its various variants are defined to return Datum,
which is a backend-only concept, but the underlying functions
actually want to return uint32 and uint64, and only return Datum
because it's convenient for callers who are using them to
implement a hash function for some SQL datatype.

However, changing these functions to return uint32 and uint64
seems like it might lead to programming errors or back-patching
difficulties, both because they are widely used and because
failure to use UInt{32,64}GetDatum() might not provoke a
compilation error. Instead, rename the existing functions as
well as changing the return type, and add static inline wrappers
for those callers that need the previous behavior.

Although this commit adapts hashutils.h and hashfn.c so that they
can be compiled as frontend code, it does not actually do
anything that would cause them to be so compiled. That is left
for another commit.

Patch by me, reviewed by Suraj Kharage and Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com

5 years agoPut all the prototypes for hashfn.c into the same header file.
Robert Haas [Mon, 24 Feb 2020 11:52:45 +0000 (17:22 +0530)]
Put all the prototypes for hashfn.c into the same header file.

Previously, some of the prototypes for functions in hashfn.c were
in utils/hashutils.h and others were in utils/hsearch.h, but that
is confusing and has no particular benefit.

Patch by me, reviewed by Suraj Kharage and Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com

5 years agoMove bitmap_hash and bitmap_match to bitmapset.c.
Robert Haas [Mon, 24 Feb 2020 11:47:08 +0000 (17:17 +0530)]
Move bitmap_hash and bitmap_match to bitmapset.c.

The closely-related function bms_hash_value is already defined in that
file, and this change means that hashfn.c no longer needs to depend on
nodes/bitmapset.h. That gets us closer to allowing use of the hash
functions in hashfn.c in frontend code.

Patch by me, reviewed by Suraj Kharage and Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com

5 years agoAdd prefix checks in exclude lists for pg_rewind, pg_checksums and base backups
Michael Paquier [Mon, 24 Feb 2020 09:13:25 +0000 (18:13 +0900)]
Add prefix checks in exclude lists for pg_rewind, pg_checksums and base backups

An instance of PostgreSQL crashing with a bad timing could leave behind
temporary pg_internal.init files, potentially causing failures when
verifying checksums.  As the same exclusion lists are used between
pg_rewind, pg_checksums and basebackup.c, all those tools are extended
with prefix checks to keep everything in sync, with dedicated checks
added for pg_internal.init.

Backpatch down to 11, where pg_checksums (pg_verify_checksums in 11) and
checksum verification for base backups have been introduced.

Reported-by: Michael Banck
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, David Steele
Discussion: https://postgr.es/m/62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de
Backpatch-through: 11

5 years agoFactor out InitControlFile() from BootStrapXLOG()
Peter Eisentraut [Mon, 17 Feb 2020 16:58:02 +0000 (17:58 +0100)]
Factor out InitControlFile() from BootStrapXLOG()

Right now this only makes BootStrapXLOG() a bit more manageable, but
in the future there may be external callers.

Discussion: https://www.postgresql.org/message-id/e8f86ba5-48f1-a80a-7f1d-b76bcb9c5c47@2ndquadrant.com

5 years agoReformat code comment
Peter Eisentraut [Mon, 17 Feb 2020 16:46:37 +0000 (17:46 +0100)]
Reformat code comment

Discussion: https://www.postgresql.org/message-id/e8f86ba5-48f1-a80a-7f1d-b76bcb9c5c47@2ndquadrant.com

5 years agopg_resetwal: Rename function to avoid potential conflict
Peter Eisentraut [Mon, 17 Feb 2020 16:35:48 +0000 (17:35 +0100)]
pg_resetwal: Rename function to avoid potential conflict

ReadControlFile() here conflicts with a function of the same name in
xlog.c.  There is no actual conflict right now, but since
pg_resetwal.c reaches deep inside backend headers, it's possible in
the future.

Discussion: https://www.postgresql.org/message-id/e8f86ba5-48f1-a80a-7f1d-b76bcb9c5c47@2ndquadrant.com

5 years agoAdjust Solution.pm to set HAVE_STDINT_H.
Tom Lane [Fri, 21 Feb 2020 21:14:09 +0000 (16:14 -0500)]
Adjust Solution.pm to set HAVE_STDINT_H.

We're not testing that anywhere anymore, but for consistency,
it should get defined.

5 years agoFix perlcritic warnings
Peter Eisentraut [Fri, 21 Feb 2020 21:03:05 +0000 (22:03 +0100)]
Fix perlcritic warnings

5 years agoAllow running src/tools/msvc/mkvcbuild.pl under not Windows
Peter Eisentraut [Fri, 21 Feb 2020 19:50:56 +0000 (20:50 +0100)]
Allow running src/tools/msvc/mkvcbuild.pl under not Windows

This to allow verifying the MSVC build file generation without having
to have Windows.

To do this, we avoid Windows-specific Perl modules and don't run the
"cl" compiler or "nmake".  The resulting build files won't actually be
completely correct, but it's useful enough.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/d73b2c7b-f081-8357-8422-7564d55f1aac%402ndquadrant.com

5 years agoAssume that we have signed integral types and flexible array members.
Tom Lane [Fri, 21 Feb 2020 19:30:21 +0000 (14:30 -0500)]
Assume that we have signed integral types and flexible array members.

These compiler features are required by C99, so remove the configure
probes for them.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have <wchar.h>.
Tom Lane [Fri, 21 Feb 2020 19:11:39 +0000 (14:11 -0500)]
Assume that we have <wchar.h>.

Windows has this, and so do all other live platforms according to the
buildfarm; it's been required by POSIX since SUSv2.  So remove the
configure probe and tests of HAVE_WCHAR_H.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have utime() and <utime.h>.
Tom Lane [Fri, 21 Feb 2020 19:04:19 +0000 (14:04 -0500)]
Assume that we have utime() and <utime.h>.

These are required by POSIX since SUSv2, and no live platforms fail
to provide them.  On Windows, utime() exists and we bring our own
<utime.h>, so we're good there too.  So remove the configure probes
and ad-hoc substitute code.  We don't need to check for utimes()
anymore either, since that was only used as a substitute.

In passing, make the Windows build include <sys/utime.h> only where
we need it, not everywhere.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have rint().
Tom Lane [Fri, 21 Feb 2020 18:47:40 +0000 (13:47 -0500)]
Assume that we have rint().

Windows has this since _MSC_VER >= 1200, and so do all other live
platforms according to the buildfarm, so remove the configure probe
and src/port/ substitution.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have memmove().
Tom Lane [Fri, 21 Feb 2020 18:42:28 +0000 (13:42 -0500)]
Assume that we have memmove().

Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and c.h's substitute code.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have cbrt().
Tom Lane [Fri, 21 Feb 2020 18:24:21 +0000 (13:24 -0500)]
Assume that we have cbrt().

Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and float.c's substitute code.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have isinf().
Tom Lane [Fri, 21 Feb 2020 18:18:27 +0000 (13:18 -0500)]
Assume that we have isinf().

Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and src/port/ substitution.

This also lets us get rid of some configure probes that existed only
to support src/port/isinf.c.  I kept the port.h hack to force using
__builtin_isinf() on clang, though.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoAssume that we have functional, 64-bit fseeko()/ftello().
Tom Lane [Fri, 21 Feb 2020 17:49:42 +0000 (12:49 -0500)]
Assume that we have functional, 64-bit fseeko()/ftello().

Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and src/port/ substitution.

Keep the probe that detects whether _LARGEFILE_SOURCE has to be
defined to get that, though ... that seems to be still relevant in
some places.

This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code.  I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.

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

5 years agoFix compiler warnings on 64-bit Windows
Peter Eisentraut [Fri, 21 Feb 2020 18:49:44 +0000 (19:49 +0100)]
Fix compiler warnings on 64-bit Windows

GCC reports various instances of

warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

and MSVC equivalently

warning C4312: 'type cast': conversion from 'int' to 'void *' of greater size
warning C4311: 'type cast': pointer truncation from 'void *' to 'long'

in ECPG test files.  This is because void* and long are cast back and
forth, but on 64-bit Windows, these have different sizes.  Fix by
using intptr_t instead.

The code actually worked fine because the integer values in use are
all small.  So this is just to get the test code to compile warning-free.

This change is simplified by having made stdint.h required (commit
957338418b69e9774ccc1bab59f765a62f0aa6f9).  Before this it would have
been more complicated because the ecpg test source files don't use the
full pg_config.h.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/5d398bbb-262a-5fed-d839-d0e5cff3c0d7%402ndquadrant.com

5 years agoFixup for nodeAgg.c refactor.
Jeff Davis [Fri, 21 Feb 2020 01:04:19 +0000 (17:04 -0800)]
Fixup for nodeAgg.c refactor.

Commit 5b618e1f made an unintended behavior change.

5 years agoAvoid redundant checks in partition_bounds_copy().
Etsuro Fujita [Fri, 21 Feb 2020 11:00:45 +0000 (20:00 +0900)]
Avoid redundant checks in partition_bounds_copy().

Previously, partition_bounds_copy() checked whether the strategy for the
given partition bounds was hash or not, and then determined the number of
elements in the datums in the datums array for the partition bounds, on
each iteration of the loop for copying the datums array, but there is no
need to do that.  Perform the checks only once before the loop iteration.

Author: Etsuro Fujita
Reported-by: Amit Langote and Julien Rouhaud
Discussion: https://postgr.es/m/CAPmGK14Rvxrm8DHWvCjdoks6nwZuHBPvMnWZ6rkEx2KhFeEoPQ@mail.gmail.com

5 years agoRequire stdint.h
Peter Eisentraut [Fri, 21 Feb 2020 08:14:03 +0000 (09:14 +0100)]
Require stdint.h

stdint.h belongs to the compiler (as opposed to inttypes.h), so by
requiring a C99 compiler we can also require stdint.h
unconditionally.  Remove configure checks and other workarounds for
it.

This also removes a few steps in the required portability adjustments
to the imported time zone code, which can be applied on the next
import.

When using GCC on a platform that is otherwise pre-C99, this will now
require at least GCC 4.5, which is the first release that supplied a
standard-conforming stdint.h if the native platform didn't have it.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/5d398bbb-262a-5fed-d839-d0e5cff3c0d7%402ndquadrant.com

5 years agoDoc: Fix instructions to control build environment with MSVC
Michael Paquier [Fri, 21 Feb 2020 03:05:29 +0000 (12:05 +0900)]
Doc: Fix instructions to control build environment with MSVC

The documentation included some outdated instructions to change the
architecture, build type or target OS of a build done with MSVC.  This
commit updates the documentation to include the modern options
available, down to Visual Studio 2013.

Reported-by: Kyotaro Horiguchi
Author: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/CAC+AXB0J7tAqW_2F1fCE4Dh2=Ccz96TcLpsGXOCvka7VvWG9Qw@mail.gmail.com
Backpatch-through: 12

5 years agoSimplify FK-to-partitioned regression test query
Alvaro Herrera [Thu, 20 Feb 2020 17:14:20 +0000 (14:14 -0300)]
Simplify FK-to-partitioned regression test query

Avoid a join between relations having the FK to detect FK violation.
The planner might optimize this considering the PK must exist on the
referenced side at some point, effectively masking a bug this test
tries to detect.

Tom Lane and Jehan-Guillaume de Rorthais
Discussion: https://postgr.es/m/467.1581270529@sss.pgh.pa.us

5 years agoRemove extra word from comment.
Etsuro Fujita [Thu, 20 Feb 2020 10:15:00 +0000 (19:15 +0900)]
Remove extra word from comment.

5 years agoCleanup more code related to ws2_32.dll loading in src/port/getaddrinfo.c
Michael Paquier [Thu, 20 Feb 2020 02:57:41 +0000 (11:57 +0900)]
Cleanup more code related to ws2_32.dll loading in src/port/getaddrinfo.c

e2e0219 has removed a code path for Windows 2000 that attempts to load
wship6.dll as fallback if ws2_32.dll is found but not getaddrinfo(),
leaving behind a dangling pointer as the library is freed.  However,
there is no point in this check as ws2_32.dll exists since Windows XP,
so just remove the duplicated check.

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

5 years agoDoc: discourage use of partial indexes for poor-man's-partitioning.
Tom Lane [Wed, 19 Feb 2020 23:52:18 +0000 (18:52 -0500)]
Doc: discourage use of partial indexes for poor-man's-partitioning.

Creating a bunch of non-overlapping partial indexes is generally
a bad idea, so add an example saying not to do that.

Back-patch to v10.  Before that, the alternative of using (real)
partitioning wasn't available, so that the tradeoff isn't quite
so clear cut.

Discussion: https://postgr.es/m/CAKVFrvFY-f7kgwMRMiPLbPYMmgjc8Y2jjUGK_Y0HVcYAmU6ymg@mail.gmail.com

5 years agoRemove support for upgrading extensions from "unpackaged" state.
Tom Lane [Wed, 19 Feb 2020 21:59:14 +0000 (16:59 -0500)]
Remove support for upgrading extensions from "unpackaged" state.

Andres Freund pointed out that allowing non-superusers to run
"CREATE EXTENSION ... FROM unpackaged" has security risks, since
the unpackaged-to-1.0 scripts don't try to verify that the existing
objects they're modifying are what they expect.  Just attaching such
objects to an extension doesn't seem too dangerous, but some of them
do more than that.

We could have resolved this, perhaps, by still requiring superuser
privilege to use the FROM option.  However, it's fair to ask just what
we're accomplishing by continuing to lug the unpackaged-to-1.0 scripts
forward.  None of them have received any real testing since 9.1 days,
so they may not even work anymore (even assuming that one could still
load the previous "loose" object definitions into a v13 database).
And an installation that's trying to go from pre-9.1 to v13 or later
in one jump is going to have worse compatibility problems than whether
there's a trivial way to convert their contrib modules into extension
style.

Hence, let's just drop both those scripts and the core-code support
for "CREATE EXTENSION ... FROM".

Discussion: https://postgr.es/m/20200213233015.r6rnubcvl4egdh5r@alap3.anarazel.de

5 years agoFix typo
Peter Eisentraut [Wed, 19 Feb 2020 19:52:42 +0000 (20:52 +0100)]
Fix typo

Reported-by: Daniel Verite <daniel@manitou-mail.org>
5 years agoFix confusion about event trigger vs. plain function in plpgsql.
Tom Lane [Wed, 19 Feb 2020 19:44:58 +0000 (14:44 -0500)]
Fix confusion about event trigger vs. plain function in plpgsql.

The function hash table keys made by compute_function_hashkey() failed
to distinguish event-trigger call context from regular call context.
This meant that once we'd successfully made a hash entry for an event
trigger (either by validation, or by normal use as an event trigger),
an attempt to call the trigger function as a plain function would
find this hash entry and thereby bypass the you-can't-do-that check in
do_compile().  Thus we'd attempt to execute the function, leading to
strange errors or even crashes, depending on function contents and
server version.

To fix, add an isEventTrigger field to PLpgSQL_func_hashkey,
paralleling the longstanding infrastructure for regular triggers.
This fits into what had been pad space, so there's no risk of an ABI
break, even assuming that any third-party code is looking at these
hash keys.  (I considered replacing isTrigger with a PLpgSQL_trigtype
enum field, but felt that that carried some API/ABI risk.  Maybe we
should change it in HEAD though.)

Per bug #16266 from Alexander Lakhin.  This has been broken since
event triggers were invented, so back-patch to all supported branches.

Discussion: https://postgr.es/m/16266-fcd7f838e97ba5d4@postgresql.org

5 years agoSet gen_random_uuid() to volatile
Peter Eisentraut [Wed, 19 Feb 2020 19:09:32 +0000 (20:09 +0100)]
Set gen_random_uuid() to volatile

It was set to immutable.  This was a mistake in the initial
commit (5925e5549890416bcf588334d9d0bc99f8ad6c7f).

Reported-by: hubert depesz lubaczewski <depesz@depesz.com>
Discussion: https://www.postgresql.org/message-id/flat/20200218185452.GA8710%40depesz.com

5 years agoMinor refactor of nodeAgg.c.
Jeff Davis [Wed, 19 Feb 2020 18:15:16 +0000 (10:15 -0800)]
Minor refactor of nodeAgg.c.

  * Separate calculation of hash value from the lookup.
  * Split build_hash_table() into two functions.
  * Change lookup_hash_entry() to return AggStatePerGroup. That's all
    the caller needed, anyway.

These changes are to support the upcoming Disk-based Hash Aggregation
work.

Discussion: https://postgr.es/m/31f5ab871a3ad5a1a91a7a797651f20e77ac7ce3.camel%40j-davis.com

5 years agologtape.c: allocate read buffer even for an empty tape.
Jeff Davis [Tue, 18 Feb 2020 20:31:24 +0000 (12:31 -0800)]
logtape.c: allocate read buffer even for an empty tape.

Prior to this commit, the read buffer was allocated at the time the tape
was rewound; but as an optimization, would not be allocated at all if
the tape was empty.

That optimization meant that it was valid to have a rewound tape with
the buffer set to NULL, but only if a number of conditions were met
and only if the API was used properly. After 7fdd919a refactored the
code to support lazily-allocating the buffer, Coverity started
complaining.

The optimization for empty tapes doesn't seem important, so just
allocate the buffer whether the tape has any data or not.

Discussion: https://postgr.es/m/20351.1581868306%40sss.pgh.pa.us

5 years agoFix mesurement of elapsed time during truncating heap in VACUUM.
Fujii Masao [Wed, 19 Feb 2020 11:37:26 +0000 (20:37 +0900)]
Fix mesurement of elapsed time during truncating heap in VACUUM.

VACUUM may truncate heap in several batches. The activity report
is logged for each batch, and contains the number of pages in the table
before and after the truncation, and also the elapsed time during
the truncation. Previously the elapsed time reported in each batch was
the total elapsed time since starting the truncation until finishing
each batch. For example, if the truncation was processed dividing into
three batches, the second batch reported the accumulated time elapsed
during both first and second batches. This is strange and confusing
because the number of pages in the table reported together is not
total. Instead, each batch should report the time elapsed during
only that batch.

The cause of this issue was that the resource usage snapshot was
initialized only at the beginning of the truncation and was never
reset later. This commit fixes the issue by changing VACUUM so that
the resource usage snapshot is reset at each batch.

Back-patch to all supported branches.

Reported-by: Tatsuhito Kasahara
Author: Tatsuhito Kasahara
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/CAP0=ZVJsf=NvQuy+QXQZ7B=ZVLoDV_JzsVC1FRsF1G18i3zMGg@mail.gmail.com

5 years agoClean up some code, comments and docs referring to Windows 2000 and older
Michael Paquier [Wed, 19 Feb 2020 04:20:33 +0000 (13:20 +0900)]
Clean up some code, comments and docs referring to Windows 2000 and older

This fixes and updates a couple of comments related to outdated Windows
versions.  Particularly, src/common/exec.c had a fallback implementation
to read a file's line from a pipe because stdin/stdout/stderr does not
exist in Windows 2000 that is removed to simplify src/common/ as there
are unlikely versions of Postgres running on such platforms.

Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Juan José Santamaría Flecha
Discussion: https://postgr.es/m/20191219021526.GC4202@paquier.xyz

5 years agoStop demanding that top xact must be seen before subxact in decoding.
Amit Kapila [Wed, 12 Feb 2020 05:53:24 +0000 (11:23 +0530)]
Stop demanding that top xact must be seen before subxact in decoding.

Manifested as

ERROR:  subtransaction logged without previous top-level txn record

this check forbids legit behaviours like
 - First xl_xact_assignment record is beyond reading, i.e. earlier
   restart_lsn.
 - After restart_lsn there is some change of a subxact.
 - After that, there is second xl_xact_assignment (for another subxact)
   revealing the relationship between top and first subxact.

Such a transaction won't be streamed anyway because we hadn't seen it in
full.  Saying for sure whether xact of some record encountered after
the snapshot was deserialized can be streamed or not requires to know
whether it wrote something before deserialization point --if yes, it
hasn't been seen in full and can't be decoded. Snapshot doesn't have such
info, so there is no easy way to relax the check.

Reported-by: Hsu, John
Diagnosed-by: Arseny Sher
Author: Arseny Sher, Amit Kapila
Reviewed-by: Amit Kapila, Dilip Kumar
Backpatch-through: 9.5
Discussion: https://postgr.es/m/AB5978B2-1772-4FEE-A245-74C91704ECB0@amazon.com

5 years agoRemove obsolete _bt_compare() comment.
Peter Geoghegan [Wed, 19 Feb 2020 00:07:16 +0000 (16:07 -0800)]
Remove obsolete _bt_compare() comment.

btbuild() has nothing to say about how NULL values compare in nbtree.
Besides, there are _bt_compare() header comments that describe how NULL
values are handled.

5 years agoMake inherited LOCK TABLE perform access permission checks on parent table only.
Fujii Masao [Tue, 18 Feb 2020 04:13:15 +0000 (13:13 +0900)]
Make inherited LOCK TABLE perform access permission checks on parent table only.

Previously, LOCK TABLE command through a parent table checked
the permissions on not only the parent table but also the children
tables inherited from it. This was a bug and inherited queries should
perform access permission checks on the parent table only. This
commit fixes LOCK TABLE so that it does not check the permission
on children tables.

This patch is applied only in the master branch. We decided not to
back-patch because it's not hard to imagine that there are some
applications expecting the old behavior and the change breaks their
security.

Author: Amit Langote
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAHGQGwE+GauyG7POtRfRwwthAGwTjPQYdFHR97+LzA4RHGnJxA@mail.gmail.com

5 years agoRemove duplicated words in comments
Michael Paquier [Tue, 18 Feb 2020 03:20:55 +0000 (12:20 +0900)]
Remove duplicated words in comments

Author: Daniel Gustafsson
Reviewed-by: Vik Fearing
Discussion: https://postgr.es/m/EBC3BFEB-664C-4063-81ED-29F1227DB012@yesql.se

5 years agoFix grammar in monitoring.sgml
Michael Paquier [Tue, 18 Feb 2020 01:49:44 +0000 (10:49 +0900)]
Fix grammar in monitoring.sgml

This is related to progress reporting for ANALYZE and partitioned
tables.

Author: Amit Langote
Reviewed-by: Daniel Gustafsson, Julien Rouhaud
Discussion: https://postgr.es/m/CA+HiwqGx6C=-bFTX=ryMThyvM7CcSC3b1x8_6zh4Uo41Kvu-zw@mail.gmail.com

5 years agoTeach pg_dump to dump comments on RLS policy objects.
Tom Lane [Mon, 17 Feb 2020 23:40:02 +0000 (18:40 -0500)]
Teach pg_dump to dump comments on RLS policy objects.

This was unaccountably omitted in the original RLS patch.
The SQL syntax is basically the same as for comments on triggers,
so crib code from dumpTrigger().

Per report from Marc Munro.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/1581889298.18009.15.camel@bloodnok.com

5 years agoOptimize update of tables with generated columns
Peter Eisentraut [Mon, 17 Feb 2020 14:19:58 +0000 (15:19 +0100)]
Optimize update of tables with generated columns

When updating a table row with generated columns, only recompute those
generated columns whose base columns have changed in this update and
keep the rest unchanged.  This can result in a significant performance
benefit.  The required information was already kept in
RangeTblEntry.extraUpdatedCols; we just have to make use of it.

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/b05e781a-fa16-6b52-6738-761181204567@2ndquadrant.com

5 years agoFill in extraUpdatedCols in logical replication
Peter Eisentraut [Mon, 17 Feb 2020 14:19:58 +0000 (15:19 +0100)]
Fill in extraUpdatedCols in logical replication

The extraUpdatedCols field of the target RTE records which generated
columns are affected by an update.  This is used in a variety of
places, including per-column triggers and foreign data wrappers.  When
an update was initiated by a logical replication subscription, this
field was not filled in, so such an update would not affect generated
columns in a way that is consistent with normal updates.  To fix,
factor out some code from analyze.c to fill in extraUpdatedCols in the
logical replication worker as well.

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/b05e781a-fa16-6b52-6738-761181204567@2ndquadrant.com

5 years agoAdd description about GSSOpenServer wait event into document.
Fujii Masao [Mon, 17 Feb 2020 07:16:08 +0000 (16:16 +0900)]
Add description about GSSOpenServer wait event into document.

This commit also updates wait event enum into alphabetical order.
Previously the enum entry for GSSOpenServer was added out-of-order.

Back-patch to v12 where commit b0b39f72b9 introduced
GSSOpenServer wait event. In v12, the commit doesn't include
the update of wait event enum, not to break ABI.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/949931aa-4ed4-d867-a7b5-de9c02b2292b@oss.nttdata.com