Commit Graph

41166 Commits

Author SHA1 Message Date
Tom Lane
b971a98cea Fix placement of initPlans when forcibly materializing a subplan.
If we forcibly place a Material node atop a finished subplan, we need
to move any initPlans attached to the subplan up to the Material node,
in order to keep SS_finalize_plan() happy.  I'd figured this out in
commit 7b67a0a49 for the case of materializing a cursor plan, but out of
an abundance of caution, I put the initPlan movement hack at the call
site for that case, rather than inside materialize_finished_plan().
That was the wrong thing, because it turns out to also be necessary for
the only other caller of materialize_finished_plan(), ie subselect.c.
We lacked any test cases that exposed the mistake, but bug#14524 from
Wei Congrui shows that it's possible to get an initPlan reference into
the top tlist in that case too, and then SS_finalize_plan() complains.
Hence, move the hack into materialize_finished_plan().

In HEAD, also relocate some recently-added tests in subselect.sql, which
I'd unthinkingly dropped into the middle of a sequence of related tests.

Report: https://postgr.es/m/20170202060020.1400.89021@wrigleys.postgresql.org
2017-02-02 19:11:27 -05:00
Heikki Linnakangas
3aab31bbc7 Add KOI8-U map files to Makefile.
These were left out by mistake back when support for KOI8-U encoding was
added.

Extracted from Kyotaro Horiguchi's larger patch.
2017-02-02 14:13:47 +02:00
Andrew Dunstan
13752743bf Don't count background workers against a user's connection limit.
Doing so doesn't seem to be within the purpose of the per user
connection limits, and has particularly unfortunate effects in
conjunction with parallel queries.

Backpatch to 9.6 where parallel queries were introduced.

David Rowley, reviewed by Robert Haas and Albe Laurenz.
2017-02-01 17:59:53 -05:00
Stephen Frost
eb5e9d90df pg_dump: Fix handling of ALTER DEFAULT PRIVILEGES
In commit 23f34fa, we changed how ACLs were handled to use the new
pg_init_privs catalog and to dump out the ACL commands as REVOKE+GRANT
combinations instead of trying to REVOKE all rights always and then
GRANT back just the ones which were in place.

Unfortunately, the DEFAULT PRIVILEGES system didn't quite get the
correct treatment with this change and ended up (incorrectly) only
including positive GRANTs instead of both the REVOKEs and GRANTs
necessary to preserve the correct privileges.

There are only a couple cases where such REVOKEs are possible because,
generally speaking, there's few rights which exist on objects by
default to be revoked.

Examples of REVOKEs which weren't being correctly preserved are when
privileges are REVOKE'd from the creator/owner, like so:

ALTER DEFAULT PRIVILEGES
  FOR ROLE myrole
  REVOKE SELECT ON TABLES FROM myrole;

or when other default privileges are being revoked, such as EXECUTE
rights granted to public for functions:

ALTER DEFAULT PRIVILEGES
  FOR ROLE myrole
  REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;

Fix this by correctly working out what the correct REVOKE statements are
(if any) and dump them out, just as we do for everything else.

Noticed while developing additional regression tests for pg_dump, which
will be landing shortly.

Back-patch to 9.6 where the bug was introduced.
2017-01-31 16:24:14 -05:00
Stephen Frost
3e9c361653 test_pg_dump: perltidy cleanup
As pointed out by Alvaro, we actually use perltidy on the perl scripts
in the source tree, so go back to the results of a perltidy run for the
test_pg_dump TAP script.

To make it look slightly less tragic, I changed most of the independent
arguments into long-form single arguments (eg: -f file.sql changed to be
--file=file.sql) to avoid having them confusingly split across lines due
to perltidy.

Back-patch to 9.6, as the last patch was.
2017-01-31 11:17:40 -05:00
Tom Lane
6da67b684a Update time zone data files to tzdata release 2016j.
DST law changes in northern Cyprus (new zone Asia/Famagusta), Russia (new
zone Europe/Saratov), Tonga, Antarctica/Casey.  Historical corrections for
Asia/Aqtau, Asia/Atyrau, Asia/Gaza, Asia/Hebron, Italy, Malta.  Replace
invented zone abbreviation "TOT" for Tonga with numeric UTC offset; but
as in the past, we'll keep accepting "TOT" for input.
2017-01-30 11:40:39 -05:00
Stephen Frost
20064c0ec2 Handle ALTER EXTENSION ADD/DROP with pg_init_privs
In commit 6c268df, pg_init_privs was added to track the initial
privileges of catalog objects and extensions.  Unfortunately, that
commit didn't include understanding of ALTER EXTENSION ADD/DROP, which
allows the objects associated with an extension to be changed after the
initial CREATE EXTENSION script has been run.

