]> granicus.if.org Git - postgresql/log
postgresql
7 years agoPrevent possibility of panics during shutdown checkpoint.
Andres Freund [Tue, 6 Jun 2017 01:53:41 +0000 (18:53 -0700)]
Prevent possibility of panics during shutdown checkpoint.

When the checkpointer writes the shutdown checkpoint, it checks
afterwards whether any WAL has been written since it started and
throws a PANIC if so.  At that point, only walsenders are still
active, so one might think this could not happen, but walsenders can
also generate WAL, for instance in BASE_BACKUP and logical decoding
related commands (e.g. via hint bits).  So they can trigger this panic
if such a command is run while the shutdown checkpoint is being
written.

To fix this, divide the walsender shutdown into two phases.  First,
checkpointer, itself triggered by postmaster, sends a
PROCSIG_WALSND_INIT_STOPPING signal to all walsenders.  If the backend
is idle or runs an SQL query this causes the backend to shutdown, if
logical replication is in progress all existing WAL records are
processed followed by a shutdown.  Otherwise this causes the walsender
to switch to the "stopping" state. In this state, the walsender will
reject any further replication commands. The checkpointer begins the
shutdown checkpoint once all walsenders are confirmed as
stopping. When the shutdown checkpoint finishes, the postmaster sends
us SIGUSR2. This instructs walsender to send any outstanding WAL,
including the shutdown checkpoint record, wait for it to be replicated
to the standby, and then exit.

Author: Andres Freund, based on an earlier patch by Michael Paquier
Reported-By: Fujii Masao, Andres Freund
Reviewed-By: Michael Paquier
Discussion: https://postgr.es/m/20170602002912.tqlwn4gymzlxpvs2@alap3.anarazel.de
Backpatch: 9.4, where logical decoding was introduced

7 years agoHave walsenders participate in procsignal infrastructure.
Andres Freund [Tue, 6 Jun 2017 01:53:41 +0000 (18:53 -0700)]
Have walsenders participate in procsignal infrastructure.

The non-participation in procsignal was a problem for both changes in
master, e.g. parallelism not working for normal statements run in
walsender backends, and older branches, e.g. recovery conflicts and
catchup interrupts not working for logical decoding walsenders.

This commit thus replaces the previous WalSndXLogSendHandler with
procsignal_sigusr1_handler.  In branches since db0f6cad48 that can
lead to additional SetLatch calls, but that only rarely seems to make
a difference.

Author: Andres Freund
Reviewed-By: Michael Paquier
Discussion: https://postgr.es/m/20170421014030.fdzvvvbrz4nckrow@alap3.anarazel.de
Backpatch: 9.4, earlier commits don't seem to benefit sufficiently

7 years agoRevert "Prevent panic during shutdown checkpoint"
Andres Freund [Tue, 6 Jun 2017 01:53:41 +0000 (18:53 -0700)]
Revert "Prevent panic during shutdown checkpoint"

This reverts commit 086221cf6b1727c2baed4703c582f657b7c5350e, which
was made to master only.

The approach implemented in the above commit has some issues.  While
those could easily be fixed incrementally, doing so would make
backpatching considerably harder, so instead first revert this patch.

Discussion: https://postgr.es/m/20170602002912.tqlwn4gymzlxpvs2@alap3.anarazel.de

7 years agoDon't set application_name in logical replication workers
Peter Eisentraut [Tue, 6 Jun 2017 02:16:02 +0000 (22:16 -0400)]
Don't set application_name in logical replication workers

This was bothering some people because it's not the intended use of
application_name and it makes the default view of pg_stat_activity
bulky.

7 years agoFix ALTER SUBSCRIPTION grammar ambiguity
Peter Eisentraut [Tue, 6 Jun 2017 01:37:00 +0000 (21:37 -0400)]
Fix ALTER SUBSCRIPTION grammar ambiguity

There was a grammar ambiguity between SET PUBLICATION name REFRESH and
SET PUBLICATION SKIP REFRESH, because SKIP is not a reserved word.  To
resolve that, fold the refresh choice into the WITH options.  Refreshing
is the default now.

Reported-by: tushar <tushar.ahuja@enterprisedb.com>
7 years agoIgnore WL_POSTMASTER_DEATH latch event in single user mode
Peter Eisentraut [Sat, 3 Jun 2017 03:02:13 +0000 (23:02 -0400)]
Ignore WL_POSTMASTER_DEATH latch event in single user mode

Otherwise code that uses this will abort with an assertion failure,
because postmaster_alive_fds are not initialized.

Reported-by: tushar <tushar.ahuja@enterprisedb.com>
7 years agoFix thinko in previous openssl change
Andrew Dunstan [Tue, 6 Jun 2017 00:38:46 +0000 (20:38 -0400)]
Fix thinko in previous openssl change

7 years agoFix record length computation in pg_waldump/xlogdump.
Andres Freund [Mon, 5 Jun 2017 22:56:58 +0000 (15:56 -0700)]
Fix record length computation in pg_waldump/xlogdump.

The current method of computing the record length (excluding the
lenght of full-page images) has been wrong since the WAL format has
been revamped in 2c03216d831160bedd72d45f712601b6f7d03f1c.  Only the
main record's length was counted, but that can be significantly too
little if there's data associated with further blocks.

Fix by computing the record length as total_lenght - fpi_length.

Reported-By: Chen Huajun
Bug: #14687
Reviewed-By: Heikki Linnakangas
Discussion: https://postgr.es/m/20170603165939.1436.58887@wrigleys.postgresql.org
Backpatch: 9.5-

7 years agoCode review for shm_toc.h/.c.
Tom Lane [Mon, 5 Jun 2017 18:50:52 +0000 (14:50 -0400)]
Code review for shm_toc.h/.c.

Declare the toc_nentry field as uint32 not Size.  Since shm_toc_lookup()
reads the field without any lock, it has to be atomically readable, and
we do not assume that for fields wider than 32 bits.  Performance would
be impossibly bad for entry counts approaching 2^32 anyway, so there is
no need to try to preserve maximum width here.

This is probably an academic issue, because even if reading int64 isn't
atomic, the high order half would never change in practice.  Still, it's
a coding rule violation, so let's fix it.

Adjust some other not-terribly-well-chosen data types too, and copy-edit
some comments.  Make shm_toc_attach's Asserts consistent with
shm_toc_create's.

None of this looks to be a live bug, so no need for back-patch.

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

7 years agoFind openssl lib files in right directory for MSVC
Andrew Dunstan [Mon, 5 Jun 2017 18:24:42 +0000 (14:24 -0400)]
Find openssl lib files in right directory for MSVC

Some openssl builds put their lib files in a VC subdirectory, others do
not. Cater for both cases.

Backpatch to all live branches.

From an offline discussion with Leonardo Cecchi.

7 years agoDon't be so trusting that shm_toc_lookup() will always succeed.
Tom Lane [Mon, 5 Jun 2017 16:05:42 +0000 (12:05 -0400)]
Don't be so trusting that shm_toc_lookup() will always succeed.

Given the possibility of race conditions and so on, it seems entirely
unsafe to just assume that shm_toc_lookup() always finds the key it's
looking for --- but that was exactly what all but one call site were
doing.  To fix, add a "bool noError" argument, similarly to what we
have in many other functions, and throw an error on an unexpected
lookup failure.  Remove now-redundant Asserts that a rather random
subset of call sites had.

