]> granicus.if.org Git - postgresql/log
postgresql
6 years agoAdd configure infrastructure (--with-llvm) to enable LLVM support.
Andres Freund [Wed, 21 Mar 2018 00:26:25 +0000 (17:26 -0700)]
Add configure infrastructure (--with-llvm) to enable LLVM support.

LLVM will be used for *optional* Just-in-time compilation
support. This commit just adds the configure infrastructure that
detects LLVM.

No documentation has been added for the --with-llvm flag, that'll be
added after the actual supporting code has been added.

Author: Andres Freund
Discussion: https://postgr.es/m/20170901064131.tazjxwus3k2w3ybh@alap3.anarazel.de

6 years agoAdd C++ support to configure.
Andres Freund [Tue, 20 Mar 2018 22:41:15 +0000 (15:41 -0700)]
Add C++ support to configure.

This is an optional dependency. It'll be used for the upcoming LLVM
based just in time compilation support, which needs to wrap a few LLVM
C++ APIs so they're accessible from C..

For now test for C++ compilers unconditionally, without failing if not
present, to ensure wide buildfarm coverage. If we're bothered by the
additional test times (which are quite short) or verbosity, we can
later make the tests conditional on --with-llvm.

Author: Andres Freund
Discussion: https://postgr.es/m/20170901064131.tazjxwus3k2w3ybh@alap3.anarazel.de

6 years agoAttempt to fix build with unusual OpenSSL versions
Peter Eisentraut [Tue, 20 Mar 2018 20:44:52 +0000 (16:44 -0400)]
Attempt to fix build with unusual OpenSSL versions

Since e3bdb2d92600ed45bd46aaf48309a436a9628218, libpq failed to build on
some platforms because they did not have SSL_clear_options().  Although
mainline OpenSSL introduced SSL_clear_options() after
SSL_OP_NO_COMPRESSION, so the code should have built fine, at least an
old NetBSD version (build farm "coypu" NetBSD 5.1 gcc 4.1.3 PR-20080704
powerpc) has SSL_OP_NO_COMPRESSION but no SSL_clear_options().

So add a configure check for SSL_clear_options().  If we don't find it,
skip the call.  That means on such a platform one cannot *enable* SSL
compression if the built-in default is off, but that seems an unlikely
combination anyway and not very interesting in practice.

6 years agoAdd PGAC_PROG_VARCC_VARFLAGS_OPT autoconf macro.
Andres Freund [Tue, 20 Mar 2018 19:58:08 +0000 (12:58 -0700)]
Add PGAC_PROG_VARCC_VARFLAGS_OPT autoconf macro.

The new macro allows to test flags for different compilers and to
store them in different CFLAG like variables.  The existing
PGAC_PROG_CC_CFLAGS_OPT and PGAC_PROG_CC_VAR_OPT are changed to be
just wrappers around the new function.

This'll be used by the upcoming LLVM support, to separately detect
capabilities used by clang, when generating bitcode.

Author: Andres Freund
Discussion: https://postgr.es/m/20170901064131.tazjxwus3k2w3ybh@alap3.anarazel.de

6 years agoMake configure check for a couple more Perl modules for --enable-tap-tests.
Tom Lane [Tue, 20 Mar 2018 19:16:16 +0000 (15:16 -0400)]
Make configure check for a couple more Perl modules for --enable-tap-tests.

Red Hat's notion of a basic Perl installation doesn't include Test::More
or Time::HiRes, and reportedly some Debian installs also omit Time::HiRes.
Check for those during configure to spare the user the pain of digging
through check-world output to find out what went wrong.  While we're at it,
we should also check the version of Test::More, since TestLib.pm requires
at least 0.87.

In principle this could be back-patched, but it's probably not necessary.

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

6 years agoDon't pass the grouping target around unnecessarily.
Robert Haas [Tue, 20 Mar 2018 15:33:44 +0000 (11:33 -0400)]
Don't pass the grouping target around unnecessarily.

Since commit 4f15e5d09de276fb77326be5567dd9796008ca2e made grouped_rel
set reltarget, a variety of other functions can just get it from
grouped_rel instead of having to pass it around explicitly.  Simplify
accordingly.

Patch by me, reviewed by Ashutosh Bapat.

Discussion: http://postgr.es/m/CA+TgmoZ+ZJTVad-=vEq393N99KTooxv9k7M+z73qnTAqkb49BQ@mail.gmail.com

6 years agoDoc: typo fix, "PG_" should be "TG_" here.
Tom Lane [Tue, 20 Mar 2018 15:34:12 +0000 (11:34 -0400)]
Doc: typo fix, "PG_" should be "TG_" here.

Too much PG on the brain in commit 769159fd3, evidently.
Noted by marcelhuberfoo@gmail.com.

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

6 years agoDetermine grouping strategies in create_grouping_paths.
Robert Haas [Tue, 20 Mar 2018 15:31:06 +0000 (11:31 -0400)]
Determine grouping strategies in create_grouping_paths.

Partition-wise aggregate will call create_ordinary_grouping_paths
multiple times and we don't want to redo this work every time; have
the caller do it instead and pass the details down.

Patch by me, reviewed by Ashutosh Bapat.

Discussion: http://postgr.es/m/CA+TgmoY7VYYn9a7YHj1nJL6zj6BkHmt4K-un9LRmXkyqRZyynA@mail.gmail.com

6 years agoDefer creation of partially-grouped relation until it's needed.
Robert Haas [Tue, 20 Mar 2018 15:18:04 +0000 (11:18 -0400)]
Defer creation of partially-grouped relation until it's needed.

This avoids unnecessarily creating a RelOptInfo for which we have no
actual need.  This idea is from Ashutosh Bapat, who wrote a very
different patch to accomplish a similar goal.  It will be more
important if and when we get partition-wise aggregate, since then
there could be many partially grouped relations all of which could
potentially be unnecessary.  In passing, this sets the grouping
relation's reltarget, which wasn't done previously but makes things
simpler for this refactoring.

Along the way, adjust things so that add_paths_to_partial_grouping_rel,
now renamed create_partial_grouping_paths, does not perform the Gather
or Gather Merge steps to generate non-partial paths from partial
paths; have the caller do it instead.  This is again for the
convenience of partition-wise aggregate, which wants to inject
additional partial paths are created and before we decide which ones
to Gather/Gather Merge.  This might seem like a separate change, but
it's actually pretty closely entangled; I couldn't really see much
value in separating it and having to change some things twice.

Patch by me, reviewed by Ashutosh Bapat.

Discussion: http://postgr.es/m/CA+TgmoZ+ZJTVad-=vEq393N99KTooxv9k7M+z73qnTAqkb49BQ@mail.gmail.com

6 years agoFix CommandCounterIncrement in partition-related DDL
Alvaro Herrera [Tue, 20 Mar 2018 14:19:41 +0000 (11:19 -0300)]
Fix CommandCounterIncrement in partition-related DDL

It makes sense to do the CCIs in the places that do catalog updates,
rather than before the places that error out because the former ones
fail to do it.  In particular, it looks like StorePartitionBound() and
IndexSetParentIndex() ought to make their own CCIs.

Per review comments from Peter Eisentraut for row-level triggers on
partitioned tables.

Discussion: https://postgr.es/m/20171229225319.ajltgss2ojkfd3kp@alvherre.pgsql

6 years agoPrevent query-lifespan memory leakage of SP-GiST traversal values.
Tom Lane [Tue, 20 Mar 2018 03:59:17 +0000 (23:59 -0400)]
Prevent query-lifespan memory leakage of SP-GiST traversal values.

The original coding of the SP-GiST scan traversalValue feature (commit
ccd6eb49a) arranged for traversal values to be stored in the query's main
executor context.  That's fine if there's only one index scan per query,
but if there are many, we have a memory leak as successive scans create
new traversal values.  Fix it by creating a separate memory context for
traversal values, which we can reset during spgrescan().  Back-patch
to 9.6 where this code was introduced.

In principle, adding the traversalCxt field to SpGistScanOpaqueData
creates an ABI break in the back branches.  But I (tgl) have little
sympathy for extensions including spgist_private.h, so I'm not very
worried about that.  Alternatively we could stick the new field at the
end of the struct in back branches, but that has its own downsides.