The result of this meant that ACLs for objects added through
ALTER EXTENSION ADD were not recorded into pg_init_privs and we would
end up including those ACLs in pg_dump when we shouldn't have.

This commit corrects that by making sure to have pg_init_privs updated
when ALTER EXTENSION ADD/DROP is run, recording the permissions as they
are at ALTER EXTENSION ADD time, and removing any if/when ALTER
EXTENSION DROP is called.

This issue was pointed out by Moshe Jacobson as commentary on bug #14456
(which was actually a bug about versions prior to 9.6 not handling
custom ACLs on extensions correctly, an issue now addressed with
pg_init_privs in 9.6).

Back-patch to 9.6 where pg_init_privs was introduced.
2017-01-29 23:05:09 -05:00
Stephen Frost
73cd4896f4 test_pg_dump TAP test whitespace cleanup
The formatting of the perl hashes used in the TAP tests for test_pg_dump
was rather horribly inconsistent and made it more difficult than it
really should have been to add new tests or adjust what tests are for
what runs, etc.

Reformat to clean that all up.

Whitespace-only changes.
2017-01-29 23:05:09 -05:00
Tom Lane
60c1d8ffae Orthography fixes for new castNode() macro.
Clean up hastily-composed comment.  Normalize whitespace.

Erik Rijkers and myself
2017-01-27 08:33:58 -05:00
Simon Riggs
40b7800da2 Check interrupts during hot standby waits 2017-01-27 12:13:20 +00:00
Andres Freund
574b091e58 Add castNode(type, ptr) for safe casting between NodeTag based types.
The new function allows to cast from one NodeTag based type to
another, while asserting that the conversion is valid.  This replaces
the common pattern of doing a cast and a Assert(IsA(ptr, type))
close-by.

As this seems likely to be used pervasively, we decided to backpatch
this change the addition of this macro. Otherwise backpatched fixes
are more likely not to work on back-branches.

On branches before 9.6, where we do not yet rely on inline functions
being available, the type assertion is only performed if PG_USE_INLINE
support is detected. The cast obviously is performed regardless.

For the benefit of verifying the macro compiles in the back-branches,
this commit contains a single use of the new macro. On master, a
somewhat larger conversion will be committed separately.

Author: Peter Eisentraut and Andres Freund
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/c5d387d9-3440-f5e0-f9d4-71d53b9fbe52@2ndquadrant.com
Backpatch: 9.2-
2017-01-26 16:47:03 -08:00
Alvaro Herrera
56b42a3341 Remove test for COMMENT ON DATABASE
Our current DDL only allows a database name to be specified in COMMENT
ON DATABASE, which Andrew Dunstan reports to make this test fail on the
buildfarm.  Remove the line until we gain a DDL command that allows the
current database to be operated on without having the specify it by
name.

Backpatch to 9.5, where these tests appeared.

Discussion: https://postgr.es/m/e6084b89-07a7-7e57-51ee-d7b8fc9ec864@2ndQuadrant.com
2017-01-26 17:45:22 -03:00
Simon Riggs
95d1b41450 Reset hot standby xmin after restart
Hot_standby_feedback could be reset by reload and worked correctly, but if
the server was restarted rather than reloaded the xmin was not reset.
Force reset always if hot_standby_feedback is enabled at startup.

Ants Aasma, Craig Ringer

Reported-by: Ants Aasma
2017-01-26 20:06:44 +00:00
Tom Lane
2dfc126471 Ensure that a tsquery like '!foo' matches empty tsvectors.
!foo means "the tsvector does not contain foo", and therefore it should
match an empty tsvector.  ts_match_vq() overenthusiastically supposed
that an empty tsvector could never match any query, so it forcibly
returned FALSE, the wrong answer.  Remove the premature optimization.

Our behavior on this point was inconsistent, because while seqscans and
GIST index searches both failed to match empty tsvectors, GIN index
searches would find them, since GIN scans don't rely on ts_match_vq().
That makes this certainly a bug, not a debatable definition disagreement,
so back-patch to all supported branches.

