Commit Graph

19907 Commits

Author SHA1 Message Date
Michael Paquier 8270a0d9a9 Handle interrupts within a transaction context in REINDEX CONCURRENTLY
Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed.  This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.

This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly.  Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.

Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191013025145.GC4475@telsasoft.com
Backpatch-through: 12
2019-10-25 10:20:08 +09:00
Fujii Masao 3b0c59ac1c Fix typo in xlog.c.
Author: Fujii Masao
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHGQGwH7dtYvOZZ8c0AG5AJwH5pfiRdKaCptY1_RdHy0HYeRfQ@mail.gmail.com
2019-10-24 14:13:36 +09:00
Michael Paquier 5d3500da72 Acquire properly session-level lock on new index in REINDEX CONCURRENTLY
In the first transaction run for REINDEX CONCURRENTLY, a thinko in the
existing logic caused two session locks to be taken on the old index,
causing the session lock on the newly-created index to be missed.  This
made possible concurrent DDL commands (like ALTER INDEX) on the new
index while REINDEX CONCURRENTLY was processing from the point where the
first internal transaction committed.

This issue has been discovered while digging into another bug.

Author: Michael Paquier
Discussion: https://postgr.es/m/20191021074323.GB1869@paquier.xyz
Backpatch-through: 12
2019-10-23 15:04:48 +09:00
Michael Paquier e3db3f829f Clean up properly error_context_stack in autovacuum worker on exception
Any callback set would have no meaning in the context of an exception.
As an autovacuum worker exits quickly in this context, this could be
only an issue within EmitErrorReport(), where the elog hook is for
example called.  That's unlikely to going to be a problem, but let's be
clean and consistent with other code paths handling exceptions.  This is
present since 2909419, which introduced autovacuum.

Author: Ashwin Agrawal
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CALfoeisM+_+dgmAdAOHAu0k-ZpEHHqSSG=GRf3pKJGm8OqWX0w@mail.gmail.com
Backpatch-through: 9.4
2019-10-23 10:25:06 +09:00
Peter Eisentraut f86f46d091 Fix comment
The last argument of smgrextend() was renamed from isTemp to skipFsync
in debcec7dc3, but the comments at two
call sites were not updated.
2019-10-22 09:58:20 +02:00
Alexander Korotkov 52ad1e6599 Refactor jsonpath's compareDatetime()
This commit refactors come ridiculous coding in compareDatetime().  Also, it
provides correct cross-datatype comparison even when one of values overflows
during cast.  That eliminates dilemma on whether we should suppress overflow
errors during cast.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/32308.1569455803%40sss.pgh.pa.us
Discussion: https://postgr.es/m/a5629d0c-8162-7559-16aa-0c8390d6ba5f%40postgrespro.ru
Author: Nikita Glukhov, Alexander Korotkov
2019-10-21 23:07:07 +03:00
Alexander Korotkov a6888fde7f Refactor timestamp2timestamptz_opt_error()
While casting from timestamp to timestamptz we do timestamp2tm() then
tm2timestamp().  This commit eliminates call to tm2timestamp().  Instead, it
directly applies timezone offset to the original timestamp value.  That makes
upcoming datetime overflow handling in jsonpath easier.  That should also save
us some CPU cycles.

Discussion: https://postgr.es/m/CAPpHfdvRPRh_mTGar5WmDeRZ%3DU5dOXHdxspYYD%3D76m3knNGjXA%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Tom Lane
2019-10-21 23:07:07 +03:00
Etsuro Fujita 80831bcdbe Update obsolete comment.
Commit b52b7dc25, which moved code creating PartitionBoundInfo in
RelationBuildPartitionDesc() in partcache.c (relocated to partdesc.c
afterwards) to partbounds.c, should have updated this, but didn't.

Author: Etsuro Fujita
Reviewed-by: Alvaro Herrera
Backpatch-through: 12
Discussion: https://postgr.es/m/CAPmGK16Uxr%3DPatiGyaRwiQVLB7Y-GqbkK3AxRLVYzU0Czv%3DsEw%40mail.gmail.com
2019-10-21 17:30:00 +09:00
Amit Kapila 70a6c37d52 Fix memory leak introduced in commit 7df159a620.
We memorize all internal and empty leaf pages in the 1st vacuum stage for
gist indexes.  They are used in the 2nd stage, to delete all the empty
pages.  There was a memory context page_set_context for this purpose, but
we never used it.

Reported-by: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Backpatch-through: 12, where it got introduced
Discussion: https://postgr.es/m/CAA4eK1LGr+MN0xHZpJ2dfS8QNQ1a_aROKowZB+MPNep8FVtwAA@mail.gmail.com
2019-10-21 08:57:32 +05:30
Peter Eisentraut 5d3587d14b Fix most -Wundef warnings
In some cases #if was used instead of #ifdef in an inconsistent style.
Cleaning this up also helps when analyzing cases like
38d8dce61f where this makes a
difference.

There are no behavior changes here, but the change in pg_bswap.h would
prevent possible accidental misuse by third-party code.

Discussion: https://www.postgresql.org/message-id/flat/3b615ca5-c595-3f1d-fdf7-a429e564f614%402ndquadrant.com
2019-10-19 18:31:38 +02:00
Noah Misch 48cc59ed24 Use standard compare_exchange loop style in ProcArrayGroupClearXid().
Besides style, this might improve performance in the contended case.

Reviewed by Amit Kapila.

Discussion: https://postgr.es/m/20191015035348.GA4166224@rfd.leadboat.com
2019-10-18 20:21:10 -07:00
Michael Paquier f25968c496 Remove last traces of heap_open/close in the tree
Since pluggable storage has been introduced, those two routines have
been replaced by table_open/close, with some compatibility macros still
present to allow extensions to compile correctly with v12.

Some code paths using the old routines still remained, so replace them.
Based on the discussion done, the consensus reached is that it is better
to remove those compatibility macros so as nothing new uses the old
routines, so remove also the compatibility macros.

Discussion: https://postgr.es/m/20191017014706.GF5605@paquier.xyz
2019-10-19 11:18:15 +09:00
Fujii Masao ec1259e880 Fix failure of archive recovery with recovery_min_apply_delay enabled.
recovery_min_apply_delay parameter is intended for use with streaming
replication deployments. However, the document clearly explains that
the parameter will be honored in all cases if it's specified. So it should
take effect even if in archive recovery. But, previously, archive recovery
with recovery_min_apply_delay enabled always failed, and caused assertion
failure if --enable-caasert is enabled.

The cause of this problem is that; the ownership of recoveryWakeupLatch
that recovery_min_apply_delay uses was taken only when standby mode
is requested. So unowned latch could be used in archive recovery, and
which caused the failure.

This commit changes recovery code so that the ownership of
recoveryWakeupLatch is taken even in archive recovery. Which prevents
archive recovery with recovery_min_apply_delay from failing.

Back-patch to v9.4 where recovery_min_apply_delay was added.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
2019-10-18 22:32:18 +09:00
Fujii Masao 9b95a36be8 Make crash recovery ignore recovery_min_apply_delay setting.
In v11 or before, this setting could not take effect in crash recovery
because it's specified in recovery.conf and crash recovery always
starts without recovery.conf. But commit 2dedf4d9a8 integrated
recovery.conf into postgresql.conf and which unexpectedly allowed
this setting to take effect even in crash recovery. This is definitely
not good behavior.

To fix the issue, this commit makes crash recovery always ignore
recovery_min_apply_delay setting.

Back-patch to v12 where the issue was added.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
Discussion: https://postgr.es/m/e445616d-023e-a268-8aa1-67b8b335340c@pgmasters.net
2019-10-18 22:24:18 +09:00
Alvaro Herrera 89403ed228 Fix typo
Apparently while this code was being developed,
ReindexRelationConcurrently operated on multiple relations.  The version
that was ultimately pushed doesn't, so this comment's use of plural is
inaccurate.
2019-10-18 14:49:39 +02:00
Alvaro Herrera d2efb90dba Update comments about progress reporting by index_drop
Michaël Paquier complained that index_drop is requesting progress
reporting for non-obvious reasons, so let's add a comment to explain
why.