Anton Dignös, reviewed by Alexander Kuzmenkov

Discussion: https://postgr.es/m/CALNdv1jb6y2Te-m8xHLxLX12RsBmZJ1f4hESX7J0HjgyOhA9eA@mail.gmail.com

6 years agoAdd missing break
Peter Eisentraut [Mon, 19 Mar 2018 23:45:25 +0000 (19:45 -0400)]
Add missing break

6 years agoFix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY.
Tom Lane [Mon, 19 Mar 2018 22:49:53 +0000 (18:49 -0400)]
Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY.

refresh_by_match_merge() has some issues in the way it builds a SQL
query to construct the "diff" table:

1. It doesn't require the selected unique index(es) to be indimmediate.
2. It doesn't pay attention to the particular equality semantics enforced
by a given index, but just assumes that they must be those of the column
datatype's default btree opclass.
3. It doesn't check that the indexes are btrees.
4. It's insufficiently careful to ensure that the parser will pick the
intended operator when parsing the query.  (This would have been a
security bug before CVE-2018-1058.)
5. It's not careful about indexes on system columns.

The way to fix #4 is to make use of the existing code in ri_triggers.c
for generating an arbitrary binary operator clause.  I chose to move
that to ruleutils.c, since that seems a more reasonable place to be
exporting such functionality from than ri_triggers.c.

While #1, #3, and #5 are just latent given existing feature restrictions,
and #2 doesn't arise in the core system for lack of alternate opclasses
with different equality behaviors, #4 seems like an issue worth
back-patching.  That's the bulk of the change anyway, so just back-patch
the whole thing to 9.4 where this code was introduced.

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

6 years agoDon't use an Msys virtual path to create a tablespace
Andrew Dunstan [Mon, 19 Mar 2018 21:55:36 +0000 (08:25 +1030)]
Don't use an Msys virtual path to create a tablespace

The new unlogged_reinit recovery tests create a new tablespace using
TestLib.pm's tempdir. However, on msys that function returns a virtual
path that isn't understood by Postgres. Here we add a new function to
TestLib.pm to turn such a path into a real path on the underlying file
system, and use it in the new test to create the tablespace. The new
function is essentially a NOOP everywhere but msys.

6 years agoFix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY.
Tom Lane [Mon, 19 Mar 2018 21:23:07 +0000 (17:23 -0400)]
Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY.

Jeff Janes discovered that commit 7ca25b7de made one of the queries run by
REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly.  The root cause is
bad cardinality estimation for correlated quals, but a principled solution
to that problem is some way off, especially since the planner lacks any
statistics about whole-row variables.  Moreover, in non-error cases this
query produces no rows, meaning it must be run to completion; but use of
LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan,
exactly not what we want.  Remove the LIMIT clause, and instead rely on
the count parameter we pass to SPI_execute() to prevent excess work if the
query does return some rows.

While we've heard no field reports of planner misbehavior with this query,
it could be that people are having performance issues that haven't reached
the level of pain needed to cause a bug report.  In any case, that LIMIT
clause can't possibly do anything helpful with any existing version of the
planner, and it demonstrably can cause bad choices in some cases, so
back-patch to 9.4 where the code was introduced.

Thomas Munro

Discussion: https://postgr.es/m/CAMkU=1z-JoGymHneGHar1cru4F1XDfHqJDzxP_CtK5cL3DOfmg@mail.gmail.com

6 years agoRemove unnecessary members from ModifyTableState and ExecInsert
Alvaro Herrera [Mon, 19 Mar 2018 21:09:43 +0000 (18:09 -0300)]
Remove unnecessary members from ModifyTableState and ExecInsert

These values can be obtained from the ModifyTable node which is already
a part of both the ModifyTableState and ExecInsert.

Author: Álvaro Herrera, Amit Langote
Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/20180316151303.rml2p5wffn3o6qy6@alvherre.pgsql

6 years agoExpand comment a little bit
Alvaro Herrera [Mon, 19 Mar 2018 21:01:14 +0000 (18:01 -0300)]
Expand comment a little bit

The previous commit removed a comment that was a bit more verbose than
its replacement.

6 years agoFix state reversal after partition tuple routing
Alvaro Herrera [Mon, 19 Mar 2018 20:43:57 +0000 (17:43 -0300)]
Fix state reversal after partition tuple routing

We make some changes to ModifyTableState and the EState it uses whenever
we route tuples to partitions; but we weren't restoring properly in all
cases, possibly causing crashes when partitions with different tuple
descriptors are targeted by tuples inserted in the same command.
Refactor some code, creating ExecPrepareTupleRouting, to encapsulate the
needed state changing logic, and have it invoked one level above its
current place (ie. put it in ExecModifyTable instead of ExecInsert);
this makes it all more readable.

Add a test case to exercise this.

We don't support having views as partitions; and since only views can
have INSTEAD OF triggers, there is no point in testing for INSTEAD OF
when processing insertions into a partitioned table.  Remove code that
appears to support this (but which is actually never relevant.)

In passing, fix location of some very confusing comments in
ModifyTableState.

Reported-by: Amit Langote
Author: Etsuro Fujita, Amit Langote
Discussion: https://postgr/es/m/0473bf5c-57b1-f1f7-3d58-455c2230bc5f@lab.ntt.co.jp

6 years agoGenerate a separate upper relation for each stage of setop planning.
Robert Haas [Mon, 19 Mar 2018 15:55:38 +0000 (11:55 -0400)]
Generate a separate upper relation for each stage of setop planning.

Commit 3fc6e2d7f5b652b417fa6937c34de2438d60fa9f made setop planning
stages return paths rather than plans, but all such paths were loosely
associated with a single RelOptInfo, and only the final path was added
to the RelOptInfo.  Even at the time, it was foreseen that this should
be changed, because there is otherwise no good way for a single stage
of setop planning to return multiple paths.  With this patch, each
stage of set operation planning now creates a separate RelOptInfo;
these are distinguished by using appropriate relid sets.  Note that
this patch does nothing whatsoever about actually returning multiple
paths for the same set operation; it just makes it possible for a
future patch to do so.

Along the way, adjust things so that create_upper_paths_hook is called
for each of these new RelOptInfos rather than just once, since that
might be useful to extensions using that hook.  It might be a good
to provide an FDW API here as well, but I didn't try to do that for
now.

Patch by me, reviewed and tested by Ashutosh Bapat and Rajkumar
Raghuwanshi.

Discussion: http://postgr.es/m/CA+TgmoaLRAOqHmMZx=ESM3VDEPceg+-XXZsRXQ8GtFJO_zbMSw@mail.gmail.com

6 years agoRewrite recurse_union_children to iterate, rather than recurse.
Robert Haas [Mon, 19 Mar 2018 15:54:56 +0000 (11:54 -0400)]
Rewrite recurse_union_children to iterate, rather than recurse.

Also, rename it to plan_union_chidren, so the old name wasn't
very descriptive.  This results in a small net reduction in code,
seems at least to me to be easier to understand, and saves
space on the process stack.

Patch by me, reviewed and tested by Ashutosh Bapat and Rajkumar
Raghuwanshi.

Discussion: http://postgr.es/m/CA+TgmoaLRAOqHmMZx=ESM3VDEPceg+-XXZsRXQ8GtFJO_zbMSw@mail.gmail.com

6 years agoFix typo in comment
Magnus Hagander [Mon, 19 Mar 2018 09:45:44 +0000 (10:45 +0100)]
Fix typo in comment

Author: Daniel Gustafsson <daniel@yesql.se>

6 years agoDoc: note that statement-level view triggers require an INSTEAD OF trigger.
Tom Lane [Sun, 18 Mar 2018 19:10:28 +0000 (15:10 -0400)]
Doc: note that statement-level view triggers require an INSTEAD OF trigger.

If a view lacks an INSTEAD OF trigger, DML on it can only work by rewriting
the command into a command on the underlying base table(s).  Then we will
fire triggers attached to those table(s), not those for the view.  This
seems appropriate from a consistency standpoint, but nowhere was the
behavior explicitly documented, so let's do that.