I doubt this will throw any light on buildfarm member lorikeet's
recent failures, because if an unnoticed lookup failure were involved,
you'd kind of expect a null-pointer-dereference crash rather than the
observed symptom.  But you never know ... and this is better coding
practice even if it never catches anything.

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

7 years agoFix typo in error message.
Heikki Linnakangas [Mon, 5 Jun 2017 08:38:26 +0000 (11:38 +0300)]
Fix typo in error message.

Daniele Varrazzo

Discussion: https://www.postgresql.org/message-id/CA+mi_8bqY5THP8hLKKSdMEr5GCz6M=hD6_uLbvFeyEBfwqUxeA@mail.gmail.com

7 years agoFix comments in simplehash.h.
Heikki Linnakangas [Mon, 5 Jun 2017 07:46:08 +0000 (10:46 +0300)]
Fix comments in simplehash.h.

Jeff Janes and me.

Discussion: https://www.postgresql.org/message-id/CAMkU=1zYnniLYg+W9itL93DXebCjx6Uk6m_=Xa8p_zM65X3S0Q@mail.gmail.com

7 years agoReplace over-optimistic Assert in partitioning code with a runtime test.
Tom Lane [Sun, 4 Jun 2017 20:20:03 +0000 (16:20 -0400)]
Replace over-optimistic Assert in partitioning code with a runtime test.