Discussion: https://postgr.es/m/20191017010412.GH2602@paquier.xyz
2019-10-18 07:23:05 -03:00
Michael Paquier 3f60f690fa Fix timeout handling in logical replication worker
The timestamp tracking the last moment a message is received in a
logical replication worker was initialized in each loop checking if a
message was received or not, causing wal_receiver_timeout to be ignored
in basically any logical replication deployments.  This also broke the
ping sent to the server when reaching half of wal_receiver_timeout.

This simply moves the initialization of the timestamp out of the apply
loop to the beginning of LogicalRepApplyLoop().

Reported-by: Jehan-Guillaume De Rorthais
Author: Julien Rouhaud
Discussion: https://postgr.es/m/CAOBaU_ZHESFcWva8jLjtZdCLspMj7vqaB2k++rjHLY897ZxbYw@mail.gmail.com
Backpatch-through: 10
2019-10-18 14:26:29 +09:00
Alvaro Herrera 38ddeab13b Fix minor bug in logical-replication walsender shutdown
Logical walsender should exit when it catches up with sending WAL during
shutdown; but there was a rare corner case when it failed to because of
a race condition that puts it back to wait for more WAL instead -- but
since there wasn't any, it'd not shut down immediately.  It would only
continue the shutdown when wal_sender_timeout terminates the sleep,
which causes annoying waits during shutdown procedure.  Restructure the
code so that we no longer forget to set WalSndCaughtUp in that case.

This was an oversight in commit c6c333436.

Backpatch all the way down to 9.4.

Author: Craig Ringer, Álvaro Herrera
Discussion: https://postgr.es/m/CAMsr+YEuz4XwZX_QmnX_-2530XhyAmnK=zCmicEnq1vLr0aZ-g@mail.gmail.com
2019-10-17 15:06:06 +02:00
Thomas Munro 3c8c55dd54 When restoring GUCs in parallel workers, show an error context.
Otherwise it can be hard to see where an error is coming from, when
the parallel worker sets all the GUCs that it received from the
leader.  Bug #15726.  Back-patch to 9.5, where RestoreGUCState()
appeared.

Reported-by: Tiago Anastacio
Reviewed-by: Daniel Gustafsson, Tom Lane
Discussion: https://postgr.es/m/15726-6d67e4fa14f027b3%40postgresql.org
2019-10-17 13:47:01 +13:00
Thomas Munro 6bda2af039 Fix bug that could try to freeze running multixacts.
Commits 801c2dc7 and 801c2dc7 made it possible for vacuum to
try to freeze a multixact that is still running.  That was
prevented by a check, but raised an error.  Repair.

Back-patch all the way.

Author: Nathan Bossart, Jeremy Schneider
Reported-by: Jeremy Schneider
Reviewed-by: Jim Nasby, Thomas Munro
Discussion: https://postgr.es/m/DAFB8AFF-2F05-4E33-AD7F-FF8B0F760C17%40amazon.com
2019-10-17 09:59:21 +13:00
Alvaro Herrera 0d21f919eb Fix crash when reporting CREATE INDEX progress
A race condition can make us try to dereference a NULL pointer to the
PGPROC struct of a process that's already finished.  That results in
crashes during REINDEX CONCURRENTLY and CREATE INDEX CONCURRENTLY.

This was introduced in ab0dfc961b, so backpatch to pg12.

Reported by: Justin Pryzby
Reviewed-by: Michaël Paquier
Discussion: https://postgr.es/m/20191012004446.GT10470@telsasoft.com
2019-10-16 14:51:34 +02:00
Michael Paquier 1de4fd1092 Refresh some incorrect links in pg_crc.c/h
Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm0LPk9vTGTBPBRv0=fX=94o4r6-DuBbHNeCN2AH5bufLw@mail.gmail.com
2019-10-16 15:10:14 +09:00
Thomas Munro d5ac14f9cc Use libc version as a collation version on glibc systems.
Using glibc's version string to detect potential collation definition
changes is not 100% reliable, but it's better than nothing.  Currently
this affects only collations explicitly provided by "libc".  More work
will be needed to handle the default collation.

Author: Thomas Munro, based on a suggestion from Christoph Berg
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/4b76c6d4-ae5e-0dc6-7d0d-b5c796a07e34%402ndquadrant.com
2019-10-16 17:28:24 +13:00
Andres Freund cef82eda14 Fix CLUSTER on expression indexes.
Since the introduction of different slot types, in 1a0586de36, we
create a virtual slot in tuplesort_begin_cluster(). While that looks
right, it unfortunately doesn't actually work, as ExecStoreHeapTuple()
is used to store tuples in the slot. Unfortunately no regression tests
for CLUSTER on expression indexes existed so far.

Fix the slot type, and add bare bones tests for CLUSTER on expression
indexes.

Reported-By: Justin Pryzby
Author: Andres Freund
Discussion: https://postgr.es/m/20191011210320.GS10470@telsasoft.com
Backpatch: 12, like 1a0586de36
2019-10-15 10:40:13 -07:00
Peter Eisentraut bdb839cbde Update unicode.org URLs
Use https, consistent host name, remove references to ftp.  Also
update the URLs for CLDR, which has moved from Trac to GitHub.
2019-10-13 22:10:38 +02:00
Tom Lane 9abb2bfc04 In the postmaster, rely on the signal infrastructure to block signals.
POSIX sigaction(2) can be told to block a set of signals while a
signal handler executes.  Make use of that instead of manually
blocking and unblocking signals in the postmaster's signal handlers.
This should save a few cycles, and it also prevents recursive
invocation of signal handlers when many signals arrive in close
succession.  We have seen buildfarm failures that seem to be due to
postmaster stack overflow caused by such recursion (exacerbated by
a Linux PPC64 kernel bug).

This doesn't change anything about the way that it works on Windows.
Somebody might consider adjusting port/win32/signal.c to let it work
similarly, but I'm not in a position to do that.

For the moment, just apply to HEAD.  Possibly we should consider
back-patching this, but it'd be good to let it age awhile first.

Discussion: https://postgr.es/m/14878.1570820201@sss.pgh.pa.us
2019-10-13 15:48:26 -04:00
Michael Paquier 1df5875d39 Fix dependency handling of column drop with partitioned tables
When dropping a column on a partitioned table which has one or more
partitioned indexes, the operation was failing as dependencies with
partitioned indexes using the column dropped were not getting removed in
a way consistent with the columns involved across all the relations part
of an inheritance tree.

This commit refactors the code executing column drop so as all the
columns from an inheritance tree to remove are gathered first, and
dropped all at the end.  This way, we let the dependency machinery sort
out by itself the deletion of all the columns with the partitioned
indexes across a partition tree.

This issue has been introduced by 1d92a0c, so backpatch down to
REL_12_STABLE.

Author: Amit Langote, Michael Paquier
Reviewed-by: Álvaro Herrera, Ashutosh Sharma
Discussion: https://postgr.es/m/CA+HiwqE9kuBsZ3b5pob2-cvE8ofzPWs-og+g8bKKGnu6b4-yTQ@mail.gmail.com
Backpatch-through: 12
2019-10-13 17:51:55 +09:00
Peter Eisentraut b4675a8ae2 Fix use of term "verifier"
Within the context of SCRAM, "verifier" has a specific meaning in the
protocol, per RFCs.  The existing code used "verifier" differently, to
mean whatever is or would be stored in pg_auth.rolpassword.

Fix this by using the term "secret" for this, following RFC 5803.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/be397b06-6e4b-ba71-c7fb-54cae84a7e18%402ndquadrant.com
2019-10-12 21:41:59 +02:00
Fujii Masao 20961ceaf0 Make crash recovery ignore restore_command and recovery_end_command settings.
In v11 or before, those settings could not take effect in crash recovery
because they are specified in recovery.conf and crash recovery always
starts without recovery.conf. But commit 2dedf4d9a8 integrated
recovery.conf into postgresql.conf and which unexpectedly allowed
those settings to take effect even in crash recovery. This is definitely
not good behavior.

To fix the issue, this commit makes crash recovery always ignore
restore_command and recovery_end_command settings.

Back-patch to v12 where the issue was added.

Author: Fujii Masao
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/e445616d-023e-a268-8aa1-67b8b335340c@pgmasters.net
2019-10-11 15:47:59 +09:00
Andres Freund 93765bd956 Fix table rewrites that include a column without a default.
In c2fe139c20 I made ATRewriteTable() use tuple slots. Unfortunately
I did not notice that columns can be added in a rewrite that do not
have a default, when another column is added/altered requiring one.