There was some discussion of throwing an error or warning if a statement
trigger is created on a view without creating a row INSTEAD OF trigger.
But a simple implementation of that would result in dump/restore ordering
hazards.  Given that it's been like this all along, and we hadn't heard
a complaint till now, a documentation improvement seems sufficient.

Per bug #15106 from Pu Qun.  Back-patch to all supported branches.

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

6 years agoFix pg_recvlogical for pre-10 versions
Magnus Hagander [Sun, 18 Mar 2018 12:08:25 +0000 (13:08 +0100)]
Fix pg_recvlogical for pre-10 versions

In e170b8c8, protection against modified search_path was added. However,
PostgreSQL versions prior to 10 does not accept SQL commands over a
replication connection, so the protection would generate a syntax error.

Since we cannot run SQL commands on it, we are also not vulnerable to
the issue that e170b8c8 fixes, so we can just skip this command for
older versions.

Author: Michael Paquier <michael@paquier.xyz>

6 years agoFix overflow handling in plpgsql's integer FOR loops.
Tom Lane [Sat, 17 Mar 2018 19:38:15 +0000 (15:38 -0400)]
Fix overflow handling in plpgsql's integer FOR loops.

The test to exit the loop if the integer control value would overflow
an int32 turns out not to work on some ICC versions, as it's dependent
on the assumption that the compiler will execute the code as written
rather than "optimize" it.  ICC lacks any equivalent of gcc's -fwrapv
switch, so it was optimizing on the assumption of no integer overflow,
and that breaks this.  Rewrite into a form that in fact does not
do any overflowing computations.

Per Tomas Vondra and buildfarm member fulmar.  It's been like this
for a long time, although it was not till we added a regression test
case covering the behavior (in commit dd2243f2a) that the problem
became apparent.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/50562fdc-0876-9843-c883-15b8566c7511@2ndquadrant.com

6 years agoFix WHERE CURRENT OF when the referenced cursor uses an index-only scan.
Tom Lane [Sat, 17 Mar 2018 18:59:31 +0000 (14:59 -0400)]
Fix WHERE CURRENT OF when the referenced cursor uses an index-only scan.

"UPDATE/DELETE WHERE CURRENT OF cursor_name" failed, with an error message
like "cannot extract system attribute from virtual tuple", if the cursor
was using a index-only scan for the target table.  Fix it by digging the
current TID out of the indexscan state.

It seems likely that the same failure could occur for CustomScan plans
and perhaps some FDW plan types, so that leaving this to be treated as an
internal error with an obscure message isn't as good an idea as it first
seemed.  Hence, add a bit of heaptuple.c infrastructure to let us deliver
a more on-topic message.  I chose to make the message match what you get
for the case where execCurrentOf can't identify the target scan node at
all, "cursor "foo" is not a simply updatable scan of table "bar"".
Perhaps it should be different, but we can always adjust that later.

In the future, it might be nice to provide hooks that would let custom
scan providers and/or FDWs deal with this in other ways; but that's
not a suitable topic for a back-patchable bug fix.

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

Yugo Nagata and Tom Lane

Discussion: https://postgr.es/m/20180201013349.937dfc5f.nagata@sraoss.co.jp

6 years agoFix closing of incorrectly named cursor.
Michael Meskes [Sat, 17 Mar 2018 16:24:32 +0000 (17:24 +0100)]
Fix closing of incorrectly named cursor.

Patch by "Shinoda, Noriyoshi" <noriyoshi.shinoda@hpe.com>

6 years agoSet libpq sslcompression to off by default
Peter Eisentraut [Sat, 17 Mar 2018 12:56:50 +0000 (08:56 -0400)]
Set libpq sslcompression to off by default

Since SSL compression is no longer recommended, turn the default in
libpq from on to off.

OpenSSL 1.1.0 and many distribution packages already turn compression
off by default, so such a server won't accept compression anyway.  So
this will mainly affect users of older OpenSSL installations.

Also update the documentation to make clear that this setting is no
longer recommended.

Discussion: https://www.postgresql.org/message-id/flat/595cf3b1-4ffe-7f05-6f72-f72b7afa7993%402ndquadrant.com

6 years agoAdd ssl_passphrase_command setting
Peter Eisentraut [Mon, 26 Feb 2018 18:28:38 +0000 (13:28 -0500)]
Add ssl_passphrase_command setting

This allows specifying an external command for prompting for or
otherwise obtaining passphrases for SSL key files.  This is useful
because in many cases there is no TTY easily available during service
startup.

Also add a setting ssl_passphrase_command_supports_reload, which allows
supporting SSL configuration reload even if SSL files need passphrases.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
6 years agoAdd 'unit' parameter to ExplainProperty{Integer,Float}.
Andres Freund [Sat, 17 Mar 2018 06:13:12 +0000 (23:13 -0700)]
Add 'unit' parameter to ExplainProperty{Integer,Float}.

This allows to deduplicate some existing code, but mainly avoids some
duplication in upcoming commits.

In passing, fix variable names indicating wrong unit (seconds instead
of ms).

Author: Andres Freund
Discussion: https://postgr.es/m/20180314002740.cah3mdsonz5mxney@alap3.anarazel.de

6 years agoMake ExplainPropertyInteger accept 64bit input, remove *Long variant.
Andres Freund [Sat, 17 Mar 2018 06:13:12 +0000 (23:13 -0700)]
Make ExplainPropertyInteger accept 64bit input, remove *Long variant.

'long' is not useful type across platforms, as it's 32bit on 32 bit
platforms, and even on some 64bit platforms (e.g. windows) it's still
only 32bits wide.

As ExplainPropertyInteger should never be performance critical, change
it to accept a 64bit argument and remove ExplainPropertyLong.

Author: Andres Freund
Discussion: https://postgr.es/m/20180314164832.n56wt7zcbpzi6zxe@alap3.anarazel.de

6 years agoFix query-lifespan memory leakage in repeatedly executed hash joins.
Tom Lane [Fri, 16 Mar 2018 20:03:45 +0000 (16:03 -0400)]
Fix query-lifespan memory leakage in repeatedly executed hash joins.

ExecHashTableCreate allocated some memory that wasn't freed by
ExecHashTableDestroy, specifically the per-hash-key function information.
That's not a huge amount of data, but if one runs a query that repeats
a hash join enough times, it builds up.  Fix by arranging for the data
in question to be kept in the hashtable's hashCxt instead of leaving it
"loose" in the query-lifespan executor context.  (This ensures that we'll
also clean up anything that the hash functions allocate in fn_mcxt.)

Per report from Amit Khandekar.  It's been like this forever, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/CAJ3gD9cFofAWGvcxLOxDHC=B0hjtW8yGmUsF2hdGh97CM38=7g@mail.gmail.com

6 years agoDoc: explicitly point out that enum values can't be dropped.
Tom Lane [Fri, 16 Mar 2018 17:44:34 +0000 (13:44 -0400)]
Doc: explicitly point out that enum values can't be dropped.

This was not stated in so many words anywhere.  Document it to make
clear that it's a design limitation and not just an oversight or
documentation omission.

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

6 years agoChange transaction state debug strings to match enum symbols
Peter Eisentraut [Sat, 17 Feb 2018 15:19:21 +0000 (10:19 -0500)]
Change transaction state debug strings to match enum symbols

In some cases, these were different for no apparent reason, making
debugging unnecessarily mysterious.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
6 years agoImprove savepoint error messages
Peter Eisentraut [Sun, 18 Feb 2018 01:29:27 +0000 (20:29 -0500)]
Improve savepoint error messages

Include the savepoint name in the error message and rephrase it a bit to
match common style.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
6 years agoSimplify parse representation of savepoint commands
Peter Eisentraut [Sat, 17 Feb 2018 01:57:06 +0000 (20:57 -0500)]
Simplify parse representation of savepoint commands

Instead of embedding the savepoint name in a list and then requiring
complex code to unpack it, just add another struct field to store it
directly.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
6 years agoRename TransactionChain functions
Peter Eisentraut [Sat, 17 Feb 2018 01:44:15 +0000 (20:44 -0500)]
Rename TransactionChain functions