get_partition_parent felt that it could simply Assert that systable_getnext
found a tuple.  This is unlike any other caller of that function, and it's
unsafe IMO --- in fact, the reason I noticed it was that the Assert failed.
(OK, I was working with known-inconsistent catalog contents, but I wasn't
expecting the DB to fall over quite that violently.  The behavior in a
non-assert-enabled build wouldn't be very nice, either.)  Fix it to do what
other callers do, namely an actual runtime-test-and-elog.

Also, standardize the wording of elog messages that are complaining about
unexpected failure of systable_getnext.  90% of them say "could not find
tuple for <object>", so make the remainder do likewise.  Many of the
holdouts were using the phrasing "cache lookup failed", which is outright
misleading since no catcache search is involved.

7 years ago#ifdef out assorted unused GEQO code.
Tom Lane [Sun, 4 Jun 2017 17:34:05 +0000 (13:34 -0400)]
#ifdef out assorted unused GEQO code.

I'd always assumed that backend/optimizer/geqo/'s remarkably poor
showing on code coverage metrics was because we weren't exercising
it much in the regression tests.  But it turns out that a good chunk
of the problem is that there's a bunch of code that is physically
unreachable (because the calls to it are #ifdef'd out in geqo_main.c)
but is being built anyway.  Making the called code have #if guards
similar to the calling code saves a couple of kilobytes of executable
size and should make the coverage numbers more reflective of reality.

It's arguable that we should just delete all the unused recombination
mechanisms altogether, but I didn't feel a need to go that far today.

7 years agoDisallow CREATE INDEX if table is already in use in current session.
Tom Lane [Sun, 4 Jun 2017 16:02:31 +0000 (12:02 -0400)]
Disallow CREATE INDEX if table is already in use in current session.

If we allow this, whatever outer command has the table open will not know
about the new index and may fail to update it as needed, as shown in a
report from Laurenz Albe.  We already had such a prohibition in place for
ALTER TABLE, but the CREATE INDEX syntax missed the check.

Fixing it requires an API change for DefineIndex(), which conceivably
would break third-party extensions if we were to back-patch it.  Given
how long this problem has existed without being noticed, fixing it in
the back branches doesn't seem worth that risk.

Discussion: https://postgr.es/m/A737B7A37273E048B164557ADEF4A58B53A4DC9A@ntex2010i.host.magwien.gv.at

7 years agoAssorted translatable string fixes
Alvaro Herrera [Sun, 4 Jun 2017 15:41:16 +0000 (11:41 -0400)]
Assorted translatable string fixes

Mark our rusage reportage string translatable; remove quotes from type
names; unify formatting of very similar messages.

7 years agoRemove dead variables.
Tom Lane [Sun, 4 Jun 2017 00:35:52 +0000 (20:35 -0400)]
Remove dead variables.

Commit 512c7356b left a couple of variables unused except for being set.
My compiler didn't whine about this, but some buildfarm members did.

7 years agoAdd some missing backslash commands to psql's tab-completion knowledge.
Tom Lane [Sat, 3 Jun 2017 21:10:25 +0000 (17:10 -0400)]
Add some missing backslash commands to psql's tab-completion knowledge.

\if and related commands were overlooked here, as were \dRp and \dRs
from the logical-replication patch, as was \?.

While here, reformat the list to put each new first command letter on
a separate line; perhaps that will limit the need to reflow the whole
list when we add more commands in future.

Masahiko Sawada (reformatting by me)

Discussion: https://postgr.es/m/CAD21AoDW1QHtBsM33hV+Fg2mYEs+FWj4qtoCU72AwHAXQ3U6ZQ@mail.gmail.com

7 years agoFix <> and pattern-NOT-match estimators to handle nulls correctly.
Tom Lane [Sat, 3 Jun 2017 18:36:25 +0000 (14:36 -0400)]
Fix <> and pattern-NOT-match estimators to handle nulls correctly.

These estimators returned 1 minus the corresponding equality/match
estimate, which is incorrect: we need to subtract off the fraction
of nulls in the column, since those are neither equal nor not equal
to the comparison value.  The error only becomes obvious if the
nullfrac is large, but it could be very bad in a mostly-nulls
column, as reported in bug #14676 from Marko Tiikkaja.

To fix the <> case, refactor eqsel() and neqsel() to call a common
support routine, which can be made to account for nullfrac correctly.
The pattern-match cases were already factored that way, and it was
simply an oversight that patternsel() wasn't subtracting off nullfrac.

neqjoinsel() has a similar problem, but since we're elsewhere discussing
changing its behavior entirely, I left it alone for now.

This is a very longstanding bug, but I'm hesitant to back-patch a fix for
it.  Given the lack of prior complaints, such cases must not come up often,
so it's probably not worth the risk of destabilizing plans in stable
branches.

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

7 years agoFix old corner-case logic error in final_cost_nestloop().
Tom Lane [Sat, 3 Jun 2017 17:48:15 +0000 (13:48 -0400)]
Fix old corner-case logic error in final_cost_nestloop().

When costing a nestloop with stop-at-first-inner-match semantics, and a
non-indexscan inner path, final_cost_nestloop() wants to charge the full
scan cost of the inner rel at least once, with additional scans charged
at inner_rescan_run_cost which might be less.  However the logic for
doing this effectively assumed that outer_matched_rows is at least 1.
If it's zero, which is not unlikely for a small outer rel, we ended up
charging inner_run_cost plus N times inner_rescan_run_cost, as much as
double the correct charge for an outer rel with only one row that
we're betting won't be matched.  (Unless the inner rel is materialized,
in which case it has very small inner_rescan_run_cost and the cost
is not so far off what it should have been.)

The upshot of this was that the planner had a tendency to select plans
that failed to make effective use of the stop-at-first-inner-match
semantics, and that might have Materialize nodes in them even when the
predicted number of executions of the Materialize subplan was only 1.
This was not so obvious before commit 9c7f5229a, because the case only
arose in connection with semi/anti joins where there's not freedom to
reverse the join order.  But with the addition of unique-inner joins,
it could result in some fairly bad planning choices, as reported by
Teodor Sigaev.  Indeed, some of the test cases added by that commit
have plans that look dubious on closer inspection, and are changed
by this patch.

Fix the logic to ensure that we don't charge for too many inner scans.
I chose to adjust it so that the full-freight scan cost is associated
with an unmatched outer row if possible, not a matched one, since that
seems like a better model of what would happen at runtime.

This is a longstanding bug, but given the lesser impact in back branches,
and the lack of field complaints, I won't risk a back-patch.

Discussion: https://postgr.es/m/CAKJS1f-LzkUsFxdJ_-Luy38orQ+AdEXM5o+vANR+-pHAWPSecg@mail.gmail.com

7 years agoReceive invalidation messages correctly in tablesync worker
Peter Eisentraut [Sat, 3 Jun 2017 15:37:47 +0000 (11:37 -0400)]
Receive invalidation messages correctly in tablesync worker

We didn't accept any invalidation messages until the whole sync process
had finished (because it flattens all the remote transactions in the
single one).  So the sync worker didn't learn about subscription
changes/drop until it has finished.  This could lead to "orphaned" sync
workers.

Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
7 years agoMake tablesync worker exit when apply dies while it was waiting for it
Peter Eisentraut [Sat, 3 Jun 2017 13:18:52 +0000 (09:18 -0400)]
Make tablesync worker exit when apply dies while it was waiting for it

This avoids "orphaned" sync workers.

This was caused by a thinko in wait_for_sync_status_change.

Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
7 years agoAllow parallelism in COPY (query) TO ...;
Andres Freund [Sat, 3 Jun 2017 02:05:57 +0000 (19:05 -0700)]
Allow parallelism in COPY (query) TO ...;

Previously this was not allowed, as copy.c didn't set the
CURSOR_OPT_PARALLEL_OK flag when planning the query. Set it.

While the lack of parallel query for COPY isn't strictly speaking a
bug, it does prevent parallelism from being used in a facility
commonly used to run long running queries. Thus backpatch to 9.6.

Author: Andres Freund
Discussion: https://postgr.es/m/20170531231958.ihanapplorptykzm@alap3.anarazel.de
Backpatch: 9.6, where parallelism was introduced.

7 years agodoc: Add note that DROP SUBSCRIPTION drops replication slot
Peter Eisentraut [Thu, 1 Jun 2017 02:35:33 +0000 (22:35 -0400)]
doc: Add note that DROP SUBSCRIPTION drops replication slot

Add some information about what to do when this fails.

7 years agoRemove replication slot name check from ReplicationSlotAcquire()
Peter Eisentraut [Tue, 30 May 2017 18:57:01 +0000 (14:57 -0400)]
Remove replication slot name check from ReplicationSlotAcquire()

When trying to access a replication slot that is supposed to already
exist, we don't need to check the naming rules again.  If the slot
does not exist, we will then get a "does not exist" error message, which
is generally more useful from the perspective of an end user.

7 years agoFix signal handling in logical replication workers
Peter Eisentraut [Fri, 2 Jun 2017 18:46:00 +0000 (14:46 -0400)]
Fix signal handling in logical replication workers

The logical replication worker processes now use the normal die()
handler for SIGTERM and CHECK_FOR_INTERRUPTS() instead of custom code.
One problem before was that the apply worker would not exit promptly
when a subscription was dropped, which could lead to deadlocks.

Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
7 years agoFix copy/paste mistake in comment
Magnus Hagander [Fri, 2 Jun 2017 09:18:24 +0000 (11:18 +0200)]
Fix copy/paste mistake in comment

Amit Langote

7 years agoFix typo in comment
Magnus Hagander [Fri, 2 Jun 2017 07:40:54 +0000 (09:40 +0200)]
Fix typo in comment

Masahiko Sawada

7 years agoReorganize logical replication worker disconnect code
Peter Eisentraut [Fri, 2 Jun 2017 03:05:47 +0000 (23:05 -0400)]
Reorganize logical replication worker disconnect code

Move the walrcv_disconnect() calls into the before_shmem_exit handler.
This makes sure the call is always made even during exit by signal, it
saves some duplicate code, and it makes the logic more similar to
walreceiver.c.

Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>

7 years agopsql: Fix display of whether table is part of publication
Peter Eisentraut [Fri, 2 Jun 2017 01:13:40 +0000 (21:13 -0400)]
psql: Fix display of whether table is part of publication

If a FOR ALL TABLES publication was present, \d of a table would claim
for each table that it was part of the publication, even for tables that
are ignored for this purpose, such as system tables and unlogged tables.
Fix the query by using the function pg_get_publication_tables(), which
was intended for this purpose.

Reported-by: tushar <tushar.ahuja@enterprisedb.com>
Reviewed-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com>
7 years agoFix typo
Alvaro Herrera [Thu, 1 Jun 2017 21:45:53 +0000 (17:45 -0400)]
Fix typo

Reported by: Tim Goodaire
Discussion: https://postgr.es/m/20170601182230.1487.26008@wrigleys.postgresql.org

7 years agoModify sequence catalog tuple before invoking post alter hook.
Andres Freund [Thu, 1 Jun 2017 00:03:10 +0000 (17:03 -0700)]
Modify sequence catalog tuple before invoking post alter hook.

This seems to have been broken in the commit (1753b1b027035029) that
moved the sequence definition into pg_sequence.

Author: Andres Freund
Discussion: https://postgr.es/m/20170601000716.qxg7c46ukkiljjb3@alap3.anarazel.de
Backpatch: Bug is in master/v10 only

7 years agoMake ALTER SEQUENCE, including RESTART, fully transactional.
Andres Freund [Wed, 31 May 2017 23:39:27 +0000 (16:39 -0700)]
Make ALTER SEQUENCE, including RESTART, fully transactional.

Previously the changes to the "data" part of the sequence, i.e. the
one containing the current value, were not transactional, whereas the
definition, including minimum and maximum value were.  That leads to
odd behaviour if a schema change is rolled back, with the potential
that out-of-bound sequence values can be returned.

To avoid the issue create a new relfilenode fork whenever ALTER
SEQUENCE is executed, similar to how TRUNCATE ... RESTART IDENTITY
already is already handled.

This commit also makes ALTER SEQUENCE RESTART transactional, as it
seems to be too confusing to have some forms of ALTER SEQUENCE behave
transactionally, some forms not.  This way setval() and nextval() are
not transactional, but DDL is, which seems to make sense.

This commit also rolls back parts of the changes made in 3d092fe540
and f8dc1985f as they're now not needed anymore.

Author: Andres Freund
Discussion: https://postgr.es/m/20170522154227.nvafbsm62sjpbxvd@alap3.anarazel.de
Backpatch: Bug is in master/v10 only

7 years agoAlways use -fPIC, not -fpic, when building shared libraries with gcc.
Tom Lane [Thu, 1 Jun 2017 17:32:55 +0000 (13:32 -0400)]
Always use -fPIC, not -fpic, when building shared libraries with gcc.

On some platforms, -fpic fails for sufficiently large shared libraries.
We've mostly not hit that boundary yet, but there are some extensions
such as Citus and pglogical where it's becoming a problem.  A bit of
research suggests that the penalty for -fPIC is small, in the
single-digit-percentage range --- and there's none at all on popular
platforms such as x86_64.  So let's just default to -fPIC everywhere
and provide one less thing for extension developers to worry about.

Per complaint from Christoph Berg.  Back-patch to all supported branches.
(I did not bother to touch the recently-removed Makefiles for sco and
unixware in the back branches, though.  We'd have no way to test that
it doesn't break anything on those platforms.)

Discussion: https://postgr.es/m/20170529155850.qojdfrwkkqnjb3ap@msg.df7cb.de

7 years agoGenerate pg_basebackup temporary slot name using backend pid
Magnus Hagander [Wed, 31 May 2017 18:57:25 +0000 (20:57 +0200)]
Generate pg_basebackup temporary slot name using backend pid

Using the client pid can easily be non-unique when used on different
hosts. Using the backend pid should be guaranteed unique, since the
temporary slot gets removed when the client disconnects so it will be
gone even if the pid is renewed.

Reported by Ludovic Vaugeois-Pepin

7 years agoRestore accidentally-removed line.
Robert Haas [Wed, 31 May 2017 17:34:41 +0000 (13:34 -0400)]
Restore accidentally-removed line.

Commit 88e66d193fbaf756b3cc9bf94cad116aacbb355b is to blame.

Masahiko Sawada

Discussion: http://postgr.es/m/CAD21AoAXeb7O4hgg+efs8JT_SxpR4doAH5c5s-Z5WoRLstBZJA@mail.gmail.com

7 years agodoc: Add another migration item to release notes
Peter Eisentraut [Wed, 31 May 2017 17:39:28 +0000 (13:39 -0400)]
doc: Add another migration item to release notes

7 years agoAvoid -Wconversion warnings from direct use of GET_n_BYTES macros.
Tom Lane [Wed, 31 May 2017 15:27:21 +0000 (11:27 -0400)]
Avoid -Wconversion warnings from direct use of GET_n_BYTES macros.

The GET/SET_n_BYTES macros are meant to be infrastructure for the
DatumGetFoo/FooGetDatum macros, which include a cast to the intended
target type.  Using them directly without a cast, as DatumGetFloat4
and friends previously did, can yield warnings when -Wconversion is on.
This is of little significance when building Postgres proper, because
there are such a huge number of such warnings in the server that nobody
would think -Wconversion is of any use.  But some extensions build with
-Wconversion due to outside constraints.  Commit 14cca1bf8 did a disservice
to those extensions by moving DatumGetFloat4 et al into postgres.h,
where they can now cause warnings in extension builds.

To fix, use DatumGetInt32 and friends in place of the low-level macros.
This is arguably a bit cleaner anyway.

Chapman Flack

Discussion: https://postgr.es/m/592E4D04.1070609@anastigmatix.net

7 years agoSort syscache identifiers into alphabetical order.
Tom Lane [Tue, 30 May 2017 22:47:10 +0000 (18:47 -0400)]
Sort syscache identifiers into alphabetical order.

Not much point in having a convention about this if we don't enforce it.

Mark Dilger

Discussion: https://postgr.es/m/7F67FBEF-C3B3-404E-8EC6-E02ACB15D894@gmail.com

7 years agobrin: Don't crash on auto-summarization
Alvaro Herrera [Tue, 30 May 2017 21:32:53 +0000 (17:32 -0400)]
brin: Don't crash on auto-summarization

We were trying to free a pointer into a shared buffer, which never
works; and we were failing to release the buffer lock appropriately.
Fix those omissions.

While at it, improve documentation for brinGetTupleForHeapBlock, the
inadequacy of which evidently caused these bugs in the first place.

Reported independently by Zhou Digoal (bug #14668) and Alexander Sosna.

Discussion: https://postgr.es/m/8c31c11b-6adb-228d-22c2-4ace89fc9209@credativ.de
Discussion: https://postgr.es/m/20170524063323.29941.46339@wrigleys.postgresql.org

7 years agoFix wording in amvalidate error messages
Alvaro Herrera [Tue, 30 May 2017 19:45:42 +0000 (15:45 -0400)]
Fix wording in amvalidate error messages

Remove some gratuituous message differences by making the AM name
previously embedded in each message be a %s instead.  While at it, get
rid of terminology that's unclear and unnecessary in one message.

Discussion: https://postgr.es/m/20170523001557.bq2hbq7hxyvyw62q@alvherre.pgsql

7 years agodoc: Fix ALTER PUBLICATION details
Peter Eisentraut [Tue, 30 May 2017 15:47:19 +0000 (11:47 -0400)]
doc: Fix ALTER PUBLICATION details

Some of the text was made nonsensical by commit
e9500240661c03750923e6f539bfa2d75cfaa32a.  Fix that and make some other
minor changes.

Reported-by: Jeff Janes <jeff.janes@gmail.com>
7 years agoFix omission of locations in outfuncs/readfuncs partitioning node support.
Tom Lane [Tue, 30 May 2017 15:32:41 +0000 (11:32 -0400)]
Fix omission of locations in outfuncs/readfuncs partitioning node support.

We could have limped along without this for v10, which was my intention
when I annotated the bug in commit 76a3df6e5.  But consensus is that it's
better to fix it now and take the cost of a post-beta1 initdb (which is
needed because these node types are stored in pg_class.relpartbound).

Since we're forcing initdb anyway, take the opportunity to make the node
type identification strings match the node struct names, instead of being
randomly different from them.

Discussion: https://postgr.es/m/E1dFBEX-0004wt-8t@gemulon.postgresql.org

7 years agoFix improper quoting of format_type_be() output.
Tom Lane [Tue, 30 May 2017 01:48:26 +0000 (21:48 -0400)]
Fix improper quoting of format_type_be() output.

Per our message style guidelines, error messages incorporating the
results of format_type_be() and its siblings should not add quotes
around those results, because those functions already add quotes
at need.  Fix a few places that hadn't gotten that memo.

7 years agoMake edge-case behavior of jsonb_populate_record match json_populate_record
Tom Lane [Mon, 29 May 2017 23:29:42 +0000 (19:29 -0400)]
Make edge-case behavior of jsonb_populate_record match json_populate_record

json_populate_record throws an error if asked to convert a JSON scalar
or array into a composite type.  jsonb_populate_record was returning
a record full of NULL fields instead.  It seems better to make it
throw an error for this case as well.

Nikita Glukhov

Discussion: https://postgr.es/m/fbd1d566-bba0-a3de-d6d0-d3b1d7c24ff2@postgrespro.ru

7 years agoFix thinko in JsObjectSize() macro.
Tom Lane [Mon, 29 May 2017 22:51:56 +0000 (18:51 -0400)]
Fix thinko in JsObjectSize() macro.

The macro gave the wrong answers for a JsObject with is_json == 0:
it would return 1 if jsonb_cont == NULL, or if that wasn't NULL,
it would return 1 for any non-zero size.

We could fix that, but the only use of this macro at present is in the
JsObjectIsEmpty() macro, so it seems simpler and clearer to get rid of
JsObjectSize() and put corrected logic into JsObjectIsEmpty().

Thinko in commit cf35346e8, so no need for back-patch.

Nikita Glukhov

Discussion: https://postgr.es/m/fbd1d566-bba0-a3de-d6d0-d3b1d7c24ff2@postgrespro.ru

7 years agoPrevent running pg_resetwal/pg_resetxlog against wrong-version data dirs.
Tom Lane [Mon, 29 May 2017 21:08:16 +0000 (17:08 -0400)]
Prevent running pg_resetwal/pg_resetxlog against wrong-version data dirs.

pg_resetwal (formerly pg_resetxlog) doesn't insist on finding a matching
version number in pg_control, and that seems like an important thing to
preserve since recovering from corrupt pg_control is a prime reason to
need to run it.  However, that means you can try to run it against a
data directory of a different major version, which is at best useless
and at worst disastrous.  So as to provide some protection against that
type of pilot error, inspect PG_VERSION at startup and refuse to do
anything if it doesn't match.  PG_VERSION is read-only after initdb,
so it's unlikely to get corrupted, and even if it were corrupted it would
be easy to fix by hand.

This hazard has been there all along, so back-patch to all supported
branches.

Michael Paquier, with some kibitzing by me

Discussion: https://postgr.es/m/f4b8eb91-b934-8a0d-b3cc-68f06e2279d1@enterprisedb.com

7 years agoAllow NumericOnly to be "+ FCONST".
Tom Lane [Mon, 29 May 2017 19:19:07 +0000 (15:19 -0400)]
Allow NumericOnly to be "+ FCONST".

The NumericOnly grammar production accepted ICONST, + ICONST, - ICONST,
FCONST, and - FCONST, but for some reason not + FCONST.  This led to
strange inconsistencies like

regression=# set random_page_cost = +4;
SET
regression=# set random_page_cost = 4000000000;
SET
regression=# set random_page_cost = +4000000000;
ERROR:  syntax error at or near "4000000000"

(because 4000000000 is too large to be an ICONST).  While there's
no actual functional reason to need to write a "+", if we allow
it for integers it seems like we should allow it for numerics too.

It's been like that forever, so back-patch to all supported branches.

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

7 years agoMore code review for get_qual_for_list().
Tom Lane [Mon, 29 May 2017 18:24:28 +0000 (14:24 -0400)]
More code review for get_qual_for_list().

Avoid trashing the input PartitionBoundSpec; while that might be safe for
current callers, it's certainly trouble waiting to happen.  In the same
vein, make sure that all of the result data structure is freshly palloc'd,
rather than some of it being pointers into the input data structures
(which we don't know the lifespans of).

Simplify the logic for tacking on IS NULL or IS NOT NULL conditions some
more; commit 85c2b9a15 left a lot on the table there.  And rearrange the
construction of the nodes into (what seems to me) a more logical order.

In passing, make sure that get_qual_for_range() also returns a freshly
palloc'd structure, since there's no value in having that guarantee for
only one kind of partitioning.  And improve some comments there.

Jeevan Ladhe, with further tweaking by me

Discussion: https://postgr.es/m/CAOgcT0MAcYoMs93W80iTUf_dP36=1mZQzeUk+nnwY_-qWDrCfw@mail.gmail.com

7 years agoFix typo in comment
Magnus Hagander [Mon, 29 May 2017 14:29:19 +0000 (16:29 +0200)]
Fix typo in comment

Masahiko Sawada

7 years agoFix reference to RFC specifying SCRAM.
Heikki Linnakangas [Mon, 29 May 2017 06:31:33 +0000 (09:31 +0300)]
Fix reference to RFC specifying SCRAM.

Noted by Peter Eisentraut

7 years agoCode review focused on new node types added by partitioning support.
Tom Lane [Mon, 29 May 2017 03:20:28 +0000 (23:20 -0400)]
Code review focused on new node types added by partitioning support.

Fix failure to check that we got a plain Const from const-simplification of
a coercion request.  This is the cause of bug #14666 from Tian Bing: there
is an int4 to money cast, but it's only stable not immutable (because of
dependence on lc_monetary), resulting in a FuncExpr that the code was
miserably unequipped to deal with, or indeed even to notice that it was
failing to deal with.  Add test cases around this coercion behavior.

In view of the above, sprinkle the code liberally with castNode() macros,
in hope of catching the next such bug a bit sooner.  Also, change some
functions that were randomly declared to take Node* to take more specific
pointer types.  And change some struct fields that were declared Node*
but could be given more specific types, allowing removal of assorted
explicit casts.

Place PARTITION_MAX_KEYS check a bit closer to the code it's protecting.
Likewise check only-one-key-for-list-partitioning restriction in a less
random place.

Avoid not-per-project-style usages like !strcmp(...).

Fix assorted failures to avoid scribbling on the input of parse
transformation.  I'm not sure how necessary this is, but it's entirely
silly for these functions to be expending cycles to avoid that and not
getting it right.

Add guards against partitioning on system columns.

Put backend/nodes/ support code into an order that matches handling
of these node types elsewhere.

Annotate the fact that somebody added location fields to PartitionBoundSpec
and PartitionRangeDatum but forgot to handle them in
outfuncs.c/readfuncs.c.  This is fairly harmless for production purposes
(since readfuncs.c would just substitute -1 anyway) but it's still bogus.
It's not worth forcing a post-beta1 initdb just to fix this, but if we
have another reason to force initdb before 10.0, we should go back and
clean this up.

Contrariwise, somebody added location fields to PartitionElem and
PartitionSpec but forgot to teach exprLocation() about them.

Consolidate duplicative code in transformPartitionBound().

Improve a couple of error messages.

Improve assorted commentary.

Re-pgindent the files touched by this patch; this affects a few comment
blocks that must have been added quite recently.

Report: https://postgr.es/m/20170524024550.29935.14396@wrigleys.postgresql.org

7 years agoFormat v10 release notes' commit references more like previous releases.
Tom Lane [Sun, 28 May 2017 20:42:22 +0000 (16:42 -0400)]
Format v10 release notes' commit references more like previous releases.

Left-justify these comments, remove committer names, remove SGML markup
that was randomly added to some of them.  Aside from being more consistent
with previous practice, this keeps the lines shorter than 80 characters,
improving readability in standard terminal windows.

7 years agoImprove v10 release notes' discussion of money operator changes.
Tom Lane [Sun, 28 May 2017 19:49:44 +0000 (15:49 -0400)]
Improve v10 release notes' discussion of money operator changes.

Mention the rounding behavioral change for money/int8.

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

7 years agoAvoid locale-dependent output in select_views regression test.
Tom Lane [Sun, 28 May 2017 18:52:18 +0000 (14:52 -0400)]
Avoid locale-dependent output in select_views regression test.

Use 'COLLATE "C"' to force locale-independent sorting of the iexit
view results in select_views.sql.  We aren't particularly interested
in the exact sorting behavior here, and this doesn't change the shape
of the generated plan, so it seems like a wash as far as the goals
of this test go.

This is in response to bug #14637 from Tomasz Kontusz.  It doesn't
fully resolve his problem, because he also saw some diffs in the
create_index test.  But other people have had issues with select_views
too, and this fix lets us drop the select_views_1.out variant expected
file altogether, which is a nice win from a maintenance standpoint.

Emre Hasegeli

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

7 years agoFix typo in pg_dump's support for dumping collations from pre-v10 servers.
Tom Lane [Fri, 26 May 2017 19:37:06 +0000 (15:37 -0400)]
Fix typo in pg_dump's support for dumping collations from pre-v10 servers.

Dunno what 'p' was supposed to mean, but since neither the code below
here nor pg_collation.h think it's valid, it must be a mistake.

Per report from Thomas Kellerer.

Discussion: https://postgr.es/m/og9q8f%24oes%241%40blaine.gmane.org

7 years agoMove autogenerated array types out of the way during ALTER ... RENAME.
Tom Lane [Fri, 26 May 2017 19:16:59 +0000 (15:16 -0400)]
Move autogenerated array types out of the way during ALTER ... RENAME.

Commit 9aa3c782c added code to allow CREATE TABLE/CREATE TYPE to not fail
when the desired type name conflicts with an autogenerated array type, by
dint of renaming the array type out of the way.  But I (tgl) overlooked
that the same case arises in ALTER TABLE/TYPE RENAME.  Fix that too.
Back-patch to all supported branches.

Report and patch by Vik Fearing, modified a bit by me

Discussion: https://postgr.es/m/0f4ade49-4f0b-a9a3-c120-7589f01d1eb8@2ndquadrant.com

7 years agoFix pg_dump to not emit invalid SQL for an empty operator class.
Tom Lane [Fri, 26 May 2017 16:51:05 +0000 (12:51 -0400)]
Fix pg_dump to not emit invalid SQL for an empty operator class.

If an operator class has no operators or functions, and doesn't need
a STORAGE clause, we emitted "CREATE OPERATOR CLASS ... AS ;" which
is syntactically invalid.  Fix by forcing a STORAGE clause to be
emitted anyway in this case.

(At some point we might consider changing the grammar to allow CREATE
OPERATOR CLASS without an opclass_item_list.  But probably we'd want to
omit the AS in that case, so that wouldn't fix this pg_dump issue anyway.)

It's been like this all along, so back-patch to all supported branches.

Daniel Gustafsson, tweaked by me to avoid a dangling-pointer bug

Discussion: https://postgr.es/m/D9E5FC64-7A37-4F3D-B946-7E4FB468F88A@yesql.se

7 years agoRemove docs mention of PGREALM variable
Magnus Hagander [Fri, 26 May 2017 14:58:15 +0000 (10:58 -0400)]
Remove docs mention of PGREALM variable

This variable was only used with Kerberos v4. That support was removed
in 2005, but we forgot to remove the documentation.

Noted by Shinichi Matsuda

7 years agoUpdate expected file
Alvaro Herrera [Thu, 25 May 2017 18:41:43 +0000 (14:41 -0400)]
Update expected file

Missed in ea3e310e712a.

7 years agoextended stats: Clarify behavior of omitting stat type clause
Alvaro Herrera [Thu, 25 May 2017 17:17:57 +0000 (13:17 -0400)]
extended stats: Clarify behavior of omitting stat type clause

Pointed out by Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1zGhK-nW10RAXhokcT3MM=YBg=j5LkG9RMDwmu3i0H0Og@mail.gmail.com

7 years agoFix message case
Alvaro Herrera [Thu, 25 May 2017 17:16:00 +0000 (13:16 -0400)]
Fix message case

7 years agoFix whitespace
Peter Eisentraut [Thu, 25 May 2017 15:17:09 +0000 (11:17 -0400)]
Fix whitespace

7 years agoAbort authentication if the client selected an invalid SASL mechanism.
Heikki Linnakangas [Thu, 25 May 2017 12:50:47 +0000 (08:50 -0400)]
Abort authentication if the client selected an invalid SASL mechanism.

Previously, the server would log an error, but then try to continue with
SCRAM-SHA-256 anyway.

Michael Paquier

Discussion: https://www.postgresql.org/message-id/CAB7nPqR0G5aF2_kc_LH29knVqwvmBc66TF5DicvpGVdke68nKw@mail.gmail.com

7 years agoFix table syncing with different column order
Peter Eisentraut [Thu, 18 May 2017 18:16:16 +0000 (14:16 -0400)]
Fix table syncing with different column order

Logical replication supports replicating between tables with different
column order.  But this failed for the initial table sync because of a
logic error in how the column list for the internal COPY command was
composed.  Fix that and also add a test.

Also fix a minor omission in the column name mapping cache.  When
creating the mapping list, it would not skip locally dropped columns.
So if a remote column had the same name as a locally dropped
column (...pg.dropped...), then the expected error would not occur.

7 years agoImprove logical replication worker log messages
Peter Eisentraut [Wed, 24 May 2017 22:56:21 +0000 (18:56 -0400)]
Improve logical replication worker log messages

Reduce some redundant messages to DEBUG1.  Be clearer about the
distinction between apply workers and table synchronization workers.
Add subscription and table name where possible.

Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
7 years agoCode review of get_qual_for_list.
Robert Haas [Wed, 24 May 2017 20:30:47 +0000 (16:30 -0400)]
Code review of get_qual_for_list.

We need not consider the case where both nulltest1 and nulltest2 are
NULL; the partition either accepts nulls or it does not.

Jeevan Ladhe.  I added an assertion.

7 years agoTighten checks for whitespace in functions that parse identifiers etc.
Tom Lane [Wed, 24 May 2017 19:28:34 +0000 (15:28 -0400)]
Tighten checks for whitespace in functions that parse identifiers etc.

This patch replaces isspace() calls with scanner_isspace() in functions
that are likely to be presented with non-ASCII input.  isspace() has
the small advantage that it will correctly recognize no-break space
in single-byte encodings (such as LATIN1); but it cannot work successfully
for any multibyte character, and depending on platform it might return
false positive results for some fragments of multibyte characters.  That's
disastrous for functions that are trying to discard whitespace between
valid strings, as noted in bug #14662 from Justin Muise.  Even treating
no-break space as whitespace is pretty questionable for the usages touched
here, because the core scanner would think it is an identifier character.

Affected functions are parse_ident(), parseNameAndArgTypes (underlying
regprocedurein() and siblings), SplitIdentifierString (used for parsing
GUCs and options that are qualified names or lists of names), and
SplitDirectoriesString (used for parsing GUCs that are lists of
directories).

All the functions adjusted here are parsing SQL identifiers and similar
constructs, so it's reasonable to insist that their definition of
whitespace match the core scanner.  So we can hope that this won't cause
many backwards-compatibility problems.  I've left alone isspace() calls
in places that aren't really expecting any non-ASCII input characters,
such as float8in().

Back-patch to all supported branches.

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

7 years agoUpdate URLs in pgindent source and README
Magnus Hagander [Tue, 23 May 2017 17:58:11 +0000 (13:58 -0400)]
Update URLs in pgindent source and README

Website and buildfarm is https, not http, and the ftp protocol will be
shut down shortly.

7 years agoVerify that the server constructed the SCRAM nonce correctly.
Heikki Linnakangas [Tue, 23 May 2017 09:55:19 +0000 (05:55 -0400)]
Verify that the server constructed the SCRAM nonce correctly.

The nonce consists of client and server nonces concatenated together. The
client checks the nonce contained the client nonce, but it would get fooled
if the server sent a truncated or even empty nonce.

Reported by Steven Fackler to security@postgresql.org. Neither me or Steven
are sure what harm a malicious server could do with this, but let's fix it.

7 years agoSynced ecpg's pg_type.h with the one used in the backend.
Michael Meskes [Tue, 23 May 2017 07:48:51 +0000 (09:48 +0200)]
Synced ecpg's pg_type.h with the one used in the backend.

Patch by Vinayak Pokale.

7 years agoFix typo in comment
Magnus Hagander [Mon, 22 May 2017 07:10:02 +0000 (09:10 +0200)]
Fix typo in comment

Author: Masahiko Sawada

7 years agoFix precision and rounding issues in money multiplication and division.
Tom Lane [Sun, 21 May 2017 17:05:16 +0000 (13:05 -0400)]
Fix precision and rounding issues in money multiplication and division.

The cash_div_intX functions applied rint() to the result of the division.
That's not merely useless (because the result is already an integer) but
it causes precision loss for values larger than 2^52 or so, because of
the forced conversion to float8.

On the other hand, the cash_mul_fltX functions neglected to apply rint() to
their multiplication results, thus possibly causing off-by-one outputs.

Per C standard, arithmetic between any integral value and a float value is
performed in float format.  Thus, cash_mul_flt4 and cash_div_flt4 produced
answers good to only about six digits, even when the float value is exact.
We can improve matters noticeably by widening the float inputs to double.
(It's tempting to consider using "long double" arithmetic if available,
but that's probably too much of a stretch for a back-patched fix.)

Also, document that cash_div_intX operators truncate rather than round.

Per bug #14663 from Richard Pistole.  Back-patch to all supported branches.

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

7 years agoFix contrib/sepgsql regression tests for partition NOT NULL change.
Tom Lane [Sun, 21 May 2017 15:46:04 +0000 (11:46 -0400)]
Fix contrib/sepgsql regression tests for partition NOT NULL change.

Commit 3ec76ff1f changed the partitioning logic to not install a forced
NOT NULL constraint on range partitioning columns.  This affects the
expected output for contrib/sepgsql, because there's no longer LOG
entries reporting allowance of such a constraint.  Per buildfarm.

7 years agoChange documentation references to PG website to use https: not http:
Tom Lane [Sun, 21 May 2017 01:50:47 +0000 (21:50 -0400)]
Change documentation references to PG website to use https: not http:

This is more secure, and saves a redirect since we no longer accept
plain HTTP connections on the website.

References in code comments should probably be updated too, but
that doesn't seem to need back-patching, whereas this does.

Also, in the 9.2 branch, remove suggestion that you can get the
source code via FTP, since that service will be shut down soon.

Daniel Gustafsson, with a few additional changes by me

Discussion: https://postgr.es/m/9A2C89A7-0BB8-41A8-B288-8B7BD09D7D44@yesql.se

7 years agoRethink flex flags for syncrep_scanner.l.
Tom Lane [Fri, 19 May 2017 22:05:20 +0000 (18:05 -0400)]
Rethink flex flags for syncrep_scanner.l.

Using flex's -i switch to achieve case-insensitivity is not a very safe
practice, because the scanner's behavior may then depend on the locale
that flex was invoked in.  In the particular example at hand, that's
not academic: the possible matches for "FIRST" will be different in a
Turkish locale than elsewhere.  Do it the hard way instead, as our
other scanners do.

Also, drop use of -b -CF -p, because this scanner is only used when
parsing the contents of a GUC variable.  That's not done often, and
the amount of text to be parsed can be expected to be trivial, so
prioritizing scanner speed over code size seems like quite the wrong
tradeoff.  Using flex's default optimization options reduces the
size of syncrep_gram.o by more than 50%.

The case-insensitivity problem is new in HEAD (cf commit 3901fd70c).
The poor choice of optimization flags exists also in 9.6, but it doesn't
seem important enough to back-patch.

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

7 years agopg_upgrade: Handle hash index upgrades more smoothly.
Robert Haas [Fri, 19 May 2017 20:49:38 +0000 (16:49 -0400)]
pg_upgrade: Handle hash index upgrades more smoothly.

Mark any old hash indexes as invalid so that they don't get used, and
create a script to run REINDEX on all of them.  Without this, we'd
still try to use any upgraded hash indexes, but it would fail.

Amit Kapila, reviewed by me.  Per a suggestion from Tom Lane.

Discussion: http://postgr.es/m/CAA4eK1Jidtagm7Q81q-WoegOVgkotv0OxvHOjFxcvFRP4X=mSw@mail.gmail.com

7 years agoFix mistake in error message
Peter Eisentraut [Fri, 19 May 2017 20:30:02 +0000 (16:30 -0400)]
Fix mistake in error message

Reported-by: tushar <tushar.ahuja@enterprisedb.com>
Author: Dilip Kumar <dilipbalaut@gmail.com>

7 years agolibpq: Try next host if one of them times out.
Robert Haas [Fri, 19 May 2017 20:19:51 +0000 (16:19 -0400)]
libpq: Try next host if one of them times out.

If one host in a multi-host connection string times out, move on to
the next specified host instead of giving up entirely.

Takayuki Tsunakawa, reviewed by Michael Paquier.  I added
a minor adjustment to the documentation.

Discussion: http://postgr.es/m/0A3221C70F24FB45833433255569204D1F6F42F5@G01JPEXMBYT05

7 years agoCapitalize SHOW when testing whether target_session_attrs=read-write.
Robert Haas [Fri, 19 May 2017 19:48:10 +0000 (15:48 -0400)]
Capitalize SHOW when testing whether target_session_attrs=read-write.

This makes it also work for replication connections.

Report and patch by Daisuke Higuchi.

Discussion: http://postgr.es/m/1803D792815FC24D871C00D17AE95905B1A34A@g01jpexmbkw24

7 years agoCopy partitioned_rels lists to avoid shared substructure.
Robert Haas [Fri, 19 May 2017 19:23:42 +0000 (15:23 -0400)]
Copy partitioned_rels lists to avoid shared substructure.

Otherwise, set_plan_refs() can get applied to the same list
multiple times through different references, leading to chaos.

Amit Langote, Dilip Kumar, and Robert Haas, reviewed by Ashutosh
Bapat.  Original report by Sveinn Sveinsson.

Discussion: http://postgr.es/m/20170517141151.1435.79890@wrigleys.postgresql.org

7 years agoFix misspelled struct tag.
Tom Lane [Fri, 19 May 2017 19:05:54 +0000 (15:05 -0400)]
Fix misspelled struct tag.

This was evidently intended to match the struct's typedef name,
but it didn't quite.  Noted while testing find_typedefs.

7 years agoFix corruption of tableElts list by MergeAttributes().
Robert Haas [Fri, 19 May 2017 19:02:16 +0000 (15:02 -0400)]
Fix corruption of tableElts list by MergeAttributes().

Since commit e7b3349a8ad7afaad565c573fbd65fb46af6abbe, MergeAttributes
destructively modifies the input List, to which the caller's
CreateStmt still points.  One may wonder whether this was already a
bug, but commit f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63 made things
noticeably worse by adding additional destructive modifications so
that the caller's List might, in the case of creation a partitioned
table, no longer even be structurally valid.  Restore the status quo
ante by assigning the return value of MergeAttributes back to
stmt->tableElts in the caller.

In most of the places where DefineRelation is called, it doesn't
matter what stmt->tableElts points to here or whether it's valid or
not, because the caller doesn't use the statement for anything after
DefineRelation returns anyway.  However, ProcessUtilitySlow passes it
to EventTriggerCollectSimpleCommand, and that function tries to invoke
copyObject on it.  If any of the CreateStmt's substructure is invalid
at that point, undefined behavior will result.

One might wonder whether this whole area needs further revision -
perhaps DefineRelation() ought not to be destructively modifying the
caller-provided CreateStmt at all.  However, that would be a behavior
change for any event triggers using C code to inspect the CreateStmt,
so for now, just fix the crash.

Report by Amit Langote, who provided a somewhat different patch for it.

Discussion: http://postgr.es/m/bf6a39a7-100a-74bd-1156-3c16a1429d88@lab.ntt.co.jp

7 years agoFix argument name differences
Peter Eisentraut [Fri, 19 May 2017 18:47:56 +0000 (14:47 -0400)]
Fix argument name differences

Different names were used between function declaration and definition.

7 years agodoc: remove duplicate PG 10 release notes entry
Bruce Momjian [Fri, 19 May 2017 16:16:53 +0000 (12:16 -0400)]
doc: remove duplicate PG 10 release notes entry

Reported-by: Daniel Gustafsson
7 years agodoc: fix PG 10 release notes with proper attribution and commit
Bruce Momjian [Fri, 19 May 2017 16:10:10 +0000 (12:10 -0400)]
doc: fix PG 10 release notes with proper attribution and commit

Fix for hot_standby=on change.

Reported-by: Huong Dangminh
Author: Huong Dangminh

7 years agoFix compilation with --with-bsd-auth.
Heikki Linnakangas [Fri, 19 May 2017 09:21:55 +0000 (12:21 +0300)]
Fix compilation with --with-bsd-auth.

Commit 8d3b9cce81 added extra arguments to the sendAuthRequest function,
but neglected this caller inside #ifdef USE_BSD_AUTH.

Per report from Pierre-Emmanuel AndrĂ©.

Discussion: https://www.postgresql.org/message-id/20170519090336.whzmjzrsap6ktbgg@digipea.digitick.local

7 years agodoc: Fix ALTER SUBSCRIPTION option syntax synopsis
Peter Eisentraut [Fri, 19 May 2017 01:37:57 +0000 (21:37 -0400)]
doc: Fix ALTER SUBSCRIPTION option syntax synopsis

Author: Masahiko Sawada <sawada.mshk@gmail.com>

7 years agoMake slab allocator work on platforms with MAXIMUM_ALIGNOF < sizeof(int).
Heikki Linnakangas [Thu, 18 May 2017 19:22:13 +0000 (22:22 +0300)]
Make slab allocator work on platforms with MAXIMUM_ALIGNOF < sizeof(int).

Notably, m68k only needs 2-byte alignment. Per report from Christoph Berg.

Discussion: https://www.postgresql.org/message-id/20170517193957.fwntkgi6epuso5l2@msg.df7cb.de

7 years agoDon't explicitly mark range partitioning columns NOT NULL.
Robert Haas [Thu, 18 May 2017 17:48:10 +0000 (13:48 -0400)]
Don't explicitly mark range partitioning columns NOT NULL.

This seemed like a good idea originally because there's no way to mark
a range partition as accepting NULL, but that now seems more like a
current limitation than something we want to lock down for all time.
For example, there's a proposal to add the notion of a default
partition which accepts all rows not otherwise routed, which directly
conflicts with the idea that a range-partitioned table should never
allow nulls anywhere.  So let's change this while we still can, by
putting the NOT NULL test into the partition constraint instead of
changing the column properties.

Amit Langote and Robert Haas, reviewed by Amit Kapila

Discussion: http://postgr.es/m/8e2dd63d-c6fb-bb74-3c2b-ed6d63629c9d@lab.ntt.co.jp

7 years agoFix typo in comment.
Heikki Linnakangas [Thu, 18 May 2017 07:33:16 +0000 (10:33 +0300)]
Fix typo in comment.

Daniel Gustafsson

7 years agopg_dump: Fix dumping of slot_name = NONE
Peter Eisentraut [Thu, 18 May 2017 01:19:14 +0000 (21:19 -0400)]
pg_dump: Fix dumping of slot_name = NONE

It previously wrote out slot_name = '', which was incorrect.

Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
7 years agoImprove CREATE SUBSCRIPTION option parsing
Peter Eisentraut [Thu, 18 May 2017 00:47:37 +0000 (20:47 -0400)]
Improve CREATE SUBSCRIPTION option parsing

When creating a subscription with slot_name = NONE, we failed to check
that also create_slot = false and enabled = false were set.  This
created an invalid subscription and could later lead to a crash if a
NULL slot name was accessed.  Add more checks around that for
robustness.

Reported-by: tushar <tushar.ahuja@enterprisedb.com>
7 years agoPost-PG 10 beta1 pgperltidy run
Bruce Momjian [Wed, 17 May 2017 23:01:23 +0000 (19:01 -0400)]
Post-PG 10 beta1 pgperltidy run

7 years agoPost-PG 10 beta1 pgindent run
Bruce Momjian [Wed, 17 May 2017 20:31:56 +0000 (16:31 -0400)]
Post-PG 10 beta1 pgindent run

perltidy run not included.

7 years agoUpdate typedefs list in prep. for post-PG10 beta1 pgindent run
Bruce Momjian [Wed, 17 May 2017 19:52:16 +0000 (15:52 -0400)]
Update typedefs list in prep. for post-PG10 beta1 pgindent run

7 years agoAdd download URL for perltidy version v20090616
Bruce Momjian [Wed, 17 May 2017 19:29:37 +0000 (15:29 -0400)]
Add download URL for perltidy version v20090616

7 years agoCode review for make_partition_op_expr.
Robert Haas [Wed, 17 May 2017 18:31:48 +0000 (14:31 -0400)]
Code review for make_partition_op_expr.

It's better to use the actual keynum here rather than 0, because
someday someone might try to make list partitioning work with
multiple partitioning columns.

Jeevan Ladhe

Discussion: http://postgr.es/m/CAOgcT0M6-mx+dSX47JGJuJP1CKr4XssBFVmKNETt0OZYWpFr+w@mail.gmail.com

7 years agoRevert changes to pg_basebackup and pg_waldump usage() code.
Tom Lane [Wed, 17 May 2017 17:04:03 +0000 (13:04 -0400)]
Revert changes to pg_basebackup and pg_waldump usage() code.

Partially revert commit c079673dcb7f210617c9fc1470e6bf166d8a2971.
There were complaints that splitting switch descriptions would
complicate translation efforts.  There are probably ways to resolve
the formatting problem without doing that, but undo it while we're
discussing.