Initialize columns to NULL again, and add tests.

Bug: #16038
Reported-By: anonymous
Author: Andres Freund
Discussion: https://postgr.es/m/16038-5c974541f2bf6749@postgresql.org
Backpatch: 12, where the bug was introduced in c2fe139c20
2019-10-09 22:00:50 -07:00
Peter Eisentraut 50518ec296 Revert "Use libc version as a collation version on glibc systems."
This reverts commit 9f90b1d08d.

This needs some refinements in the pg_dump and pg_upgrade tests.
2019-10-09 21:36:01 +02:00
Peter Eisentraut 9f90b1d08d Use libc version as a collation version on glibc systems.
Using glibc's version number to detect potential collation definition
changes is not 100% reliable, but it's better than nothing.

Author: Thomas Munro
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/4b76c6d4-ae5e-0dc6-7d0d-b5c796a07e34%402ndquadrant.com
2019-10-09 21:17:47 +02:00
Michael Paquier b8e19b932a Flush logical mapping files with fd opened for read/write at checkpoint
The file descriptor was opened with read-only to fsync a regular file,
which would cause EBADFD errors on some platforms.

This is similar to the recent fix done by a586cc4b (which was broken by
me with 82a5649), except that I noticed this issue while monitoring the
backend code for similar mistakes.  Backpatch to 9.4, as this has been
introduced since logical decoding exists as of b89e151.

Author: Michael Paquier
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20191006045548.GA14532@paquier.xyz
Backpatch-through: 9.4
2019-10-09 13:30:43 +09:00
Peter Eisentraut 38d8dce61f Remove some code for old unsupported versions of MSVC
As of d9dd406fe2, we require MSVC 2013,
which means _MSC_VER >= 1800.  This means that conditionals about
older versions of _MSC_VER can be removed or simplified.

Previous code was also in some cases handling MinGW, where _MSC_VER is
not defined at all, incorrectly, such as in pg_ctl.c and win32_port.h,
leading to some compiler warnings.  This should now be handled better.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
2019-10-08 10:50:54 +02:00
Michael Paquier a7471bd85c Update some outdated links about XLC and UNIX specification
Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm3Dy=dTdx8UCVw=DWbzLzmRUC1dkq45=heOZDUg3U_PtA@mail.gmail.com
2019-10-08 14:31:30 +09:00
Tom Lane 3887e9455f Check for too many postmaster children before spawning a bgworker.
The postmaster's code path for spawning a bgworker neglected to check
whether we already have the max number of live child processes.  That's
a bit hard to hit, since it would necessarily be a transient condition;
but if we do, AssignPostmasterChildSlot() fails causing a postmaster
crash, as seen in a report from Bhargav Kamineni.

To fix, invoke canAcceptConnections() in the bgworker code path, as we
do in the other code paths that spawn children.  Since we don't want
the same pmState tests in this case, add a child-process-type parameter
to canAcceptConnections() so that it can know what to do.

Back-patch to 9.5.  In principle the same hazard exists in 9.4, but the
code is enough different that this patch wouldn't quite fix it there.
Given the tiny usage of bgworkers in that branch it doesn't seem worth
creating a variant patch for it.

Discussion: https://postgr.es/m/18733.1570382257@sss.pgh.pa.us
2019-10-07 12:39:09 -04:00
Tom Lane ac12ab06a9 Avoid trying to release a List's initial allocation via repalloc().
Commit 1cff1b95a included some code that supposed it could repalloc()
a memory chunk to a smaller size without risk of the chunk moving.
That was not a great idea, because it depended on undocumented behavior
of AllocSetRealloc, which commit c477f3e44 changed thereby breaking it.
(Not to mention that this code ought to work with other memory context
types, which might not work the same...)  So get rid of the repalloc
calls, and instead just wipe the now-unused ListCell array and/or tell
Valgrind it's NOACCESS, as if we'd freed it.

In cases where the initial list allocation had been quite large, this
could represent an annoying waste of space.  In principle we could
ameliorate that by allocating the initial cell array separately when
it exceeds some threshold.  But that would complicate new_list() which
is hot code, and the returns would materialize only in narrow cases.
On balance I don't think it'd be worth it.

Discussion: https://postgr.es/m/17059.1570208426@sss.pgh.pa.us
2019-10-06 12:06:30 -04:00
Tomas Vondra 36425ece5d Change MemoryContextMemAllocated to return Size
Commit f2369bc610 switched most of the memory accounting from int64 to
Size, but it forgot to change the MemoryContextMemAllocated return type.
So this fixes that omission.

Discussion: https://www.postgresql.org/message-id/11238.1570200198%40sss.pgh.pa.us
2019-10-05 20:49:39 +02:00
Andres Freund d986d4e87f Fix crash caused by EPQ happening with a before update trigger present.
When ExecBRUpdateTriggers()'s GetTupleForTrigger() follows an EPQ
chain the former needs to run the result tuple through the junkfilter
again, and update the slot containing the new version of the tuple to
contain that new version. The input tuple may already be in the
junkfilter's output slot, which used to be OK - we don't need the
previous version anymore. Unfortunately ff11e7f4b9 started to use
ExecCopySlot() to update newslot, and ExecCopySlot() doesn't support
copying a slot into itself, leading to a slot in a corrupt
state, which then can cause crashes or other symptoms.

Fix this by skipping the ExecCopySlot() when copying into itself.

While we could have easily made ExecCopySlot() handle that case, it
seems better to add an assert forbidding doing so instead. As the goal
of copying might be to make the contents of one slot independent from
another, it seems failure prone to handle doing so silently.

A follow-up commit will add tests for the obviously under-covered
combination of EPQ and triggers. Done as a separate commit as it might
make sense to backpatch them further than this bug.

Also remove confusion with confusing variable names for slots in
ExecBRDeleteTriggers() and ExecBRUpdateTriggers().

Bug: #16036
Reported-By: Антон Власов
Author: Andres Freund
Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org
Backpatch: 12-, where ff11e7f4b9 was merged
2019-10-04 13:50:49 -07:00
Andres Freund a586cc4b6c Use a fd opened for read/write when syncing slots during startup, take 2.
Cribbing from dfbaed45975:
    Some operating systems, including the reporter's windows, return EBADFD
    or similar when fsync() is invoked on a O_RDONLY file descriptor.
    Unfortunately RestoreSlotFromDisk() does exactly that; which causes
    failures after restarts in at least some scenarios.

    If you hit the bug the error message will be something like
    ERROR: could not fsync file "pg_replslot/$name/state": Bad file descriptor

    Simply use O_RDWR instead of O_RDONLY when opening the relevant file
    descriptor to fix the bug.

Unfortunately this fix was undone in 82a5649fb9. Re-apply, and add a
comment.

Bug: 16039
Reported-By: Hans Buschmann
Author: Andres Freund
Discussion: https://postgr.es/m/16039-196fc97cc05e141c@postgresql.org
Backpatch: 12-, as 82a5649fb9
2019-10-04 13:34:28 -07:00
Robert Haas 2e8b6bfa90 Rename some toasting functions based on whether they are heap-specific.
The old names for the attribute-detoasting functions names included
the word "heap," which seems outdated now that the heap is only one of
potentially many table access methods.

On the other hand, toast_insert_or_update and toast_delete are
heap-specific, so rename them by adding "heap_" as a prefix.

Not all of the work of making the TOAST system fully accessible to AMs
other than the heap is done yet, but there seems to be little harm in
getting this renaming out of the way now. Commit
8b94dab066 already divided up the
functions among various files partially according to whether it was
intended that they should be heap-specific or AM-agnostic, so this is
just clarifying the division contemplated by that commit.

Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro,
Andres Freund, and Álvaro Herrera.

Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
2019-10-04 14:24:46 -04:00
Tom Lane 61aa9f544a Fix bitshiftright()'s zero-padding some more.
Commit 5ac0d9360 failed to entirely fix bitshiftright's habit of
leaving one-bits in the pad space that should be all zeroes,
because in a moment of sheer brain fade I'd concluded that only
the code path used for not-a-multiple-of-8 shift distances needed
to be fixed.  Of course, a multiple-of-8 shift distance can also
cause the problem, so we need to forcibly zero the extra bits
in both cases.

Per bug #16037 from Alexander Lakhin.  As before, back-patch to all
supported branches.