We call this thing a "transaction block" everywhere except in a few
functions, where it is mysteriously called a "transaction chain".  In
the SQL standard, a transaction chain is something different.  So rename
these functions to match the common terminology.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
6 years agoUpdate function comments
Peter Eisentraut [Sat, 17 Feb 2018 01:29:20 +0000 (20:29 -0500)]
Update function comments

After a6542a4b6870a019cd952d055d2e7af2da2fe102, some function comments
were misplaced.  Fix that.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
6 years agoMop-up for letting VOID-returning SQL functions end with a SELECT.
Tom Lane [Fri, 16 Mar 2018 16:48:13 +0000 (12:48 -0400)]
Mop-up for letting VOID-returning SQL functions end with a SELECT.

Part of the intent in commit fd1a421fe was to allow SQL functions that are
declared to return VOID to contain anything, including an unrelated final
SELECT, the same as SQL-language procedures can.  However, the planner's
inlining logic didn't get that memo.  Fix it, and add some regression tests
covering this area, since evidently we had none.

In passing, clean up some typos in comments in create_function_3.sql,
and get rid of its none-too-safe assumption that DROP CASCADE notice
output is immutably ordered.

Per report from Prabhat Sahu.

Discussion: https://postgr.es/m/CANEvxPqxAj6nNHVcaXxpTeEFPmh24Whu+23emgjiuKrhJSct0A@mail.gmail.com

6 years agoFix msvc/ecpg_regression.proj for recent ECPG test additions.
Tom Lane [Fri, 16 Mar 2018 02:36:19 +0000 (22:36 -0400)]
Fix msvc/ecpg_regression.proj for recent ECPG test additions.

Commit 3b7ab4380 added some tests that require ecpg to be given the
new "-C ORACLE" switch.  Teach the MSVC build infrastructure about
that.

Michael Paquier

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

6 years agoClean up duplicate table and function names in regression tests.
Tom Lane [Thu, 15 Mar 2018 21:08:51 +0000 (17:08 -0400)]
Clean up duplicate table and function names in regression tests.

Many of the objects we create during the regression tests are put in the
public schema, so that using the same names in different regression tests
creates a hazard of test failures if any two such scripts run concurrently.
This patch cleans up a bunch of latent hazards of that sort, as well as two
live hazards.

The current situation in this regard is far worse than it was a year or two
back, because practically all of the partitioning-related test cases have
reused table names with enthusiasm.  I despaired of cleaning up that mess
within the five most-affected tests (create_table, alter_table, insert,
update, inherit); fortunately those don't run concurrently.

Other than partitioning problems, most of the issues boil down to using
names like "foo", "bar", "tmp", etc, without thought for the fact that
other test scripts might use similar names concurrently.  I've made an
effort to make all such names more specific.

One of the live hazards was that commit 7421f4b8 caused with.sql to
create a table named "test", conflicting with a similarly-named table
in alter_table.sql; this was exposed in the buildfarm recently.
The other one was that join.sql and transactions.sql both create tables
named "foo" and "bar"; but join.sql's uses of those names date back
only to December or so.

Since commit 7421f4b8 was back-patched to v10, back-patch a minimal
fix for that problem.  The rest of this is just future-proofing.

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

6 years agoSplit create_grouping_paths into degenerate and non-degenerate cases.
Robert Haas [Thu, 15 Mar 2018 18:43:58 +0000 (14:43 -0400)]
Split create_grouping_paths into degenerate and non-degenerate cases.

There's no functional change here, or at least I hope there isn't,
just code rearrangement.  The rearrangement is motivated by
partition-wise aggregate, which doesn't need to consider the
degenerate case but wants to reuse the logic for the ordinary case.

Based loosely on a patch from Ashutosh Bapat and Jeevan Chalke, but I
whacked it around pretty heavily. The larger patch series of which
this patch is a part was also reviewed and tested by Antonin Houska,
Rajkumar Raghuwanshi, David Rowley, Dilip Kumar, Konstantin Knizhnik,
Pascal Legrand, Rafia Sabih, and me.

Discussion: http://postgr.es/m/CAFjFpRewpqCmVkwvq6qrRjmbMDpN0CZvRRzjd8UvncczA3Oz1Q@mail.gmail.com

6 years agotest_ddl_deparse: rename matview
Alvaro Herrera [Thu, 15 Mar 2018 18:14:15 +0000 (15:14 -0300)]
test_ddl_deparse: rename matview

Should have done this in e69f5e0efca ...
Per note from Tom Lane.

6 years agoClean up duplicate role and schema names in regression tests.
Tom Lane [Thu, 15 Mar 2018 18:00:31 +0000 (14:00 -0400)]
Clean up duplicate role and schema names in regression tests.

Since these names are global, using the same ones in different regression
tests creates a hazard of test failures if any two such scripts run
concurrently.  Let's establish a policy of not doing that.  In the cases
where a conflict existed, I chose to rename both sides: in principle one
script or the other could've been left in possession of the common name,
but that seems to just invite more trouble of the same sort.

There are a number of places where scripts are using names that seem
unduly generic, but in the absence of actual conflicts I left them alone.

In addition, fix insert.sql's use of "someone_else" as a role name.
That's a flat out violation of longstanding project policy, so back-patch
that change to v10 where the usage appeared.  The rest of this is just
future-proofing, as no two of these scripts are actually run concurrently
in the existing parallel_schedule.

Conflicts of schema-qualified names also exist, but will be dealt with
separately.

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

6 years agoFix more format truncation issues
Peter Eisentraut [Thu, 15 Mar 2018 15:10:41 +0000 (11:10 -0400)]
Fix more format truncation issues

Fix the warnings created by the compiler warning options
-Wformat-overflow=2 -Wformat-truncation=2, supported since GCC 7.  This
is a more aggressive variant of the fixes in
6275f5d28a1577563f53f2171689d4f890a46881, which GCC 7 warned about by
default.

The issues are all harmless, but some dubious coding patterns are
cleaned up.

One issue that is of external interest is that BGW_MAXLEN is increased
from 64 to 96.  Apparently, the old value would cause the bgw_name of
logical replication workers to be truncated in some circumstances.

But this doesn't actually add those warning options.  It appears that
the warnings depend a bit on compilation and optimization options, so it
would be annoying to have to keep up with that.  This is more of a
once-in-a-while cleanup.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoPass additional arguments to a couple of grouping-related functions.
Robert Haas [Thu, 15 Mar 2018 15:33:52 +0000 (11:33 -0400)]
Pass additional arguments to a couple of grouping-related functions.

get_number_of_groups() and make_partial_grouping_target() currently
fish information directly out of the PlannerInfo; in the former case,
the target list, and in the latter case, the HAVING qual.  This works
fine if there's only one grouping relation, but if the pending patch
for partition-wise aggregate gets committed, we'll have multiple
grouping relations and must therefore use appropriately translated
versions of these values for each one.  To make that simpler, pass the
values to be used as arguments.

Jeevan Chalke.  The larger patch series of which this patch is a part
was also reviewed and tested by Antonin Houska, Rajkumar Raghuwanshi,
David Rowley, Dilip Kumar, Konstantin Knizhnik, Pascal Legrand, Rafia
Sabih, and me.

Discussion: http://postgr.es/m/CAM2+6=UqFnFUypOvLdm5TgC+2M=-E0Q7_LOh0VDFFzmk2BBPzQ@mail.gmail.com
Discussion: http://postgr.es/m/CAM2+6=W+L=C4yBqMrgrfTfNtbtmr4T53-hZhwbA2kvbZ9VMrrw@mail.gmail.com

6 years agorestrict -> pg_restrict
Alvaro Herrera [Thu, 15 Mar 2018 13:02:51 +0000 (10:02 -0300)]
restrict -> pg_restrict

So that it works on MSVC, too.

Author: Michaël Paquier
Discussion: https://postgr.es/m/29889.1520968202@sss.pgh.pa.us

6 years agotest_ddl_deparse: Don't use pg_class as source for a matview
Alvaro Herrera [Thu, 15 Mar 2018 12:51:12 +0000 (09:51 -0300)]
test_ddl_deparse: Don't use pg_class as source for a matview

Doing so causes a pg_upgrade of a database containing these objects to
fail whenever pg_class changes.  And it's pointless anyway: we have more
interesting tables anyhow.