Report and diagnosis by Tom Dunstan (bug #14515); added test cases by me.

Discussion: https://postgr.es/m/20170126025524.1434.97828@wrigleys.postgresql.org
2017-01-26 12:17:47 -05:00
Tatsuo Ishii
e4e5ea64c0 Fix comments in StrategyNotifyBgWriter().
The interface for the function was changed in
d72731a704 but the comments of the
function was not updated.

Patch by Yugo Nagata.
2017-01-24 09:49:35 +09:00
Peter Eisentraut
012691a04e doc: Update URL for Microsoft download site 2017-01-23 15:00:55 -05:00
Robert Haas
746ba76f1e Avoid useless respawining the autovacuum launcher at high speed.
When (1) autovacuum = off and (2) there's at least one database with
an XID age greater than autovacuum_freeze_max_age and (3) all tables
in that database that need vacuuming are already being processed by a
worker and (4) the autovacuum launcher is started, a kind of infinite
loop occurs.  The launcher starts a worker and immediately exits.  The
worker, finding no worker to do, immediately starts the launcher,
supposedly so that the next database can be processed.  But because
datfrozenxid for that database hasn't been advanced yet, the new
worker gets put right back into the same database as the old one,
where it once again starts the launcher and exits.  High-speed ping
pong ensues.

There are several possible ways to break the cycle; this seems like
the safest one.

Amit Khandekar (code) and Robert Haas (comments), reviewed by
Álvaro Herrera.

Discussion: http://postgr.es/m/CAJ3gD9eWejf72HKquKSzax0r+epS=nAbQKNnykkMA0E8c+rMDg@mail.gmail.com
2017-01-20 16:11:45 -05:00
Stephen Frost
fd081cabf7 Dump sequence data based on the TableDataInfo flag
When considering a sequence's Data entry in dumpSequenceData, we were
actually looking at the sequence definition's dump flag to decide if we
should dump the data or not.  That's generally fine, except for when the
sequence data entry was created by processExtensionTables() because it's
a config sequence.  In that case, the sequence itself won't be marked as
dumping data because it's part of an extension, leading to the need for
processExtensionTables() to create the sequence data entry.

This leads to extension config sequence data not being included in the
dump when it should be.  Fix this by looking at the sequence data's dump
flag instead, just as dumpTableData() was doing for tables (which is why
config tables were correctly being handled), and add a regression test
to make sure we don't break it moving forward.

All of this is a bit round-about since we can now represent which
components of a given dump item should be dumped out through the dump
flag.  A future improvement might be to change checkExtensionMembership()
to check for config sequences/tables and set the dump flag based on that
directly, possibly removing the need for processExtensionTables().

Bug found by Daniele Varrazzo.

Discussion: https://postgr.es/m/CA+mi_8ZmxQM7+nZ7pJ8uyfxc9V3o=UAG14dVqvftdmvw8OJ3gQ@mail.gmail.com

Patch by Michael Paquier, with some tweaking of the regression tests by
me.

Back-patch to 9.6 where the bug was introduced.
2017-01-19 12:06:27 -05:00
Tom Lane
692a70427a Reset the proper GUC in create_index test.
Thinko in commit a4523c5aa.  It doesn't really affect anything at
present, but it would be a problem if any tests added later in this
file ought to get index-only-scan plans.  Back-patch, like the previous
commit, just to avoid surprises in case we add such a test and then
back-patch it.

Nikita Glukhov

Discussion: https://postgr.es/m/8b70135d-ad38-bdd8-ac92-71e2b3c273cf@postgrespro.ru
2017-01-18 16:33:55 -05:00
Alvaro Herrera
fce4609d5e Change some test macros to return true booleans
These macros work fine when they are used directly in an "if" test or
similar, but as soon as the return values are assigned to boolean
variables (or passed as boolean arguments to some function), they become
bugs, hopefully caught by compiler warnings.  To avoid future problems,
fix the definitions so that they return actual booleans.

To further minimize the risk that somebody uses them in back-patched
fixes that only work correctly in branches starting from the current
master and not in old ones, back-patch the change to supported branches
as appropriate.

See also commit af4472bcb8, and the long
discussion (and larger patch) in the thread mentioned in its commit
message.

Discussion: https://postgr.es/m/18672.1483022414@sss.pgh.pa.us
2017-01-18 18:06:13 -03:00
Tom Lane
b21e665f2d Disable transforms that replaced AT TIME ZONE with RelabelType.
These resulted in wrong answers if the relabeled argument could be matched
to an index column, as shown in bug #14504 from Evgeniy Kozlov.  We might
be able to resurrect these optimizations by adjusting the planner's
treatment of RelabelType, or by adjusting btree's rules for selecting
comparison functions, but either solution will take careful analysis
and does not sound like a fit candidate for backpatching.

I left the catalog infrastructure in place and just reduced the transform
functions to always-return-NULL.  This would be necessary anyway in the
back branches, and it doesn't seem important to be more invasive in HEAD.

Bug introduced by commit b8a18ad48.  Back-patch to 9.5 where that came in.

Report: https://postgr.es/m/20170118144828.1432.52823@wrigleys.postgresql.org
Discussion: https://postgr.es/m/18771.1484759439@sss.pgh.pa.us
2017-01-18 15:21:52 -05:00
Fujii Masao
60a8b63d24 Fix an assertion failure related to an exclusive backup.
Previously multiple sessions could execute pg_start_backup() and
pg_stop_backup() to start and stop an exclusive backup at the same time.
This could trigger the assertion failure of
"FailedAssertion("!(XLogCtl->Insert.exclusiveBackup)".
This happend because, even while pg_start_backup() was starting
an exclusive backup, other session could run pg_stop_backup()
concurrently and mark the backup as not-in-progress unconditionally.

This patch introduces ExclusiveBackupState indicating the state of
an exclusive backup. This state is used to ensure that there is only
one session running pg_start_backup() or pg_stop_backup() at
the same time, to avoid the assertion failure.

Back-patch to all supported versions.

Author: Michael Paquier
Reviewed-By: Kyotaro Horiguchi and me
Reported-By: Andreas Seltenreich
Discussion: <87mvktojme.fsf@credativ.de>
2017-01-17 17:29:15 +09:00
Tom Lane
8a70d8ae75 Throw suitable error for COPY TO STDOUT/FROM STDIN in a SQL function.
A client copy can't work inside a function because the FE/BE wire protocol
doesn't support nesting of a COPY operation within query results.  (Maybe
it could, but the protocol spec doesn't suggest that clients should support
this, and libpq for one certainly doesn't.)

In most PLs, this prohibition is enforced by spi.c, but SQL functions don't
use SPI.  A comparison of _SPI_execute_plan() and init_execution_state()
shows that rejecting client COPY is the only discrepancy in what they
allow, so there's no other similar bugs.

This is an astonishingly ancient oversight, so back-patch to all supported
branches.

Report: https://postgr.es/m/BY2PR05MB2309EABA3DEFA0143F50F0D593780@BY2PR05MB2309.namprd05.prod.outlook.com
2017-01-14 13:27:47 -05:00
Peter Eisentraut
f7acc0397c pg_upgrade: Fix for changed pg_ctl default stop mode
In 9.5, the default pg_ctl stop mode was changed from "smart" to "fast".
pg_upgrade still thought the default mode was "smart" and only specified
the mode when "fast" was asked for.  This results in using "fast" all
the time.  It's not clear what the effect in practice is, but fix it
nonetheless to restore the previous behavior.
2017-01-13 16:19:48 -05:00
Robert Haas
2d443ae1b0 Fix cardinality estimates for parallel joins.
For a partial path, the cardinality estimate needs to reflect the
number of rows we think each worker will see, rather than the total
number of rows; otherwise, costing will go wrong.  The previous coding
got this completely wrong for parallel joins.

Unfortunately, this change may destabilize plans for users of 9.6 who
have enabled parallel query, but since 9.6 is still fairly new I'm
hoping expectations won't be too settled yet.  Also, this is really a
brown-paper-bag bug, so leaving it unfixed for the entire lifetime of
9.6 seems unwise.

Related reports (whose import I initially failed to recognize) by
Tomas Vondra and Tom Lane.

Discussion: http://postgr.es/m/CA+TgmoaDxZ5z5Kw_oCQoymNxNoVaTCXzPaODcOuao=CzK8dMZw@mail.gmail.com
2017-01-13 13:36:09 -05:00
Peter Eisentraut
d2d7163d47 Fix mistake in comment
The node->restart() function doesn't take a mode argument.
2017-01-12 10:25:33 -05:00
Stephen Frost
4b5f399177 pg_restore: Don't allow non-positive number of jobs
pg_restore will currently accept invalid values for the number of
parallel jobs to run (eg: -1), unlike pg_dump which does check that the
value provided is reasonable.

Worse, '-1' is actually a valid, independent, parameter (as an alias for
--single-transaction), leading to potentially completely unexpected
results from a command line such as:

  -> pg_restore -j -1

Where a user would get neither parallel jobs nor a single-transaction.

Add in validity checking of the parallel jobs option, as we already have
in pg_dump, before we try to open up the archive.  Also move the check
that we haven't been asked to run more parallel jobs than possible on
Windows to the same place, so we do all the option validity checking
before opening the archive.

Back-patch all the way, though for 9.2 we're adding the Windows-specific
check against MAXIMUM_WAIT_OBJECTS as that check wasn't back-patched
originally.

Discussion: https://www.postgresql.org/message-id/20170110044815.GC18360%40tamriel.snowman.net
2017-01-11 15:45:53 -05:00
Bruce Momjian
1c15f84348 pg_xlogdump: document --path behavior
The previous --path documentation and --help output were wrong in both
its meaning and the defaults.

Reviewed-by: Michael Paquier

Backpatch-through: 9.6
2017-01-10 22:38:13 -05:00
Stephen Frost
22a85b3fbe pg_dump: Strict names with no matching schema
When using pg_dump --strict-names and a schema pattern which doesn't
match any schemas (eg: --schema='nonexistant*'), we were incorrectly
throwing an error claiming no tables were found when, really, there
were no schemas found:

  -> pg_dump --strict-names --schema='nonexistant*'
  pg_dump: no matching tables were found for pattern "nonexistant*"

Fix that by changing the error message to say 'schemas' instead, since
that is what we are actually complaining about.

Noticed while testing pg_dump error cases.

Back-patch to 9.6 where --strict-names and this error message were
introduced.
2017-01-10 11:34:55 -05:00
Stephen Frost
96f2344f38 Fix invalid-parallel-jobs error message
Including the program name twice is not helpful:

-> pg_dump -j -1
pg_dump: pg_dump: invalid number of parallel jobs

Correct by removing the progname from the exit_horribly() call used when
validating the number of parallel jobs.

Noticed while testing various pg_dump error cases.

Back-patch to 9.3 where parallel pg_dump was added.
2017-01-09 23:09:33 -05:00
Alvaro Herrera
4e563a1f65 Fix ALTER TABLE / SET TYPE for irregular inheritance
If inherited tables don't have exactly the same schema, the USING clause
in an ALTER TABLE / SET DATA TYPE misbehaves when applied to the
children tables since commit 9550e8348b.  Starting with that commit,
the attribute numbers in the USING expression are fixed during parse
analysis.  This can lead to bogus errors being reported during
execution, such as:
   ERROR:  attribute 2 has wrong type
   DETAIL:  Table has type smallint, but query expects integer.

Since it wouldn't do to revert to the original coding, we now apply a
transformation to map the attribute numbers to the correct ones for each
child.

Reported by Justin Pryzby
Analysis by Tom Lane; patch by me.
Discussion: https://postgr.es/m/20170102225618.GA10071@telsasoft.com
2017-01-09 19:26:58 -03:00
Alvaro Herrera
4482c6a23f BRIN revmap pages are not standard pages ...
... and therefore we ought not to tell XLogRegisterBuffer the opposite,
when writing XLog for a brin update that moves the index tuple to a
different page.  Otherwise, xlog insertion would try to "compress the
hole" when producing a full-page image for it; but since we don't update
pd_lower/upper, the hole covers the whole page.  On WAL replay, the
revmap page becomes empty and so the entire portion of the index is
useless and needs to be recomputed.

This is low-probability: a BRIN update only moves an index tuple to a
different page when the summary tuple is larger than the existing one,
which doesn't happen with fixed-width datatypes.  Also, the revmap
page must be first after a checkpoint.

Report and patch: Kuntal Ghosh
Bug is alleged to have detected by a WAL-consistency-checking tool.
Discussion: https://postgr.es/m/CAGz5QCJ=00UQjScSEFbV=0qO5ShTZB9WWz_Fm7+Wd83zPs9Geg@mail.gmail.com

I posted a test case demonstrating the problem, but I'm refraining from
adding it to the test suite; if the WAL consistency tool makes it in,
that will be a better way to catch this from regressing.  (We should
definitely have someting that causes not-same-page updates, though.)
2017-01-09 18:19:29 -03:00
Stephen Frost
8b1bf3161b Protect against NULL-dereference in pg_dump
findTableByOid() is allowed to return NULL and we should therefore be
checking for that case.  getOwnedSeqs() and dumpSequence() shouldn't
ever actually see this happen, but given odd circumstances it might and
commit f9e439b1 probably shouldn't have removed that check.

Pointed out by Coverity.  Initial patch from Michael Paquier.

Back-patch to 9.6, where that commit had removed the check.
2017-01-06 15:27:50 -05:00
Tom Lane
4103a2f200 Invalidate cached plans on FDW option changes.
This fixes problems where a plan must change but fails to do so,
as seen in a bug report from Rajkumar Raghuwanshi.

For ALTER FOREIGN TABLE OPTIONS, do this through the standard method of
forcing a relcache flush on the table.  For ALTER FOREIGN DATA WRAPPER
and ALTER SERVER, just flush the whole plan cache on any change in
pg_foreign_data_wrapper or pg_foreign_server.  That matches the way
we handle some other low-probability cases such as opclass changes, and
it's unclear that the case arises often enough to be worth working harder.
Besides, that gives a patch that is simple enough to back-patch with
confidence.

Back-patch to 9.3.  In principle we could apply the code change to 9.2 as
well, but (a) we lack postgres_fdw to test it with, (b) it's doubtful that
anyone is doing anything exciting enough with FDWs that far back to need
this desperately, and (c) the patch doesn't apply cleanly.

Patch originally by Amit Langote, reviewed by Etsuro Fujita and Ashutosh
Bapat, who each contributed substantial changes as well.

Discussion: https://postgr.es/m/CAKcux6m5cA6rRPTKkqVdJ-R=KKDfe35Q_ZuUqxDSV_4hwga=og@mail.gmail.com
2017-01-06 14:12:52 -05:00
Robert Haas
603299167f Fix possible leak of semaphore count.
Commit 4aec49899e reorganized the order
of operations here so that we no longer increment the number of "extra
waits" before locking the semaphore, but it did not change the
starting value of extraWaits from 0 to -1 to compensate.  In the worst
case, this could leak a semaphore count, but that seems to be unlikely
in practice.

Discussion: http://postgr.es/m/CAA4eK1JyVqXiMba+-a589Rk0pyHsyKkGxeumVKjU6Y74hdrVLQ@mail.gmail.com

Amit Kapila, per an off-list report by Dilip Kumar.  Reviewed by me.
2017-01-05 14:33:29 -05:00
Tom Lane
5b4f8f4c6d Fix handling of empty arrays in array_fill().
array_fill(..., array[0]) produced an empty array, which is probably
what users expect, but it was a one-dimensional zero-length array
which is not our standard representation of empty arrays.  Also, for
no very good reason, it rejected empty input arrays; that case should
be allowed and produce an empty output array.

In passing, remove the restriction that the input array(s) have lower
bound 1.  That seems rather pointless, and it would have needed extra
complexity to make the check deal with empty input arrays.

Per bug #14487 from Andrew Gierth.  It's been broken all along, so
back-patch to all supported branches.

Discussion: https://postgr.es/m/20170105152156.10135.64195@wrigleys.postgresql.org
2017-01-05 11:33:51 -05:00
Tom Lane
f64554b99a Handle OID column inheritance correctly in ALTER TABLE ... INHERIT.
Inheritance operations must treat the OID column, if any, much like
regular user columns.  But MergeAttributesIntoExisting() neglected to
do that, leading to weird results after a table with OIDs is associated
to a parent with OIDs via ALTER TABLE ... INHERIT.

Report and patch by Amit Langote, reviewed by Ashutosh Bapat, some
adjustments by me.  It's been broken all along, so back-patch to
all supported branches.

Discussion: https://postgr.es/m/cb13cfe7-a48c-5720-c383-bb843ab28298@lab.ntt.co.jp
2017-01-04 18:00:11 -05:00
Tom Lane
6e5de703b6 Prefer int-wide pg_atomic_flag over char-wide when using gcc intrinsics.
configure can only probe the existence of gcc intrinsics, not how well
they're implemented, and unfortunately the answer is sometimes "badly".
In particular we've found that multiple compilers fail to implement
char-width __sync_lock_test_and_set() correctly on PPC; and even a correct
implementation would necessarily be pretty inefficient, since that hardware
has only a word-wide primitive to work with.

Given the knowledge we've accumulated in s_lock.h, it appears that it's
best to rely on int-width TAS operations on most non-Intel architectures.
Hence, pick int not char when both are nominally available to us in
generic-gcc.h (note that that code is not used for x86[_64]).

Back-patch to fix regression test failures on FreeBSD/PPC.  Ordinarily
back-patching a change like this would be verboten because of ABI breakage.
But since pg_atomic_flag is not yet used in any Postgres data structure,
there's no ABI to break.  It seems safer to back-patch to avoid possible
gotchas, if someday we do back-patch something that uses pg_atomic_flag.

Discussion: https://postgr.es/m/25414.1483076673@sss.pgh.pa.us
2017-01-04 13:36:44 -05:00
Bruce Momjian
344ae600ac Update copyright for 2017
Backpatch-through: certain files through 9.2
2017-01-03 12:37:53 -05:00
Heikki Linnakangas
7546c135dc Remove bogus notice that older clients might not work with MD5 passwords.
That was written when we still had "crypt" authentication, and it was
referring to the fact that an older client might support "crypt"
authentication but not "md5". But we haven't supported "crypt" for years.
(As soon as we add a new authentication mechanism that doesn't work with
MD5 hashes, we'll need a similar notice again. But this text as it's worded
now is just wrong.)

Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/9a7263eb-0980-2072-4424-440bb2513dc7@iki.fi
2017-01-03 14:09:35 +02:00
Joe Conway
5099e8ee23 Silence compiler warnings
Rearrange a bit of code to ensure that 'mode' in LWLockRelease is
obviously always set, which seems a bit cleaner and avoids a compiler
warning (thanks to Robert for the suggestion!).

Back-patch back to 9.5 where the warning is first seen.

Author: Stephen Frost
Discussion: https://postgr.es/m/20161129152102.GR13284%40tamriel.snowman.net
2017-01-02 14:42:28 -08:00
Joe Conway
7911e78f6c Silence compiler warnings
In GetCachedPlan(), initialize 'plan' to silence a compiler warning, but
also add an Assert() to make sure we don't ever actually fall through
with 'plan' still being set to NULL, since we are about to dereference
it.

Back-patch back to 9.2.

Author: Stephen Frost
Discussion: https://postgr.es/m/20161129152102.GR13284%40tamriel.snowman.net
2017-01-02 14:11:50 -08:00
Tom Lane
f7e7d6fcef Fix incorrect example of to_timestamp() usage.
Must use HH24 not HH to read a hour value exceeding 12.

This was already fixed in HEAD in commit d3cd36a13, but I didn't think
of backpatching it.

Report: https://postgr.es/m/20161229170043.10139.21416@wrigleys.postgresql.org
2016-12-29 18:05:34 -05:00
Tom Lane
21e24eb9a0 Fix interval_transform so it doesn't throw away non-no-op casts.
interval_transform() contained two separate bugs that caused it to
sometimes mistakenly decide that a cast from interval to restricted
interval is a no-op and throw it away.

First, it was wrong to rely on dt.h's field type macros to have an
ordering consistent with the field's significance; in one case they do
not.  This led to mistakenly treating YEAR as less significant than MONTH,
so that a cast from INTERVAL MONTH to INTERVAL YEAR was incorrectly
discarded.

Second, fls(1<<k) produces k+1 not k, so comparing its output directly
to SECOND was wrong.  This led to supposing that a cast to INTERVAL
MINUTE was really a cast to INTERVAL SECOND and so could be discarded.

To fix, get rid of the use of fls(), and make a function based on
intervaltypmodout to produce a field ID code adapted to the need here.

Per bug #14479 from Piotr Stefaniak.  Back-patch to 9.2 where transform
functions were introduced, because this code was born broken.

Discussion: https://postgr.es/m/20161227172307.10135.7747@wrigleys.postgresql.org
2016-12-27 15:43:54 -05:00
Andrew Dunstan
e9cf6e685b Explain unaccounted for space in pgstattuple.
In addition to space accounted for by tuple_len, dead_tuple_len and
free_space, the table_len includes page overhead, the item pointers
table and padding bytes.

Backpatch to live branches.
2016-12-27 11:28:27 -05:00
Tom Lane
462ab6c8b2 Remove triggerable Assert in hashname().
hashname() asserted that the key string it is given is shorter than
NAMEDATALEN.  That should surely always be true if the input is in fact a
regular value of type "name".  However, for reasons of coding convenience,
we allow plain old C strings to be treated as "name" values in many places.
Some SQL functions accept arbitrary "text" inputs, convert them to C
strings, and pass them otherwise-untransformed to syscache lookups for name
columns, allowing an overlength input value to trigger hashname's Assert.

This would be a DOS problem, except that it only happens in assert-enabled
builds which aren't recommended for production.  In a production build,
you'll just get a name lookup error, since regardless of the hash value
computed by hashname, the later equality comparison checks can't match.
Likewise, if the catalog lookup is done by seqscan or indexscan searches,
there will just be a lookup error, since the name comparison functions
don't contain any similar length checks, and will see an overlength input
as unequal to any stored entry.

After discussion we concluded that we should simply remove this Assert.
It's inessential to hashname's own functionality, and having such an
assertion in only some paths for name lookup is more of a foot-gun than
a useful check.  There may or may not be a case for the affected callers
to do something other than let the name lookup fail, but we'll consider
that separately; in any case we probably don't want to change such
behavior in the back branches.

Per report from Tushar Ahuja.  Back-patch to all supported branches.

Report: https://postgr.es/m/7d0809ee-6f25-c9d6-8e74-5b2967830d49@enterprisedb.com
Discussion: https://postgr.es/m/17691.1482523168@sss.pgh.pa.us
2016-12-26 14:58:13 -05:00
Tom Lane
6a8c67f507 Fix incorrect error reporting for duplicate data in \crosstabview.
\crosstabview's complaint about multiple entries for the same crosstab
cell quoted the wrong row and/or column values.  It would accidentally
appear to work if the data had been in strcmp() order to start with,
which probably explains how we missed noticing this during development.

This could be fixed in more than one way, but the way I chose was to
hang onto both result pointers from bsearch() and use those to get at
the value names.

In passing, avoid casting away const in the bsearch comparison functions.
No bug there, just poor style.

Per bug #14476 from Tomonari Katsumata.  Back-patch to 9.6 where
\crosstabview was introduced.

Report: https://postgr.es/m/20161225021519.10139.45460@wrigleys.postgresql.org
2016-12-25 16:04:47 -05:00
Stephen Frost
d51af65705 pg_dumpall: Include --verbose option in --help output
The -v/--verbose option was not included in the output from --help for
pg_dumpall even though it's in the pg_dumpall documentation and has
apparently been around since pg_dumpall was reimplemented in C in 2002.

Fix that by adding it.

Pointed out by Daniel Westermann.

Back-patch to all supported branches.

Discussion: https://www.postgresql.org/message-id/2020970042.4589542.1482482101585.JavaMail.zimbra%40dbi-services.com
2016-12-24 01:42:07 -05:00
Stephen Frost
dc61580bd9 Fix tab completion in psql for ALTER DEFAULT PRIVILEGES
When providing tab completion for ALTER DEFAULT PRIVILEGES, we are
including the list of roles as possible options for completion after the
GRANT or REVOKE.  Further, we accept FOR ROLE/IN SCHEMA at the same time
and in either order, but the tab completion was only working for one or
the other.  Lastly, we weren't using the actual list of allowed kinds of
objects for default privileges for completion after the 'GRANT X ON' but
instead were completeing to what 'GRANT X ON' supports, which isn't the
ssame at all.

Address these issues by improving the forward tab-completion for ALTER
DEFAULT PRIVILEGES and then constrain and correct how the tail
completion is done when it is for ALTER DEFAULT PRIVILEGES.

Back-patch the forward/tail tab-completion to 9.6, where we made it easy
to handle such cases.

For 9.5 and earlier, correct the initial tab-completion to at least be
correct as far as it goes and then add a check for GRANT/REVOKE to only
tab-complete when the GRANT/REVOKE is the start of the command, so we
don't try to do tab-completion after we get to the GRANT/REVOKE part of
the ALTER DEFAULT PRIVILEGES command, which is better than providing
incorrect completions.

Initial patch for master and 9.6 by Gilles Darold, though I cleaned it
up and added a few comments.  All bugs in the 9.5 and earlier patch are
mine.

Discussion: https://www.postgresql.org/message-id/1614593c-e356-5b27-6dba-66320a9bc68b@dalibo.com
2016-12-23 21:01:33 -05:00
Tom Lane
a377c8d921 Doc: improve index entry for "median".
We had an index entry for "median" attached to the percentile_cont function
entry, which was pretty useless because a person following the link would
never realize that that function was the one they were being hinted to use.

Instead, make the index entry point at the example in syntax-aggregates,
and add a <seealso> link to "percentile".

Also, since that example explicitly claims to be calculating the median,
make it use percentile_cont not percentile_disc.  This makes no difference
in terms of the larger goals of that section, but so far as I can find,
nearly everyone thinks that "median" means the continuous not discrete
calculation.

Per gripe from Steven Winfield.  Back-patch to 9.4 where we introduced
percentile_cont.

Discussion: https://postgr.es/m/20161223102056.25614.1166@wrigleys.postgresql.org
2016-12-23 12:53:14 -05:00