Discussion: https://postgr.es/m/16037-1d1ebca564db54f4@postgresql.org
2019-10-04 10:34:40 -04:00
Tomas Vondra f2369bc610 Use Size instead of int64 to track allocated memory
Commit 5dd7fc1519 added block-level memory accounting, but used int64 variable to
track the amount of allocated memory. That is incorrect, because we have Size for
exactly these purposes, but it was mostly harmless until c477f3e449 which changed
how we handle with repalloc() when downsizing the chunk. Previously we've ignored
these cases and just kept using the original chunk, but now we need to update the
accounting, and the code was doing this:

    context->mem_allocated += blksize - oldblksize;

Both blksize and oldblksize are Size (so unsigned) which means the subtraction
underflows, producing a very high positive value. On 64-bit platforms (where Size
has the same size as mem_alllocated) this happens to work because the result wraps
to the right value, but on (some) 32-bit platforms this fails.

This fixes two things - it changes mem_allocated (and related variables) to Size,
and it splits the update to two separate steps, to prevent any underflows.

Discussion: https://www.postgresql.org/message-id/15151.1570163761%40sss.pgh.pa.us
2019-10-04 16:10:56 +02:00
Robert Haas 967e276e9f Remove AtSubStart_Notify.
Allocate notify-related state lazily instead. This makes trivial
subtransactions noticeably faster.

Patch by me, reviewed and tested by Dilip Kumar, Kyotaro Horiguchi,
and Jeevan Ladhe.

Discussion: https://postgr.es/m/CA+TgmobE1J22S1eC-6N-je9LgrcwZypkwp+zH6JXo9mc=4Nk3A@mail.gmail.com
2019-10-04 08:19:25 -04:00
Tom Lane 8e10405c74 Avoid unnecessary out-of-memory errors during encoding conversion.
Encoding conversion uses the very simplistic rule that the output
can't be more than 4X longer than the input, and palloc's a buffer
of that size.  This results in failure to convert any string longer
than 1/4 GB, which is becoming an annoying limitation.

As a band-aid to improve matters, allow the allocated output buffer
size to exceed 1GB.  We still insist that the final result fit into
MaxAllocSize (1GB), though.  Perhaps it'd be safe to relax that
restriction, but it'd require close analysis of all callers, which
is daunting (not least because external modules might call these
functions).  For the moment, this should allow a 2X to 4X improvement
in the longest string we can convert, which is a useful gain in
return for quite a simple patch.

Also, once we have successfully converted a long string, repalloc
the output down to the actual string length, returning the excess
to the malloc pool.  This seems worth doing since we can usually
expect to give back several MB if we take this path at all.

This still leaves much to be desired, most notably that the assumption
that MAX_CONVERSION_GROWTH == 4 is very fragile, and yet we have no
guard code verifying that the output buffer isn't overrun.  Fixing
that would require significant changes in the encoding conversion
APIs, so it'll have to wait for some other day.

The present patch seems safely back-patchable, so patch all supported
branches.

Alvaro Herrera and Tom Lane

Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql
Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us
2019-10-03 17:34:25 -04:00
Tom Lane c477f3e449 Allow repalloc() to give back space when a large chunk is downsized.
Up to now, if you resized a large (>8K) palloc chunk down to a smaller
size, aset.c made no attempt to return any space to the malloc pool.
That's unpleasant if a really large allocation is resized to a
significantly smaller size.  I think no such cases existed when this
code was designed, and I'm not sure whether they're common even yet,
but an upcoming fix to encoding conversion will certainly create such
cases.  Therefore, fix AllocSetRealloc so that it gives realloc()
a chance to do something with the block.  This doesn't noticeably
increase complexity, we mostly just have to change the order in which
the cases are considered.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql
Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us
2019-10-03 13:56:26 -04:00
Andrew Gierth b7a1c5539a Selectively include window frames in expression walks/mutates.
query_tree_walker and query_tree_mutator were skipping the
windowClause of the query, without regard for the fact that the
startOffset and endOffset in a WindowClause node are expression trees
that need to be processed. This was an oversight in commit ec4be2ee6
from 2010 which added the expression fields; the main symptom is that
function parameters in window frame clauses don't work in inlined
functions.

Fix (as conservatively as possible since this needs to not break
existing out-of-tree callers) and add tests.

Backpatch all the way, since this has been broken since 9.0.

Per report from Alastair McKinley; fix by me with kibitzing and review
from Tom Lane.

Discussion: https://postgr.es/m/DB6PR0202MB2904E7FDDA9D81504D1E8C68E3800@DB6PR0202MB2904.eurprd02.prod.outlook.com
2019-10-03 10:54:52 +01:00
Michael Paquier df86e52cac Remove temporary WAL and history files at the end of archive recovery
cbc55da has reworked the order of some actions at the end of archive
recovery.  Unfortunately this overlooked the fact that the startup
process needs to remove RECOVERYXLOG (for temporary WAL segment newly
recovered from archives) and RECOVERYHISTORY (for temporary history
file) at this step, leaving the files around even after recovery ended.

Backpatch to 9.5, like the previous commit.

Author: Sawada Masahiko
Reviewed-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/CAD21AoBO_eDQub6zojFnWtnmutRBWvYf7=cW4Hsqj+U_R26w3Q@mail.gmail.com
Backpatch-through: 9.5
2019-10-02 15:53:07 +09:00
Michael Paquier 9555cc8d2b Revert hooks for session start and end, take two
The location of the session end hook has been chosen so as it is
possible to allow modules to do their own transactions, however any
trying to any any subsystem which went through before_shmem_exit()
would cause issues, limiting the pluggability of the hook.

Per discussion with Tom Lane and Andres Freund.

Discussion: https://postgr.es/m/18722.1569906636@sss.pgh.pa.us
2019-10-02 09:55:27 +09:00
Tomas Vondra fa2fe04bf1 Mark two variables in in aset.c with PG_USED_FOR_ASSERTS_ONLY
This fixes two compiler warnings about unused variables in non-assert builds,
introduced by 5dd7fc1519.
2019-10-01 14:39:06 +02:00
Tomas Vondra 11a078cf87 Optimize partial TOAST decompression
Commit 4d0e994eed added support for partial TOAST decompression, so the
decompression is interrupted after producing the requested prefix. For
prefix and slices near the beginning of the entry, this may saves a lot
of decompression work.

That however only deals with decompression - the whole compressed entry
was still fetched and re-assembled, even though the compression used
only a small fraction of it. This commit improves that by computing how
much compressed data may be needed to decompress the requested prefix,
and then fetches only the necessary part.

We always need to fetch a bit more compressed data than the requested
(uncompressed) prefix, because the prefix may not be compressible at all
and pglz itself adds a bit of overhead. That means this optimization is
most effective when the requested prefix is much smaller than the whole
compressed entry.

Author: Binguo Bao
Reviewed-by: Andrey Borodin, Tomas Vondra, Paul Ramsey
Discussion: https://www.postgresql.org/message-id/flat/CAL-OGkthU9Gs7TZchf5OWaL-Gsi=hXqufTxKv9qpNG73d5na_g@mail.gmail.com
2019-10-01 14:28:28 +02:00
Michael Paquier e788bd924c Add hooks for session start and session end, take two
These hooks can be used in loadable modules.  A simple test module is
included.

The first attempt was done with cd8ce3a but we lacked handling for
NO_INSTALLCHECK in the MSVC scripts (problem solved afterwards by
431f1599) so the buildfarm got angry.  This also fixes a couple of
issues noticed upon review compared to the first attempt, so the code
has slightly changed, resulting in a more simple test module.

Author: Fabrízio de Royes Mello, Yugo Nagata
Reviewed-by: Andrew Dunstan, Michael Paquier, Aleksandr Parfenov
Discussion: https://postgr.es/m/20170720204733.40f2b7eb.nagata@sraoss.co.jp
Discussion: https://postgr.es/m/20190823042602.GB5275@paquier.xyz
2019-10-01 12:15:25 +09:00
Tomas Vondra 5dd7fc1519 Add transparent block-level memory accounting
Adds accounting of memory allocated in a memory context. Compared to
various ad hoc solutions, the main advantage is that the accounting is
transparent and does not require direct control over allocations (this
matters for use cases where the allocations happen in user code, like
for example aggregate states allocated in a transition functions).

To reduce overhead, the accounting happens at the block level (not for
individual chunks) and only the context immediately owning the block is
updated. When inquiring about amount of memory allocated in a context,
we have to recursively walk all children contexts.