Discussion: https://postgr.es/m/CAD5tBc+s8pW9WvH2+_z=B4x95FD4QuzZKcaMpff_9H4rS0VU1A@mail.gmail.com

6 years agological replication: fix OID type mapping mechanism
Alvaro Herrera [Thu, 15 Mar 2018 00:34:26 +0000 (21:34 -0300)]
logical replication: fix OID type mapping mechanism

The logical replication type map seems to have been misused by its only
caller -- it would try to use the remote OID as input for local type
routines, which unsurprisingly could result in bogus "cache lookup
failed for type XYZ" errors, or random other type names being picked up
if they happened to use the right OID.  Fix that, changing
Oid logicalrep_typmap_getid(Oid remoteid) to
char *logicalrep_typmap_gettypname(Oid remoteid)
which is more useful.  If the remote type is not part of the typmap,
this simply prints "unrecognized type" instead of choking trying to
figure out -- a pointless exercise (because the only input for that
comes from replication messages, which are not under the local node's
control) and dangerous to boot, when called from within an error context
callback.

Once that is done, it comes to light that the local OID in the typmap
entry was not being used for anything; the type/schema names are what we
need, so remove local type OID from that struct.

Once you do that, it becomes pointless to attach a callback to regular
syscache invalidation.  So remove that also.

Reported-by: Dang Minh Huong
Author: Masahiko Sawada
Reviewed-by: Álvaro Herrera, Petr Jelínek, Dang Minh Huong, Atsushi Torikoshi
Discussion: https://postgr.es/m/75DB81BEEA95B445AE6D576A0A5C9E936A6BE964@BPXM05GP.gisp.nec.co.jp
Discussion: https://postgr.es/m/75DB81BEEA95B445AE6D576A0A5C9E936A6C4B0A@BPXM05GP.gisp.nec.co.jp

6 years agoFix compiler warning
Peter Eisentraut [Wed, 14 Mar 2018 20:43:40 +0000 (16:43 -0400)]
Fix compiler warning

6 years agoRemove pg_class.relhaspkey
Peter Eisentraut [Wed, 14 Mar 2018 18:59:40 +0000 (14:59 -0400)]
Remove pg_class.relhaspkey

It is not used for anything internally, and it cannot be relied on for
external uses, so it can just be removed.  To correct recommended way to
check for a primary key is in pg_index.

Discussion: https://www.postgresql.org/message-id/flat/b1a24c6c-6913-f89c-674e-0704f0ed69db@2ndquadrant.com

6 years agoFix function-header comments in planner.c
Stephen Frost [Wed, 14 Mar 2018 17:51:15 +0000 (13:51 -0400)]
Fix function-header comments in planner.c

In b5635948ab1, a couple of function header comments weren't changed, or
weren't changed correctly, to reflect the arguments being passed into
the functions.  Specifically, get_number_of_groups() had the wrong
argument name in the commit and create_grouping_paths() wasn't
updated even though the arguments had been changed.

The issue with create_grouping_paths() was noticed by Ashutosh Bapat,
while I discovered the issue with get_number_of_groups() by looking to
see if there were any similar issues from that commit.

Discussion: https://postgr.es/m/CAFjFpRcbp4702jcp387PExt3fNCt62QJN8++DQGwBhsW6wRHWA@mail.gmail.com

6 years agoFix typo in add_paths_to_append_rel()
Stephen Frost [Wed, 14 Mar 2018 17:51:14 +0000 (13:51 -0400)]
Fix typo in add_paths_to_append_rel()

The comment should have been referring to the number of workers, not the
number of paths.

Author: Ashutosh Bapat
Discussion: https://postgr.es/m/CAFjFpRcbp4702jcp387PExt3fNCt62QJN8++DQGwBhsW6wRHWA@mail.gmail.com

6 years agoFixed compiler warnings in test case.
Michael Meskes [Wed, 14 Mar 2018 16:17:53 +0000 (17:17 +0100)]
Fixed compiler warnings in test case.

6 years agoSupport INOUT arguments in procedures
Peter Eisentraut [Wed, 14 Mar 2018 15:47:21 +0000 (11:47 -0400)]
Support INOUT arguments in procedures

In a top-level CALL, the values of INOUT arguments will be returned as a
result row.  In PL/pgSQL, the values are assigned back to the input
arguments.  In other languages, the same convention as for return a
record from a function is used.  That does not require any code changes
in the PL implementations.

Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
6 years agoLog when a BRIN autosummarization request fails
Alvaro Herrera [Wed, 14 Mar 2018 14:53:56 +0000 (11:53 -0300)]
Log when a BRIN autosummarization request fails

Autovacuum's 'workitem' request queue is of limited size, so requests
can fail if they arrive more quickly than autovacuum can process them.
Emit a log message when this happens, to provide better visibility of
this.

Backpatch to 10.  While this represents an API change for
AutoVacuumRequestWork, that function is not yet prepared to deal with
external modules calling it, so there doesn't seem to be any risk (other
than log spam, that is.)

Author: Masahiko Sawada
Reviewed-by: Fabrízio Mello, Ildar Musin, Álvaro Herrera
Discussion: https://postgr.es/m/CAD21AoB1HrQhp6_4rTyHN5kWEJCEsG8YzsjZNt-ctoXSn5Uisw@mail.gmail.com

6 years agoFix comment for ExecProcessReturning
Stephen Frost [Wed, 14 Mar 2018 13:28:08 +0000 (09:28 -0400)]
Fix comment for ExecProcessReturning

resultRelInfo is the argument for the function, not projectReturning.

Author: Etsuro Fujita
Discussion: https://postgr.es/m/5AA8E11E.1040609@lab.ntt.co.jp

6 years agoAdd tests for reinit.c
Peter Eisentraut [Wed, 14 Mar 2018 13:03:44 +0000 (09:03 -0400)]
Add tests for reinit.c

This tests how the different forks of unlogged tables behave in crash
recovery.

Author: David Steele <david@pgmasters.net>

6 years agoAdd Oracle like handling of char arrays.
Michael Meskes [Tue, 13 Mar 2018 23:54:13 +0000 (00:54 +0100)]
Add Oracle like handling of char arrays.

In some cases Oracle Pro*C handles char array differently than ECPG. This patch
adds a Oracle compatibility mode to make ECPG behave like Pro*C.

Patch by David Rader <davidr@openscg.com>

6 years agoFix double frees in ecpg.
Michael Meskes [Tue, 13 Mar 2018 23:47:49 +0000 (00:47 +0100)]
Fix double frees in ecpg.

Patch by Patrick Krecker <patrick@judicata.com>

6 years agoExpand AND/OR regression tests around NULL handling.
Andres Freund [Fri, 9 Mar 2018 01:07:16 +0000 (17:07 -0800)]
Expand AND/OR regression tests around NULL handling.