This "lazy" accounting works well for cases with relatively small number
of contexts in the relevant subtree and/or with infrequent inquiries.

Author: Jeff Davis
Reivewed-by: Tomas Vondra, Melanie Plageman, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/flat/027a129b8525601c6a680d27ce3a7172dab61aab.camel@j-davis.com
2019-10-01 03:13:39 +02:00
Andres Freund 36d22dd95b Don't generate EEOP_*_FETCHSOME operations for slots know to be virtual.
That avoids unnecessary work during both interpreted execution, and
JIT compiled expression evaluation. Both benefit from fewer expression
steps needing be processed, and for interpreted execution there now is
a fastpath dedicated to just fetching a value from a virtual
slot. That's e.g. beneficial for hashjoins over nodes that perform
projections, as the hashed columns are currently fetched individually.

Author: Soumyadeep Chakraborty, Andres Freund
Discussion: https://postgr.es/m/CAE-ML+9OKSN71+mHtfMD-L24oDp8dGTfaVjDU6U+j+FNAW5kRQ@mail.gmail.com
2019-09-30 16:06:16 -07:00
Andres Freund 34c9c53bb0 Reduce code duplication for ExecJust*Var operations.
This is mainly in preparation for adding further fastpath evaluation
routines.

Also reorder ExecJust*Var functions to be consistent with the order in
which they're used.

Author: Andres Freund
Discussion: https://postgr.es/m/CAE-ML+9OKSN71+mHtfMD-L24oDp8dGTfaVjDU6U+j+FNAW5kRQ@mail.gmail.com
2019-09-30 15:32:00 -07:00
Fujii Masao 7acf8a876b Make crash recovery ignore recovery target settings.
In v11 or before, recovery target settings could not take effect in
crash recovery because they are specified in recovery.conf and
crash recovery always starts without recovery.conf. But commit
2dedf4d9a8 integrated recovery.conf into postgresql.conf and
which unexpectedly allowed recovery target settings to take effect
even in crash recovery. This is definitely not good behavior.

To fix the issue, this commit makes crash recovery always ignore
recovery target settings.

Back-patch to v12.

Author: Peter Eisentraut
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/e445616d-023e-a268-8aa1-67b8b335340c@pgmasters.net
2019-09-30 10:18:15 +09:00
Andres Freund ac88807f9b jit: Re-allow JIT compilation of execGrouping.c hashtable comparisons.
In the course of 5567d12ce0, 356687bd8 and 317ffdfeaa, I changed
BuildTupleHashTable[Ext]'s call to ExecBuildGroupingEqual to not pass
in the parent node, but NULL. Which in turn prevents the tuple
equality comparator from being JIT compiled.  While that fixes
bug #15486, it is not actually necessary after all of the above commits,
as we don't re-build the comparator when using the new
BuildTupleHashTableExt() interface (as the content of the hashtable
are reset, but the TupleHashTable itself is not).

Therefore re-allow jit compilation for callers that use
BuildTupleHashTableExt with a separate context for "metadata" and
content.

As in the previous commit, there's ongoing work to make this easier to
test to prevent such regressions in the future, but that
infrastructure is not going to be backpatchable.

The performance impact of not JIT compiling hashtable equality
comparators can be substantial e.g. for aggregation queries that
aggregate a lot of input rows to few output rows (when there are a lot
of output groups, there will be fewer comparisons).

Author: Andres Freund
Discussion: https://postgr.es/m/20190927072053.njf6prdl3vb7y7qb@alap3.anarazel.de
Backpatch: 11, just as 5567d12ce0
2019-09-29 16:24:32 -07:00
Andres Freund 97e971ee05 Fix determination when slot types for upper executor nodes are fixed.
For many queries the fact that the tuple descriptor from the lower
node was not taken into account when determining whether the type of a
slot is fixed, lead to tuple deforming for such upper nodes not to be
JIT accelerated.

I broke this in 675af5c01e.

There is ongoing work to enable writing regression tests for related
behavior (including a patch that would have detected this
regression), by optionally showing such details in EXPLAIN. But as it
seems unlikely that that will be suitable for stable branches, just
merge the fix for now.

While it's fairly close to the 12 release window, the fact that 11
continues to perform JITed tuple deforming in these cases, that
there's still cases where we do so in 12, and the fact that the
performance regression can be sizable, weigh in favor of fixing it
now.

Author: Andres Freund
Discussion: https://postgr.es/m/20190927072053.njf6prdl3vb7y7qb@alap3.anarazel.de
Backpatch: 12-, where 675af5c01e was merged.
2019-09-29 15:46:17 -07:00
Peter Eisentraut 4e6f101e92 Fix compilation with older OpenSSL versions
Some older OpenSSL versions (0.9.8 branch) define TLS*_VERSION macros
but not the corresponding SSL_OP_NO_* macro, which causes the code for
handling ssl_min_protocol_version/ssl_max_protocol_version to fail to
compile.  To fix, add more #ifdefs and error handling.

Reported-by: Victor Wagner <vitus@wagner.pp.ru>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/20190924101859.09383b4f%40fafnir.local.vm
2019-09-28 22:49:01 +02:00
Michael Paquier 55282fa20f Remove code relevant to OpenSSL 0.9.6 in be/fe-secure-openssl.c
HEAD supports OpenSSL 0.9.8 and newer versions, and this code likely got
forgotten as its surrounding comments mention an incorrect version
number.

Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/20190927032311.GB8485@paquier.xyz
2019-09-28 15:22:49 +09:00
Andres Freund 3f6b3be39c Silence -Wmaybe-uninitialized compiler warnings in dbcommands.c.
When compiling postgres using gcc -O3, there are false-positive
warnings about the now initialized variables. Silence them.

Author: Peter Eisentraut, Andres Freund
Discussion: https://postgr.es/m/15fb2350-b8b8-e188-278f-0b34fdee5210@2ndquadrant.com
2019-09-27 14:14:30 -07:00
Andres Freund c967e13f40 Fix implicit-fallthrough compiler warning introduced in 6dda292d4d.
For some reason at least gcc-9 warns about the fallthrough, even
though it otherwise recognizes that elog(ERROR, ...) doesn't return.

Author: Andres Freund
2019-09-27 10:29:25 -07:00
Michael Paquier fbfa566488 Fix lockmode initialization for custom relation options
The code was enforcing AccessExclusiveLock for all custom relation
options, which is incorrect as the APIs allow a custom lock level to be
set.

While on it, fix a couple of inconsistencies in the tests and the README
of dummy_index_am.

Oversights in commit 773df88.

Discussion: https://postgr.es/m/20190925234152.GA2115@paquier.xyz
2019-09-27 09:31:20 +09:00
Michael Paquier 6e22813b2d Fix comment in xlogreader.c
This has been introduced by 709d003, that has moved readSegNo, readOff
and readPageTLI into a new structure called WALOpenSegment initialized
separately.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20190926.110809.248342687.horikyota.ntt@gmail.com
2019-09-26 11:53:37 +09:00
Alexander Korotkov 7881bb14f4 Correctly cast types to Datum and back in compareDatetime()
Discussion: https://postgr.es/m/CAPpHfdteFKW6MLpXM4md99m55YAuXs0n9_P2wiTq_EmG09doUA%40mail.gmail.com
2019-09-26 02:09:01 +03:00
Tom Lane b81a9c2fc5 Fix handling of GENERATED columns in CREATE TABLE LIKE INCLUDING DEFAULTS.
LIKE INCLUDING DEFAULTS tried to copy the attrdef expression without
copying the state of the attgenerated column.  This is in fact wrong,
because GENERATED and DEFAULT expressions are not the same kind of animal;
one can contain Vars and the other not.  We *must* copy attgenerated
when we're copying the attrdef expression.  Rearrange the if-tests
so that the expression is copied only when the correct one of
INCLUDING DEFAULTS and INCLUDING GENERATED has been specified.

Per private report from Manuel Rigger.

Tom Lane and Peter Eisentraut
2019-09-25 17:30:42 -04:00
Alexander Korotkov bffe1bd684 Implement jsonpath .datetime() method
This commit implements jsonpath .datetime() method as it's specified in
SQL/JSON standard.  There are no-argument and single-argument versions of
this method.  No-argument version selects first of ISO datetime formats
matching input string.  Single-argument version accepts template string as
its argument.

Additionally to .datetime() method itself this commit also implements
comparison ability of resulting date and time values.  There is some difficulty
because exising jsonb_path_*() functions are immutable, while comparison of
timezoned and non-timezoned types involves current timezone.  At first, current
timezone could be changes in session.  Moreover, timezones themselves are not
immutable and could be updated.  This is why we let existing immutable functions
throw errors on such non-immutable comparison.  In the same time this commit
provides jsonb_path_*_tz() functions which are stable and support operations
involving timezones.  As new functions are added to the system catalog,
catversion is bumped.

Support of .datetime() method was the only blocker prevents T832 from being
marked as supported.  sql_features.txt is updated correspondingly.

Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov.
Heavily revised by me.  Comments were adjusted by Liudmila Mantrova.

Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com
Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Alexander Korotkov, Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Liudmila Mantrova
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
2019-09-25 22:51:51 +03:00
Alexander Korotkov 6dda292d4d Allow datetime values in JsonbValue
SQL/JSON standard allows manipulation with datetime values.  So, it appears to
be convinient to allow datetime values to be represented in JsonbValue struct.
These datetime values are allowed for temporary representation only.  During
serialization datetime values are converted into strings.

SQL/JSON requires writing timestamps with timezone in the same timezone offset
as they were parsed.  This is why we allow storage of timezone offset in
JsonbValue struct.  For the same reason timezone offset argument is added to
JsonEncodeDateTime() function.

Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov.
Revised by me.  Comments were adjusted by Liudmila Mantrova.

Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com
Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Alexander Korotkov, Liudmila Mantrova
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
2019-09-25 22:51:51 +03:00
Alexander Korotkov 5bc450629b Error suppression support for upcoming jsonpath .datetime() method
Add support of error suppression in some date and time manipulation functions
as it's required for jsonpath .datetime() method support.  This commit doesn't
use PG_TRY()/PG_CATCH() in order to implement that.  Instead, it provides
internal versions of date and time functions used, which support error
suppression.

Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Alexander Korotkov, Nikita Glukhov
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
2019-09-25 22:51:51 +03:00
Alexander Korotkov 66c74f8b6e Implement parse_datetime() function
This commit adds parse_datetime() function, which implements datetime
parsing with extended features demanded by upcoming jsonpath .datetime()
method:

 * Dynamic type identification based on template string,
 * Support for standard-conforming 'strict' mode,
 * Timezone offset is returned as separate value.

Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov.
Revised by me.

Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com
Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Alexander Korotkov
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
2019-09-25 22:51:51 +03:00
Alexander Korotkov 1a950f37d0 Implement standard datetime parsing mode
SQL Standard 2016 defines rules for handling separators in datetime template
strings, which are different to to_date()/to_timestamp() rules.  Standard
allows only small set of separators and requires strict matching for them.

Standard applies to jsonpath .datetime() method and CAST (... FORMAT ...) SQL
clause.  We're not going to change handling of separators in existing
to_date()/to_timestamp() functions, because their current behavior is familiar
for users.  Standard behavior now available by special flag, which will be used
in upcoming .datetime() jsonpath method.

Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Alexander Korotkov
2019-09-25 22:51:29 +03:00
Alvaro Herrera 773df883e8 Support reloptions of enum type
All our current in core relation options of type string (not many,
admittedly) behave in reality like enums.  But after seeing an
implementation for enum reloptions, it's clear that strings are messier,
so introduce the new reloption type.  Switch all string options to be
enums instead.

Fortunately we have a recently introduced test module for reloptions, so
we don't lose coverage of string reloptions, which may still be used by
third-party modules.

Authors: Nikolay Shaplov, Álvaro Herrera
Reviewed-by: Nikita Glukhov, Aleksandr Parfenov
Discussion: https://postgr.es/m/43332102.S2V5pIjXRx@x200m
2019-09-25 15:56:52 -03:00
Michael Paquier 69f9410807 Allow definition of lock mode for custom reloptions
Relation options can define a lock mode other than AccessExclusiveMode
since 47167b7, but modules defining custom relation options did not
really have a way to enforce that.  Correct that by extending the
current API set so as modules can define a custom lock mode.

Author: Michael Paquier
Reviewed-by: Kuntal Ghosh
Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz
2019-09-25 10:13:52 +09:00
Michael Paquier 736b84eede Fix failure with lock mode used for custom relation options
In-core relation options can use a custom lock mode since 47167b7, that
has lowered the lock available for some autovacuum parameters.  However
it forgot to consider custom relation options.  This causes failures
with ALTER TABLE SET when changing a custom relation option, as its lock
is not defined.  The existing APIs to define a custom reloption does not
allow to define a custom lock mode, so enforce its initialization to
AccessExclusiveMode which should be safe enough in all cases.  An
upcoming patch will extend the existing APIs to allow a custom lock mode
to be defined.

The problem can be reproduced with bloom indexes, so add a test there.

Reported-by: Nikolay Sharplov
Analyzed-by: Thomas Munro, Michael Paquier
Author: Michael Paquier
Reviewed-by: Kuntal Ghosh
Discussion: https://postgr.es/m/20190920013831.GD1844@paquier.xyz
Backpatch-through: 9.6
2019-09-25 10:07:23 +09:00
Alexander Korotkov 90c0987258 Fix bug in pairingheap_SpGistSearchItem_cmp()
Our item contains only so->numberOfNonNullOrderBys of distances.  Reflect that
in the loop upper bound.

Discussion: https://postgr.es/m/53536807-784c-e029-6e92-6da802ab8d60%40postgrespro.ru
Author: Nikita Glukhov
Backpatch-through: 12
2019-09-25 01:47:36 +03:00
Alvaro Herrera 709d003fbd Rework WAL-reading supporting structs
The state-tracking of WAL reading in various places was pretty messy,
mostly because the ancient physical-replication WAL reading code wasn't
using the XLogReader abstraction.  This led to some untidy code.  Make
it prettier by creating two additional supporting structs,
WALSegmentContext and WALOpenSegment which keep track of WAL-reading
state.  This makes code cleaner, as well as supports more future
cleanup.

Author: Antonin Houska
Reviewed-by: Álvaro Herrera and (older versions) Robert Haas
Discussion: https://postgr.es/m/14984.1554998742@spoje.net
2019-09-24 16:39:53 -03:00
Tom Lane a9ae99d019 Prevent bogus pullup of constant-valued functions returning composite.
Fix an oversight in commit 7266d0997: as it stood, the code failed
when a function-in-FROM returns composite and can be simplified
to a composite constant.

For the moment, just test for composite result and abandon pullup
if we see one.  To make it actually work, we'd have to decompose
the composite constant into per-column constants; which is surely
do-able, but I'm not convinced it's worth the code space.

Per report from Raúl Marín Rodríguez.

Discussion: https://postgr.es/m/CAM6_UM4isP+buRA5sWodO_MUEgutms-KDfnkwGmryc5DGj9XuQ@mail.gmail.com
2019-09-24 12:11:32 -04:00
Fujii Masao 6d05086c0a Speedup truncations of relation forks.
When a relation is truncated, shared_buffers needs to be scanned
so that any buffers for the relation forks are invalidated in it.
Previously, shared_buffers was scanned for each relation forks, i.e.,
MAIN, FSM and VM, when VACUUM truncated off any empty pages
at the end of relation or TRUNCATE truncated the relation in place.
Since shared_buffers needed to be scanned multiple times,
it could take a long time to finish those commands especially
when shared_buffers was large.

This commit changes the logic so that shared_buffers is scanned only
one time for those three relation forks.

Author: Kirk Jamison
Reviewed-by: Masahiko Sawada, Thomas Munro, Alvaro Herrera, Takayuki Tsunakawa and Fujii Masao
Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C64E2067@g01jpexmbkw24
2019-09-24 17:31:26 +09:00
Andres Freund 30d1379658 Fix ExprState's tag to be of type NodeTag rather than Node.
This appears to have been an oversight in b8d7f053c5. As it's
effectively harmless, though confusing, only fix in master.

Author: Andres Freund
2019-09-23 15:28:13 -07:00
Peter Eisentraut 887248e97e Message style fixes 2019-09-23 13:38:39 +02:00
Tom Lane 5ac0d93600 Fix failure to zero-pad the result of bitshiftright().
If the bitstring length is not a multiple of 8, we'd shift the
rightmost bits into the pad space, which must be zeroes --- bit_cmp,
for one, depends on that.  This'd lead to the result failing to
compare equal to what it should compare equal to, as reported in
bug #16013 from Daryl Waycott.