Previously there were no tests verifying that NULL handling in AND/OR
was correct (i.e. that NULL rather than false is returned if
expression doesn't return true).

Author: Andres Freund

6 years agoAdd COSTS off to two EXPLAIN using tests.
Andres Freund [Tue, 13 Mar 2018 06:09:58 +0000 (23:09 -0700)]
Add COSTS off to two EXPLAIN using tests.

Discussion: https://postgr.es/m/20180312222023.i4sgkbl4oqtstus3@alap3.anarazel.de

6 years agoLet Parallel Append over simple UNION ALL have partial subpaths.
Robert Haas [Tue, 13 Mar 2018 20:34:08 +0000 (16:34 -0400)]
Let Parallel Append over simple UNION ALL have partial subpaths.

A simple UNION ALL gets flattened into an appendrel of subquery
RTEs, but up until now it's been impossible for the appendrel to use
the partial paths for the subqueries, so we can implement the
appendrel as a Parallel Append but only one with non-partial paths
as children.

There are three separate obstacles to removing that limitation.
First, when planning a subquery, propagate any partial paths to the
final_rel so that they are potentially visible to outer query levels
(but not if they have initPlans attached, because that wouldn't be
safe).  Second, after planning a subquery, propagate any partial paths
for the final_rel to the subquery RTE in the outer query level in the
same way we do for non-partial paths.  Third, teach finalize_plan() to
account for the possibility that the fake parameter we use for rescan
signalling when the plan contains a Gather (Merge) node may be
propagated from an outer query level.

Patch by me, reviewed and tested by Amit Khandekar, Rajkumar
Raghuwanshi, and Ashutosh Bapat.  Test cases based on examples by
Rajkumar Raghuwanshi.

Discussion: http://postgr.es/m/CA+Tgmoa6L9A1nNCk3aTDVZLZ4KkHDn1+tm7mFyFvP+uQPS7bAg@mail.gmail.com

6 years agoWhen updating reltuples after ANALYZE, just extrapolate from our sample.
Tom Lane [Tue, 13 Mar 2018 17:24:27 +0000 (13:24 -0400)]
When updating reltuples after ANALYZE, just extrapolate from our sample.

The existing logic for updating pg_class.reltuples trusted the sampling
results only for the pages ANALYZE actually visited, preferring to
believe the previous tuple density estimate for all the unvisited pages.
While there's some rationale for doing that for VACUUM (first that
VACUUM is likely to visit a very nonrandom subset of pages, and second
that we know for sure that the unvisited pages did not change), there's
no such rationale for ANALYZE: by assumption, it's looked at an unbiased
random sample of the table's pages.  Furthermore, in a very large table
ANALYZE will have examined only a tiny fraction of the table's pages,
meaning it cannot slew the overall density estimate very far at all.
In a table that is physically growing, this causes reltuples to increase
nearly proportionally to the change in relpages, regardless of what is
actually happening in the table.  This has been observed to cause reltuples
to become so much larger than reality that it effectively shuts off
autovacuum, whose threshold for doing anything is a fraction of reltuples.
(Getting to the point where that would happen seems to require some
additional, not well understood, conditions.  But it's undeniable that if
reltuples is seriously off in a large table, ANALYZE alone will not fix it
in any reasonable number of iterations, especially not if the table is
continuing to grow.)

Hence, restrict the use of vac_estimate_reltuples() to VACUUM alone,
and in ANALYZE, just extrapolate from the sample pages on the assumption
that they provide an accurate model of the whole table.  If, by very bad
luck, they don't, at least another ANALYZE will fix it; in the old logic
a single bad estimate could cause problems indefinitely.

In HEAD, let's remove vac_estimate_reltuples' is_analyze argument
altogether; it was never used for anything and now it's totally pointless.
But keep it in the back branches, in case any third-party code is calling
this function.

Per bug #15005.  Back-patch to all supported branches.

David Gould, reviewed by Alexander Kuzmenkov, cosmetic changes by me

Discussion: https://postgr.es/m/20180117164916.3fdcf2e9@engels

6 years agoAvoid holding AutovacuumScheduleLock while rechecking table statistics.
Tom Lane [Tue, 13 Mar 2018 16:28:15 +0000 (12:28 -0400)]
Avoid holding AutovacuumScheduleLock while rechecking table statistics.

In databases with many tables, re-fetching the statistics takes some time,
so that this behavior seriously decreases the available concurrency for
multiple autovac workers.  There's discussion afoot about more complete
fixes, but a simple and back-patchable amelioration is to claim the table
and release the lock before rechecking stats.  If we find out there's no
longer a reason to process the table, re-taking the lock to un-claim the
table is cheap enough.