This is, if memory serves, not the first such bug in the bitstring
functions.  In hopes of making it the last one, do a bit more work
than minimally necessary to fix the bug:

* Add assertion checks to bit_out() and varbit_out() to complain if
they are given incorrectly-padded input.  This will improve the
odds that manual testing of any new patch finds problems.

* Encapsulate the padding-related logic in macros to make it
easier to use.

Also, remove unnecessary padding logic from bit_or() and bitxor().
Somebody had already noted that we need not re-pad the result of
bit_and() since the inputs are required to be the same length,
but failed to extrapolate that to the other two.

Also, move a comment block that once was near the head of varbit.c
(but people kept putting other stuff in front of it), to put it in
the header block.

Note for the release notes: if anyone has inconsistent data as a
result of saving the output of bitshiftright() in a table, it's
possible to fix it with something like
UPDATE mytab SET bitcol = ~(~bitcol) WHERE bitcol != ~(~bitcol);

This has been broken since day one, so back-patch to all supported
branches.

Discussion: https://postgr.es/m/16013-c2765b6996aacae9@postgresql.org
2019-09-22 17:45:59 -04:00
Tom Lane 0a2f894c3c Fix typo in tts_virtual_copyslot.
The code used the destination slot's natts where it intended to
use the source slot's natts.  Adding an Assert shows that there
is no case in "make check-world" where these counts are different,
so maybe this is a harmless bug, but it's still a bug.

Takayuki Tsunakawa

Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1FD34C0E@G01JPEXMBYT05
2019-09-22 14:21:07 -04:00
Tom Lane 51004c7172 Make some efficiency improvements in LISTEN/NOTIFY.
Move the responsibility for advancing the NOTIFY queue tail pointer
from the listener(s) to the notification sender, and only have the
sender do it once every few queue pages, rather than after every batch
of notifications as at present.  This reduces the number of times we
execute asyncQueueAdvanceTail, and reduces contention when there are
multiple listeners (since that function requires exclusive lock).
This change relies on the observation that we don't really need the tail
pointer to be exactly up-to-date.  It's certainly not necessary to
attempt to release disk space more often than once per SLRU segment.
The only other usage of the tail pointer is that an incoming listener,
if it's the only listener in its database, will need to scan the queue
forward from the tail; but that's surely a less performance-critical
path than routine sending and receiving of notifies.  We compromise by
advancing the tail pointer after every 4 pages of output, so that it
shouldn't get more than a few pages behind.

Also, when sending signals to other backends after adding notify
message(s) to the queue, recognize that only backends in our own
database are going to care about those messages, so only such
backends really need to be awakened promptly.  Backends in other
databases should get kicked if they're well behind on reading the
queue, else they'll hold back the global tail pointer; but wakening
them for every single message is pointless.  This change can
substantially reduce signal traffic if listeners are spread among
many databases.  It won't help for the common case of only a single
active database, but the extra check costs very little.

Martijn van Oosterhout, with some adjustments by me

Discussion: https://postgr.es/m/CADWG95vtRBFDdrx1JdT1_9nhOFw48KaeTev6F_LtDQAFVpSPhA@mail.gmail.com
Discussion: https://postgr.es/m/CADWG95uFj8rLM52Er80JnhRsTbb_AqPP1ANHS8XQRGbqLrU+jA@mail.gmail.com
2019-09-22 11:46:29 -04:00
Tom Lane c160b8928c Straighten out leakproofness markings on text comparison functions.
Since we introduced the idea of leakproof functions, texteq and textne
were marked leakproof but their sibling text comparison functions were
not.  This inconsistency seemed justified because texteq/textne just
relied on memcmp() and so could easily be seen to be leakproof, while
the other comparison functions are far more complex and indeed can
throw input-dependent errors.

However, that argument crashed and burned with the addition of
nondeterministic collations, because now texteq/textne may invoke
the exact same varstr_cmp() infrastructure as the rest.  It makes no
sense whatever to give them different leakproofness markings.

After a certain amount of angst we've concluded that it's all right
to consider varstr_cmp() to be leakproof, mostly because the other
choice would be disastrous for performance of many queries where
leakproofness matters.  The input-dependent errors should only be
reachable for corrupt input data, or so we hope anyway; certainly,
if they are reachable in practice, we've got problems with requirements
as basic as maintaining a btree index on a text column.

Hence, run around to all the SQL functions that derive from varstr_cmp()
and mark them leakproof.  This should result in a useful gain in
flexibility/performance for queries in which non-leakproofness degrades
the efficiency of the query plan.

Back-patch to v12 where nondeterministic collations were added.
While this isn't an essential bug fix given the determination
that varstr_cmp() is leakproof, we might as well apply it now that
we've been forced into a post-beta4 catversion bump.

Discussion: https://postgr.es/m/31481.1568303470@sss.pgh.pa.us
2019-09-21 16:56:30 -04:00
Tom Lane 2810396312 Fix up handling of nondeterministic collations with pattern_ops opclasses.
text_pattern_ops and its siblings can't be used with nondeterministic
collations, because they use the text_eq operator which will not behave
as bitwise equality if applied with a nondeterministic collation.  The
initial implementation of that restriction was to insert a run-time test
in the related comparison functions, but that is inefficient, may throw
misleading errors, and will throw errors in some cases that would work.
It seems sufficient to just prevent the combination during CREATE INDEX,
so do that instead.

Lacking any better way to identify the opclasses involved, we need to
hard-wire tests for them, which requires hand-assigned values for their
OIDs, which forces a catversion bump because they previously had OIDs
that would be assigned automatically.  That's slightly annoying in the
v12 branch, but fortunately we're not at rc1 yet, so just do it.

Back-patch to v12 where nondeterministic collations were added.

In passing, run make reformat-dat-files, which found some unrelated
whitespace issues (slightly different ones in HEAD and v12).

Peter Eisentraut, with small corrections by me

Discussion: https://postgr.es/m/22566.1568675619@sss.pgh.pa.us
2019-09-21 16:29:17 -04:00
Alvaro Herrera 1a2983231d Split out code into new getKeyJsonValueFromContainer()
The new function stashes its output value in a JsonbValue that can be
passed in by the caller, which enables some of them to pass
stack-allocated structs -- saving palloc cycles.  It also allows some
callers that know they are handling a jsonb object to use this new jsonb
object-specific API, instead of going through generic container
findJsonbValueFromContainer.

Author: Nikita Glukhov
Discussion: https://postgr.es/m/7c417f90-f95f-247e-ba63-d95e39c0ad14@postgrespro.ru
2019-09-20 20:18:11 -03:00
Alvaro Herrera dbb9aeda99 Optimize get_jsonb_path_all avoiding an iterator
Instead of creating an iterator object at each step down the JSONB
object/array, we can just just examine its object/array flags, which is
faster.  Also, use the recently introduced JsonbValueAsText instead of
open-coding the same thing, for code simplicity.

Author: Nikita Glukhov
Discussion: https://postgr.es/m/7c417f90-f95f-247e-ba63-d95e39c0ad14@postgrespro.ru
2019-09-20 19:31:32 -03:00
Alvaro Herrera abb014a631 Refactor code into new JsonbValueAsText, and use it more
jsonb_object_field_text and jsonb_array_element_text both contained
identical copies of this code, so extract that into new routine
JsonbValueAsText.  This can also be used in other places, to measurable
performance benefit: the jsonb_each() and jsonb_array_elements()
functions can use it for outputting text forms instead of their less
efficient current implementation (because we no longer need to build
intermediate a jsonb representation of each value).

Author: Nikita Glukhov
Discussion: https://postgr.es/m/7c417f90-f95f-247e-ba63-d95e39c0ad14@postgrespro.ru
2019-09-20 19:30:16 -03:00
Tom Lane e56cad84d5 Fix some minor spec-compliance issues in jsonpath lexer.
Although the SQL/JSON tech report makes reference to ECMAScript which
allows both single- and double-quoted strings, all the rest of the
report speaks only of double-quoted string literals in jsonpaths.
That's more compatible with JSON itself; moreover single-quoted strings
are hard to use inside a jsonpath that is itself a single-quoted SQL
literal.  So guess that the intent is to allow only double-quoted
literals, and remove lexer support for single-quoted literals.
It'll be less painful to add this again later if we're wrong, than to
remove a shipped feature.

Also, adjust the lexer so that unrecognized backslash sequences are
treated as just meaning the escaped character, not as errors.  This
change has much better support in the standards, as JSON, JavaScript
and ECMAScript all make it plain that that's what's supposed to
happen.

Back-patch to v12.

Discussion: https://postgr.es/m/CAPpHfdvDci4iqNF9fhRkTqhe-5_8HmzeLt56drH%2B_Rv2rNRqfg@mail.gmail.com
2019-09-20 14:22:58 -04:00
Alvaro Herrera d1b0007639 Fix progress report of REINDEX INDEX
I (Álvaro) broke that in commit 6212276e43 -- forgot to set the
necessary flag.  Repair.

Author: Amit Langote
Discussion: https://postgr.es/m/CA+HiwqEaM2tV5awKhP1vSbgjQe_uXVU15Oi4sTgwgempwMiT8g@mail.gmail.com
2019-09-20 12:56:00 -03:00
Alexander Korotkov 8c8a267201 Fix freeing old values in index_store_float8_orderby_distances()
6cae9d2c10 has added an error in freeing old values in
index_store_float8_orderby_distances() function.  It looks for old value in
scan->xs_orderbynulls[i] after setting a new value there.
This commit fixes that.  Also it removes short-circuit in handling
distances == NULL situation.  Now distances == NULL will be treated the same
way as array with all null distances.  That is, previous values will be freed
if any.

Reported-by: Tom Lane, Nikita Glukhov
Discussion: https://postgr.es/m/CAPpHfdu2wcoAVAm3Ek66rP%3Duo_C-D84%2B%2Buf1VEcbyi_caBXWCA%40mail.gmail.com
Discussion: https://postgr.es/m/426580d3-a668-b9d1-7b8e-f74d1a6524e0%40postgrespro.ru
Backpatch-through: 12
2019-09-20 01:19:08 +03:00
Alexander Korotkov 6cae9d2c10 Improve handling of NULLs in KNN-GiST and KNN-SP-GiST
This commit improves subject in two ways:

 * It removes ugliness of 02f90879e7, which stores distance values and null
   flags in two separate arrays after GISTSearchItem struct.  Instead we pack
   both distance value and null flag in IndexOrderByDistance struct.  Alignment
   overhead should be negligible, because we typically deal with at most few
   "col op const" expressions in ORDER BY clause.
 * It fixes handling of "col op NULL" expression in KNN-SP-GiST.  Now, these
   expression are not passed to support functions, which can't deal with them.
   Instead, NULL result is implicitly assumed.  It future we may decide to
   teach support functions to deal with NULL arguments, but current solution is
   bugfix suitable for backpatch.

Reported-by: Nikita Glukhov
Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru
Author: Nikita Glukhov
Reviewed-by: Alexander Korotkov
Backpatch-through: 9.4
2019-09-19 21:48:39 +03:00
Peter Eisentraut e1c8743e6c GSSAPI error message improvements
Make the error messages around GSSAPI encryption a bit clearer.  Tweak
some messages to avoid plural problems.

Also make a code change for clarity.  Using "conf" for "confidential"
is quite confusing.  Using "conf_state" is perhaps not much better but
that's what the GSSAPI documentation uses, so there is at least some
hope of understanding it.
2019-09-19 15:09:49 +02:00
Fujii Masao 33a94bae60 Remove unused smgrdounlinkfork() function.
smgrdounlinkfork() became dead code as the result of commit ece01aae47,
but it was left in place just in case we want it someday. However no users
have appeared in 7 years, so it's time to remove this unused function.

Author: Kirk Jamison
Discussion: https://www.postgresql.org/message-id/D09B13F772D2274BB348A310EE3027C64E2067@g01jpexmbkw24
2019-09-18 21:05:33 +09:00
Peter Eisentraut 48770492c3 Add some const decorations to array constants
Author: Mark G <markg735@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAEeOP_YFVeFjq4zDZLDQbLSRFxBiTpwBQHxCNgGd%2Bp5VztTXyQ%40mail.gmail.com
2019-09-17 22:03:00 +02:00
Tom Lane d5b90cd648 Fix bogus handling of XQuery regex option flags.
The SQL spec defers to XQuery to define what the option flags are
for LIKE_REGEX patterns.  XQuery says that:
* 's' allows the dot character to match newlines, which by
  default it will not;
* 'm' allows ^ and $ to match at newlines, not only at the
  start/end of the whole string.
Thus, these are *not* inverses as they are for the similarly-named
POSIX options, and neither one corresponds to the POSIX 'n' option.
Fortunately, Spencer's library does expose these two behaviors as
separately twiddlable flags, so we just have to fix the mapping from
JSP flag bits to REG flag bits.  I also chose to rename the symbol
for 's' to DOTALL, to make it clearer that it's not the inverse
of MLINE.

Also, XQuery says that if the 'q' flag "is used together with the m, s,
or x flag, that flag has no effect".  I read this as saying that 'q'
overrides the other flags; whoever wrote our code seems to have read
it backwards.

Lastly, while XQuery's 'x' flag is related to what Spencer's code
does for REG_EXPANDED, it's not the same or a subset.  It seems best
to treat XQuery's 'x' as unimplemented for now.  Maybe later we can
expand our regex code to offer 'x'-style parsing as a separate option.

While at it, refactor the jsonpath code so that (a) there's only
one copy of the flag transformation logic not two, and (b) the
processing of flags is independent of the order in which the flags
are written.

We need some documentation updates to go with this, but I'll
tackle that separately.

Back-patch to v12 where this code originated.

Discussion: https://postgr.es/m/CAPpHfdvDci4iqNF9fhRkTqhe-5_8HmzeLt56drH%2B_Rv2rNRqfg@mail.gmail.com
Reference: https://www.w3.org/TR/2017/REC-xpath-functions-31-20170321/#flags
2019-09-17 15:39:51 -04:00
Peter Eisentraut a25221f53c Remove mingwcompat.c
We believe that the issues that this was working around have been
fixed in MinGW more than 5 years ago, so this isn't necessary anymore.

Discussion: https://www.postgresql.org/message-id/flat/20190719050830.GK1859%40paquier.xyz
2019-09-17 11:34:28 +02:00
Alexander Korotkov b64b857f50 Support for SSSSS datetime format pattern
SQL Standard 2016 defines SSSSS format pattern for seconds past midnight in
jsonpath .datetime() method and CAST (... FORMAT ...) SQL clause.  In our
datetime parsing engine we currently support it with SSSS name.

This commit adds SSSSS as an alias for SSSS.  Alias is added in favor of
upcoming jsonpath .datetime() method.  But it's also supported in to_date()/
to_timestamp() as positive side effect.

Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Nikita Glukhov, Alexander Korotkov
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
2019-09-16 21:14:56 +03:00
Alexander Korotkov d589f94460 Support for FF1-FF6 datetime format patterns
SQL Standard 2016 defines FF1-FF9 format patters for fractions of seconds in
jsonpath .datetime() method and CAST (... FORMAT ...) SQL clause.  Parsing
engine of upcoming .datetime() method will be shared with to_date()/
to_timestamp().

This patch implements FF1-FF6 format patterns for upcoming jsonpath .datetime()
method.  to_date()/to_timestamp() functions will also get support of this
format patterns as positive side effect.  FF7-FF9 are not supported due to
lack of precision in our internal timestamp representation.

Extracted from original patch by Nikita Glukhov, Teodor Sigaev, Oleg Bartunov.
Heavily revised by me.

Discussion: https://postgr.es/m/fcc6fc6a-b497-f39a-923d-aa34d0c588e8%402ndQuadrant.com
Discussion: https://postgr.es/m/CAPpHfdsZgYEra_PeCLGNoXOWYx6iU-S3wF8aX0ObQUcZU%2B4XTw%40mail.gmail.com
Author: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov, Alexander Korotkov
Reviewed-by: Anastasia Lubennikova, Peter Eisentraut
2019-09-16 21:14:32 +03:00
Tom Lane d812257809 Fix bogus sizeof calculations.
Noted by Coverity.  Typo in 27cc7cd2b, so back-patch to v12
as that was.
2019-09-15 11:51:57 -04:00