(This patch is quite old, but got lost amongst a discussion of more
aggressive fixes.  It's not clear when or if such a fix will be
accepted, but in any case it'd be unlikely to get back-patched.
Let's do this now so we have some improvement for the back branches.)

In passing, make the normal un-claim step take AutovacuumScheduleLock
not AutovacuumLock, since that is what is documented to protect the
wi_tableoid field.  This wasn't an actual bug in view of the fact that
readers of that field hold both locks, but it creates some concurrency
penalty against operations that need only AutovacuumLock.

Back-patch to all supported versions.

Jeff Janes

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

6 years agoSet connection back to NULL after freeing it.
Michael Meskes [Mon, 12 Mar 2018 22:52:08 +0000 (23:52 +0100)]
Set connection back to NULL after freeing it.

Patch by Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>

6 years agoMove strtoint() to common
Peter Eisentraut [Tue, 13 Mar 2018 14:21:09 +0000 (10:21 -0400)]
Move strtoint() to common

Several places used similar code to convert a string to an int, so take
the function that we already had and make it globally available.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoChange internal integer representation of Value node
Peter Eisentraut [Mon, 12 Mar 2018 16:17:58 +0000 (12:17 -0400)]
Change internal integer representation of Value node

A Value node would store an integer as a long.  This causes needless
portability risks, as long can be of varying sizes.  Change it to use
int instead.  All code using this was already careful to only store
32-bit values anyway.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoFix CREATE TABLE / LIKE with bigint identity column
Peter Eisentraut [Wed, 7 Mar 2018 19:38:35 +0000 (14:38 -0500)]
Fix CREATE TABLE / LIKE with bigint identity column

CREATE TABLE / LIKE with a bigint identity column would fail on
platforms where long is 32 bits.  Copying the sequence values used
makeInteger(), which would truncate the 64-bit sequence data to 32 bits.
To fix, use makeFloat() instead, like the parser.  (This does not
actually make use of floats, but stores the values as strings.)

Bug: #15096
Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoAvoid having two PKs in a partition
Alvaro Herrera [Mon, 12 Mar 2018 22:42:32 +0000 (19:42 -0300)]
Avoid having two PKs in a partition

If a table containing a primary key is attach as partition to a
partitioned table which has a primary key with a different definition,
we would happily create a second one in the new partition.  Oops.  It
turns out that this is because an error check in DefineIndex is executed
only if you tell it that it's being run by ALTER TABLE, and the original
code here wasn't.  Change it so that it does.

Added a couple of test cases for this, also.  A previously working test
started to fail in a different way than before patch because the new
check is called earlier; change the PK to plain UNIQUE so that the new
behavior isn't invoked, so that the test continues to verify what we
want it to verify.

Reported by: Noriyoshi Shinoda
Discussion: https://postgr.es/m/DF4PR8401MB102060EC2615EC9227CC73F7EEDF0@DF4PR8401MB1020.NAMPRD84.PROD.OUTLOOK.COM

6 years agodoc: Reword restriction on partition keys in unique indexes
Alvaro Herrera [Mon, 12 Mar 2018 16:28:52 +0000 (13:28 -0300)]
doc: Reword restriction on partition keys in unique indexes

New wording from David G. Johnston, who noticed the unreadable original
also.  Include his suggested test case as well.

Fix a typo I noticed elsewhere while doing this.

Discussion: https://postgr.es/m/CAKFQuwY4Ld7ecxL_KAmaxwt0FUu5VcPPN2L4dh+3BeYbrdBa5g@mail.gmail.com

6 years agodocs: Fix typo: a -> an
Alvaro Herrera [Mon, 12 Mar 2018 15:58:35 +0000 (12:58 -0300)]
docs: Fix typo: a -> an

David Rowley

6 years agoRemove doc sentence no longer applicable
Alvaro Herrera [Mon, 12 Mar 2018 14:38:20 +0000 (11:38 -0300)]
Remove doc sentence no longer applicable

Amit Langote

6 years agoFix improper uses of canonicalize_qual().
Tom Lane [Sun, 11 Mar 2018 22:10:42 +0000 (18:10 -0400)]
Fix improper uses of canonicalize_qual().

One of the things canonicalize_qual() does is to remove constant-NULL
subexpressions of top-level AND/OR clauses.  It does that on the assumption
that what it's given is a top-level WHERE clause, so that NULL can be
treated like FALSE.  Although this is documented down inside a subroutine
of canonicalize_qual(), it wasn't mentioned in the documentation of that
function itself, and some callers hadn't gotten that memo.

Notably, commit d007a9505 caused get_relation_constraints() to apply
canonicalize_qual() to CHECK constraints.  That allowed constraint
exclusion to misoptimize situations in which a CHECK constraint had a
provably-NULL subclause, as seen in the regression test case added here,
in which a child table that should be scanned is not.  (Although this
thinko is ancient, the test case doesn't fail before 9.2, for reasons
I've not bothered to track down in detail.  There may be related cases
that do fail before that.)

More recently, commit f0e44751d added an independent bug by applying
canonicalize_qual() to index expressions, which is even sillier since
those might not even be boolean.  If they are, though, I think this
could lead to making incorrect index entries for affected index
expressions in v10.  I haven't attempted to prove that though.

To fix, add an "is_check" parameter to canonicalize_qual() to specify
whether it should assume WHERE or CHECK semantics, and make it perform
NULL-elimination accordingly.  Adjust the callers to apply the right
semantics, or remove the call entirely in cases where it's not known
that the expression has one or the other semantics.  I also removed
the call in some cases involving partition expressions, where it should
be a no-op because such expressions should be canonical already ...
and was a no-op, independently of whether it could in principle have
done something, because it was being handed the qual in implicit-AND
format which isn't what it expects.  In HEAD, add an Assert to catch
that type of mistake in future.

This represents an API break for external callers of canonicalize_qual().
While that's intentional in HEAD to make such callers think about which
case applies to them, it seems like something we probably wouldn't be
thanked for in released branches.  Hence, in released branches, the
extra parameter is added to a new function canonicalize_qual_ext(),
and canonicalize_qual() is a wrapper that retains its old behavior.

Patch by me with suggestions from Dean Rasheed.  Back-patch to all
supported branches.

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

6 years agoClarify initdb --help message for --wal-segsize
Magnus Hagander [Sun, 11 Mar 2018 13:12:36 +0000 (14:12 +0100)]
Clarify initdb --help message for --wal-segsize

Specify that the value is in megabytes. This aligns the message with
what's in the documentation.

6 years agoIn psql, restore old behavior of Query_for_list_of_functions.
Tom Lane [Sat, 10 Mar 2018 18:18:21 +0000 (13:18 -0500)]
In psql, restore old behavior of Query_for_list_of_functions.

Historically, tab completion for functions has offered the names of
aggregates as well.  This is essential in at least one context, namely
GRANT/REVOKE, because there is no GRANT ON AGGREGATE syntax.  There
are other cases where a command that nominally is for functions will
allow aggregates as well, though not all do.

Commit fd1a421fe changed this query to disallow aggregates, but that
doesn't seem to have been thought through very carefully.  Change it
to allow aggregates (but still ignore procedures).

We might at some point tighten this up, but it'd require sorting through
all the uses of this query to see which ones should offer aggregate
names and which shouldn't.  Given the lack of field complaints about
the historical laxity here, that's work I'm not eager to do right now.

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

6 years agoImprove predtest.c's internal docs, and enhance its functionality a bit.
Tom Lane [Fri, 9 Mar 2018 21:58:26 +0000 (16:58 -0500)]
Improve predtest.c's internal docs, and enhance its functionality a bit.

Commit b08df9cab left things rather poorly documented as far as the
exact semantics of "clause_is_check" mode went.  Also, that mode did
not really work correctly for predicate_refuted_by; although given the
lack of specification as to what it should do, as well as the lack
of any actual use-case, that's perhaps not surprising.

Rename "clause_is_check" to "weak" proof mode, and provide specifications
for what it should do.  I defined weak refutation as meaning "truth of A
implies non-truth of B", which makes it possible to use the mode in the
part of relation_excluded_by_constraints that checks for mutually
contradictory WHERE clauses.  Fix up several places that did things wrong
for that definition.  (As far as I can see, these errors would only lead
to failure-to-prove, not incorrect claims of proof, making them not
serious bugs even aside from the fact that v10 contains no use of this
mode.  So there seems no need for back-patching.)

In addition, teach predicate_refuted_by_recurse that it can use
predicate_implied_by_recurse after all when processing a strong NOT-clause,
so long as it asks for the correct proof strength.  This is an optimization
that could have been included in commit b08df9cab, but wasn't.

Also, simplify and generalize the logic that checks for whether nullness of
the argument of IS [NOT] NULL would force overall nullness of the predicate
or clause.  (This results in a change in the partition_prune test's output,
as it is now able to prune an all-nulls partition that it did not recognize
before.)

In passing, in PartConstraintImpliedByRelConstraint, remove bogus
conversion of the constraint list to explicit-AND form and then right back
again; that accomplished nothing except forcing a useless extra level of
recursion inside predicate_implied_by.

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

6 years agoFix test_predtest's idea of what weak refutation means.
Tom Lane [Fri, 9 Mar 2018 00:44:14 +0000 (19:44 -0500)]
Fix test_predtest's idea of what weak refutation means.

I'd initially supposed that predicate_refuted_by(..., true) ought to
say that "A refutes B" means "non-falsity of A implies non-truth of B".
But it seems better to define it as "truth of A implies non-truth of B".
This is more useful in the current system, slightly easier to prove,
and in closer correspondence to the existing code behavior.

With this change, test_predtest no longer claims that any existing
test cases show false proof reports, though there still are cases
where we could prove something and fail to.

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

6 years agoCorrectly assess parallel-safety of tlists when SRFs are used.
Robert Haas [Thu, 8 Mar 2018 19:25:31 +0000 (14:25 -0500)]
Correctly assess parallel-safety of tlists when SRFs are used.

Since commit 69f4b9c85f168ae006929eec44fc44d569e846b9, the existing
code was no longer assessing the parallel-safety of the real tlist
for each upper rel, but rather the first of possibly several tlists
created by split_pathtarget_at_srfs().  Repair.

Even though this is clearly wrong, it's not clear that it has any
user-visible consequences at the moment, so no back-patch for now.  If
we discover later that it does have user-visible consequences, we
might need to back-patch this to v10.

Patch by me, per a report from Rajkumar Raghuwanshi.

Discussion: http://postgr.es/m/CA+Tgmoaob_Strkg4Dcx=VyxnyXtrmkV=ofj=pX7gH9hSre-g0Q@mail.gmail.com

6 years agoAdd test scaffolding for exercising optimizer's predicate-proof logic.
Tom Lane [Thu, 8 Mar 2018 18:25:26 +0000 (13:25 -0500)]
Add test scaffolding for exercising optimizer's predicate-proof logic.

The predicate-proof code in predtest.c is a bit hard to test as-is:
you have to construct a query whose plan changes depending on the success
of a test, and in tests that have been around for awhile, it's always
possible that the plan shape is now being determined by some other factor.
Our existing regression tests aren't doing real well at providing full
code coverage of predtest.c, either.  So, let's add a small test module
that allows directly inspecting the results of predicate_implied_by()
and predicate_refuted_by() for arbitrary boolean expressions.

I chose the set of tests committed here in order to get reasonably
complete code coverage of predtest.c just from running this test
module, and to cover some cases called out as being interesting in
the existing comments.  We might want to add more later.  But this
set already shows a few cases where perhaps things could be improved.

Indeed, this exercise proves that predicate_refuted_by() is buggy for
the case of clause_is_check = true, though fortunately we aren't using
that case anywhere yet.  I'll look into doing something about that in
a separate commit.  For now, just memorialize the current behavior.

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

6 years agoRevert "Temporarily instrument postgres_fdw test to look for statistics changes."
Tom Lane [Thu, 8 Mar 2018 16:33:27 +0000 (11:33 -0500)]
Revert "Temporarily instrument postgres_fdw test to look for statistics changes."

This reverts commit c2c537c56dc30ec3cdc12051f4ea5363aa66d73c.
It's now clear that whatever is going on there, it can't be blamed
on unexpected ANALYZE runs, because the statistics are the same
just before the failing query as they were at the start of the test.

6 years agoIn initdb, don't bother trying max_connections = 10.
Tom Lane [Thu, 8 Mar 2018 16:26:20 +0000 (11:26 -0500)]
In initdb, don't bother trying max_connections = 10.

The server won't actually start with that setting anymore, not since
we raised the default max_wal_senders to 10.  Per discussion, we don't
wish to back down on that default, so instead raise the effective floor
for max_connections (to 20).  It's still possible to configure a smaller
setting manually, but initdb won't set it that way.

Since that change happened in v10, back-patch to v10.

Kyotaro Horiguchi

Discussion: https://postgr.es/m/20180209.170823.42008365.horiguchi.kyotaro@lab.ntt.co.jp

6 years agoFix cross-checking of ReservedBackends/max_wal_senders/MaxConnections.
Tom Lane [Thu, 8 Mar 2018 16:25:26 +0000 (11:25 -0500)]
Fix cross-checking of ReservedBackends/max_wal_senders/MaxConnections.

We were independently checking ReservedBackends < MaxConnections and
max_wal_senders < MaxConnections, but because walsenders aren't allowed
to use superuser-reserved connections, that's really the wrong thing.
Correct behavior is to insist on ReservedBackends + max_wal_senders being
less than MaxConnections.  Fix the code and associated documentation.

This has been wrong for a long time, but since the situation probably
hardly ever arises in the field (especially pre-v10, when the default
for max_wal_senders was zero), no back-patch.

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

6 years agotest_decoding: Remove unused #include directives.
Robert Haas [Wed, 7 Mar 2018 22:09:39 +0000 (17:09 -0500)]
test_decoding: Remove unused #include directives.

Euler Taveira

Discussion: http://postgr.es/m/CAHE3wghBwKoCmK_sRu4xUL7f-q3dVOSwqjnOkaGmvWAqWUKaSQ@mail.gmail.com

6 years agodoc: Add more substructure to SSL documentation
Peter Eisentraut [Wed, 7 Mar 2018 16:32:51 +0000 (11:32 -0500)]
doc: Add more substructure to SSL documentation

The SSL documentation text has gotten a bit long, so add some
subsections and reorder for better flow.

6 years agoAdd missing debug lines during bootstrap
Alvaro Herrera [Wed, 7 Mar 2018 14:47:35 +0000 (11:47 -0300)]
Add missing debug lines during bootstrap

Noticed while playing with changes that mess with the bootstrap
sequence; the operations patched here failed to emit anything, leading
the developer to think that the bug was in the previous operation that
did emit a message.

6 years agoFix typo
Peter Eisentraut [Wed, 7 Mar 2018 14:02:57 +0000 (09:02 -0500)]
Fix typo

Author: Daniel Gustafsson <daniel@yesql.se>

6 years agoFix typo
Alvaro Herrera [Wed, 7 Mar 2018 10:07:41 +0000 (07:07 -0300)]
Fix typo

Author: Kyotaro HORIGUCHI
Discussion: https://postgr.es/m/20180307.163428.209919771.horiguchi.kyotaro@lab.ntt.co.jp

6 years agoFix test counting in SSL tests
Peter Eisentraut [Tue, 6 Mar 2018 19:49:07 +0000 (14:49 -0500)]
Fix test counting in SSL tests

The branch that does not support tls-server-end-point runs more tests,
so we need to structure the test counting dynamically.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoFix typo for RangeVarGetRelidExtended
Stephen Frost [Wed, 7 Mar 2018 04:36:26 +0000 (23:36 -0500)]
Fix typo for RangeVarGetRelidExtended

The function is actually RangeVarGetRelidExtended, so the comment should
reflect that.

Author: Michael Paquier
Discussion: https://postgr.es/m/20180307035216.GA3184@paquier.xyz

6 years agoFix costing of parallel hash joins.
Peter Eisentraut [Wed, 7 Mar 2018 02:54:37 +0000 (21:54 -0500)]
Fix costing of parallel hash joins.

Commit 1804284042e659e7d16904e7bbb0ad546394b6a3 established that single-batch
parallel-aware hash joins could create one large shared hash table using the
combined work_mem budget of all participants.  The costing accidentally
assumed that parallel-oblivious hash joins could also do that.  The
documentation for initial_cost_hashjoin() also failed to mention the new
argument.  Repair.

Author: Thomas Munro
Reported-By: Antonin Houska
Reviewed-By: Antonin Houska
Discussion: https://postgr.es/m/12441.1513935950%40localhost

6 years agodoc: Improve calculation of vm.nr_hugepages
Peter Eisentraut [Wed, 7 Mar 2018 02:45:28 +0000 (21:45 -0500)]
doc: Improve calculation of vm.nr_hugepages

The previous method worked off the full virtual address space, not just
the shared memory usage.

Author: Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Vasundhar Boddapati <bvasundhar@gmail.com>
6 years agodoc: Add replication parameter to libpq documentation
Peter Eisentraut [Wed, 7 Mar 2018 02:00:10 +0000 (21:00 -0500)]
doc: Add replication parameter to libpq documentation

Author: Michael Paquier <michael@paquier.xyz>
Reported-by: Şahap Aşçı <sahapasci@gmail.com>
Reviewed-by: Vik Fearing <vik.fearing@2ndquadrant.com>
6 years agoRefrain from duplicating data in reorderbuffers
Alvaro Herrera [Tue, 6 Mar 2018 19:22:03 +0000 (16:22 -0300)]
Refrain from duplicating data in reorderbuffers

If a walsender exits leaving data in reorderbuffers, the next walsender
that tries to decode the same transaction would append its decoded data
in the same spill files without truncating it first, which effectively
duplicate the data.  Avoid that by removing any leftover reorderbuffer
spill files when a walsender starts.

Backpatch to 9.4; this bug has been there from the very beginning of
logical decoding.

Author: Craig Ringer, revised by me
Reviewed by: Álvaro Herrera, Petr Jelínek, Masahiko Sawada

6 years agoFix expected error message in test
Peter Eisentraut [Tue, 6 Mar 2018 19:45:05 +0000 (14:45 -0500)]
Fix expected error message in test

6 years agoRaise Test::More version requirement
Peter Eisentraut [Tue, 6 Mar 2018 19:42:10 +0000 (14:42 -0500)]
Raise Test::More version requirement

Since ed8a7c6fcf92b6b57ed8003bbd4a4eb92a6039bc, version 0.87 is
required.

6 years agoFix bogus Name assignment in CreateStatistics
Alvaro Herrera [Tue, 6 Mar 2018 16:17:13 +0000 (13:17 -0300)]
Fix bogus Name assignment in CreateStatistics

Apparently, it doesn't work to use a plain cstring as a Name datum: you
may end up having random bytes because of failing to zero the bytes
after the terminating \0, as indicated by valgrind.  I introduced this
bug in 5564c1181548, so backpatch this fix to REL_10_STABLE, like that
commit.

While at it, fix a slightly misleading comment, pointed out by David
Rowley.

6 years agoTests for Kerberos/GSSAPI authentication
Peter Eisentraut [Mon, 5 Mar 2018 19:42:11 +0000 (14:42 -0500)]
Tests for Kerberos/GSSAPI authentication

Like the LDAP and SSL tests, these are not run by default but can be
selected via PG_TEST_EXTRA.

Reviewed-by: Thomas Munro <thomas.munro@enterprisedb.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoFix parent node of WCO expressions in partitioned tables.
Andres Freund [Tue, 6 Mar 2018 01:49:59 +0000 (17:49 -0800)]
Fix parent node of WCO expressions in partitioned tables.

Since edd44738bc8814 WCO expressions of partitioned tables are
initialized with the first subplan as parent. That's not correct, as
the correct context is the ModifyTableState node. That's also what is
used for RETURNING processing, initialized nearby.

This appears not to cause any visible problems for in core code, but
is problematic for in development patch.

Discussion: https://postgr.es/m/20180303043818.tnvlo243bgy7una3@alap3.anarazel.de

6 years agoAdd parenthesized options syntax for ANALYZE.
Andres Freund [Tue, 6 Mar 2018 00:21:05 +0000 (16:21 -0800)]
Add parenthesized options syntax for ANALYZE.

This is analogous to the syntax allowed for VACUUM. This allows us to
avoid making new options reserved keywords and makes it easier to
allow arbitrary argument order. Oh, and it's consistent with the other
commands, too.

Author: Nathan Bossart
Reviewed-By: Michael Paquier, Masahiko Sawada
Discussion: https://postgr.es/m/D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com

6 years agoFix HEAP_INSERT_IS_SPECULATIVE to HEAP_INSERT_SPECULATIVE in comments.
Andres Freund [Mon, 5 Mar 2018 23:28:03 +0000 (15:28 -0800)]
Fix HEAP_INSERT_IS_SPECULATIVE to HEAP_INSERT_SPECULATIVE in comments.

This was wrong since 168d5805e4c08bed7b95d351bf097cff7c07dd65, which
introduced speculative inserts.

Author: Andres Freund