- 17 Jul, 2016 4 commits
-
-
Tom Lane authored
To ensure that "make installcheck" can be used safely against an existing installation, we need to be careful about what global object names (database, role, and tablespace names) we use; otherwise we might accidentally clobber important objects. There's been a weak consensus that test databases should have names including "regression", and that test role names should start with "regress_", but we didn't have any particular rule about tablespace names; and neither of the other rules was followed with any consistency either. This commit moves us a long way towards having a hard-and-fast rule that regression test databases must have names including "regression", and that test role and tablespace names must start with "regress_". It's not completely there because I did not touch some test cases in rolenames.sql that test creation of special role names like "session_user". That will require some rethinking of exactly what we want to test, whereas the intent of this patch is just to hit all the cases in which the needed renamings are cosmetic. There is no enforcement mechanism in this patch either, but if we don't add one we can expect that the tests will soon be violating the convention again. Again, that's not such a cosmetic change and it will require discussion. (But I did use a quick-hack enforcement patch to find these cases.) Discussion: <16638.1468620817@sss.pgh.pa.us>
-
Peter Eisentraut authored
The default is no decoration, which looks confusing, for example on the CREATE SEQUENCE man page.
-
Peter Eisentraut authored
The old code used SEQ_MINVALUE to get the smallest int64 value. This was done as a convenience to avoid having to deal with INT64_IS_BUSTED, but that is obsolete now. Also, it is incorrect because the smallest int64 value is actually SEQ_MINVALUE-1. Fix by using PG_INT64_MIN.
-
Stephen Frost authored
Dump out the appropriate GRANT/REVOKE commands for databases and tablespaces from pg_dumpall to replicate what the current state is. This was broken during the changes to buildACLCommands for 9.6+ servers for pg_init_privs.
-
- 16 Jul, 2016 7 commits
-
-
Tom Lane authored
-
Tom Lane authored
On second thought, we should probably do at least a minimal check that the constructed index is valid, since the big problem with the most recent breakage was not whether the sorting was correct but that the index had incorrect hash codes placed in it.
-
Tom Lane authored
We've broken this code path at least twice in the past, so it's prudent to have a test case that covers it. To allow exercising the code path without creating a very large (and slow to run) test case, redefine the sort threshold to be bounded by maintenance_work_mem as well as the number of available buffers. While at it, fix an ancient oversight that when building a temp index, the number of available buffers is not NBuffers but NLocBuffer. Also, if assertions are enabled, apply a direct test that the sort actually does return the tuples in the expected order. Peter Geoghegan Patch: <CAM3SWZTBAo4hjbBd780+MrOKiKp_TMo1N3A0Rw9_im8gbD7fQA@mail.gmail.com>
-
Tom Lane authored
The Assert() here seems unreasonably optimistic. Andreas Seltenreich found that it could fail with NaNs in the input geometries, and it seems likely to me that it might fail in corner cases due to roundoff error, even for ordinary input values. As a band-aid, make the function return SQL NULL instead of crashing. Report: <87d1md1xji.fsf@credativ.de>
-
Tom Lane authored
For some reason this option wasn't discussed at all in client-auth.sgml. Document it there, and be more explicit about its relationship to the "cert" authentication method. Per gripe from Srikanth Venkatesh. I failed to resist the temptation to do some minor wordsmithing in the same area, too. Discussion: <20160713110357.1410.30407@wrigleys.postgresql.org>
-
Tom Lane authored
This should have been done in commit 73c986ad which added several new fields to pg_control, and again in commit 5028f22f which changed the CRC algorithm, but it wasn't. It's far too late to fix it in the 9.5 branch, but let's do so in 9.6, so that if a 9.6 postmaster is started against a 9.4-era pg_control it will complain about a versioning problem rather than a CRC failure. We already forced initdb/pg_upgrade for beta3, so there's no downside to doing this now. Discussion: <7615.1468598094@sss.pgh.pa.us>
-
Andres Freund authored
When heap_update needs to look for a page for the new tuple version, because the current one doesn't have sufficient free space, or when columns have to be processed by the tuple toaster, it has to release the lock on the old page during that. Otherwise there'd be lock ordering and lock nesting issues. To avoid concurrent sessions from trying to update / delete / lock the tuple while the page's content lock is released, the tuple's xmax is set to the current session's xid. That unfortunately was done without any WAL logging, thereby violating the rule that no XIDs may appear on disk, without an according WAL record. If the database were to crash / fail over when the page level lock is released, and some activity lead to the page being written out to disk, the xid could end up being reused; potentially leading to the row becoming invisible. There might be additional risks by not having t_ctid point at the tuple itself, without having set the appropriate lock infomask fields. To fix, compute the appropriate xmax/infomask combination for locking the tuple, and perform WAL logging using the existing XLOG_HEAP_LOCK record. That allows the fix to be backpatched. This issue has existed for a long time. There appears to have been partial attempts at preventing dangers, but these never have fully been implemented, and were removed a long time ago, in 11919160 (cf. HEAP_XMAX_UNLOGGED). In master / 9.6, there's an additional issue, namely that the visibilitymap's freeze bit isn't reset at that point yet. Since that's a new issue, introduced only in a892234f, that'll be fixed in a separate commit. Author: Masahiko Sawada and Andres Freund Reported-By: Different aspects by Thomas Munro, Noah Misch, and others Discussion: CAEepm=3fWAbWryVW9swHyLTY4sXVf0xbLvXqOwUoDiNCx9mBjQ@mail.gmail.com Backpatch: 9.1/all supported versions
-
- 15 Jul, 2016 8 commits
-
-
Andres Freund authored
0ac5ad51 started to compress infomask bits in WAL records. Unfortunately the replay routines for XLOG_HEAP_LOCK/XLOG_HEAP2_LOCK_UPDATED forgot to reset the HEAP_XMAX_INVALID (and some other) hint bits. Luckily that's not problematic in the majority of cases, because after a crash/on a standby row locks aren't meaningful. Unfortunately that does not hold true in the presence of prepared transactions. This means that after a crash, or after promotion, row level locks held by a prepared, but not yet committed, prepared transaction might not be enforced. Discussion: 20160715192319.ubfuzim4zv3rqnxv@alap3.anarazel.de Backpatch: 9.3, the oldest branch on which 0ac5ad51 is present.
-
Tom Lane authored
We must not push down a foreign join when the foreign tables involved should be accessed under different user mappings. Previously we tried to enforce that rule literally during planning, but that meant that the resulting plans were dependent on the current contents of the pg_user_mapping catalog, and we had to blow away all cached plans containing any remote join when anything at all changed in pg_user_mapping. This could have been improved somewhat, but the fact that a syscache inval callback has very limited info about what changed made it hard to do better within that design. Instead, let's change the planner to not consider user mappings per se, but to allow a foreign join if both RTEs have the same checkAsUser value. If they do, then they necessarily will use the same user mapping at runtime, and we don't need to know specifically which one that is. Post-plan-time changes in pg_user_mapping no longer require any plan invalidation. This rule does give up some optimization ability, to wit where two foreign table references come from views with different owners or one's from a view and one's directly in the query, but nonetheless the same user mapping would have applied. We'll sacrifice the first case, but to not regress more than we have to in the second case, allow a foreign join involving both zero and nonzero checkAsUser values if the nonzero one is the same as the prevailing effective userID. In that case, mark the plan as only runnable by that userID. The plancache code already had a notion of plans being userID-specific, in order to support RLS. It was a little confused though, in particular lacking clarity of thought as to whether it was the rewritten query or just the finished plan that's dependent on the userID. Rearrange that code so that it's clearer what depends on which, and so that the same logic applies to both RLS-injected role dependency and foreign-join-injected role dependency. Note that this patch doesn't remove the other issue mentioned in the original complaint, which is that while we'll reliably stop using a foreign join if it's disallowed in a new context, we might fail to start using a foreign join if it's now allowed, but we previously created a generic cached plan that didn't use one. It was agreed that the chance of winning that way was not high enough to justify the much larger number of plan invalidations that would have to occur if we tried to cause it to happen. In passing, clean up randomly-varying spelling of EXPLAIN commands in postgres_fdw.sql, and fix a COSTS ON example that had been allowed to leak into the committed tests. This reverts most of commits fbe5a3fb and 5d4171d1, which were the previous attempt at ensuring we wouldn't push down foreign joins that span permissions contexts. Etsuro Fujita and Tom Lane Discussion: <d49c1e5b-f059-20f4-c132-e9752ee0113e@lab.ntt.co.jp>
-
Alvaro Herrera authored
When key-share locking a tuple that has been not-key-updated, and the update is a committed transaction, in some cases we raised serializability errors: ERROR: could not serialize access due to concurrent update Because the key-share doesn't conflict with the update, the error is unnecessary and inconsistent with the case that the update hasn't committed yet. This causes problems for some usage patterns, even if it can be claimed that it's sufficient to retry the aborted transaction: given a steady stream of updating transactions and a long locking transaction, the long transaction can be starved indefinitely despite multiple retries. To fix, we recognize that HeapTupleSatisfiesUpdate can return HeapTupleUpdated when an updating transaction has committed, and that we need to deal with that case exactly as if it were a non-committed update: verify whether the two operations conflict, and if not, carry on normally. If they do conflict, however, there is a difference: in the HeapTupleBeingUpdated case we can just sleep until the concurrent transaction is gone, while in the HeapTupleUpdated case this is not possible and we must raise an error instead. Per trouble report from Olivier Dony. In addition to a couple of test cases that verify the changed behavior, I added a test case to verify the behavior that remains unchanged, namely that errors are raised when a update that modifies the key is used. That must still generate serializability errors. One pre-existing test case changes behavior; per discussion, the new behavior is actually the desired one. Discussion: https://www.postgresql.org/message-id/560AA479.4080807@odoo.com https://www.postgresql.org/message-id/20151014164844.3019.25750@wrigleys.postgresql.org Backpatch to 9.3, where the problem appeared.
-
Teodor Sigaev authored
Digging around bug #14245 I found that commit 6734a1ca missed that NOT operation is right associative in opposite to all other. This miss is resposible for tsquery parser fail on sequence of NOT operations
-
Teodor Sigaev authored
During normalization of tsquery tree it tries to simplify nested NOT operations but there it's obvioulsy missed that subsequent node could be a leaf node (value node) Bug #14245: Segfault on weird to_tsquery Reported by David Kellum.
-
Tom Lane authored
Clarify that the reason for recommending that pg_temp be put last is to prevent temporary tables from capturing unqualified table names. Per discussion with Albe Laurenz. Discussion: <A737B7A37273E048B164557ADEF4A58B5386C6E1@ntex2010i.host.magwien.gv.at>
-
Peter Eisentraut authored
-
Peter Eisentraut authored
From: Alexander Law <exclusion@gmail.com>
-
- 14 Jul, 2016 2 commits
-
-
Tom Lane authored
GiST index build could go into an infinite loop when presented with boxes (or points, circles or polygons) containing NaN component values. This happened essentially because the code assumed that x == x is true for any "double" value x; but it's not true for NaNs. The looping behavior was not the only problem though: we also attempted to sort the items using simple double comparisons. Since NaNs violate the trichotomy law, qsort could (in principle at least) get arbitrarily confused and mess up the sorting of ordinary values as well as NaNs. And we based splitting choices on box size calculations that could produce NaNs, again resulting in undesirable behavior. To fix, replace all comparisons of doubles in this logic with float8_cmp_internal, which is NaN-aware and is careful to sort NaNs consistently, higher than any non-NaN. Also rearrange the box size calculation to not produce NaNs; instead it should produce an infinity for a box with NaN on one side and not-NaN on the other. I don't by any means claim that this solves all problems with NaNs in geometric values, but it should at least make GiST index insertion work reliably with such data. It's likely that the index search side of things still needs some work, and probably regular geometric operations too. But with this patch we're laying down a convention for how such cases ought to behave. Per bug #14238 from Guang-Dih Lei. Back-patch to 9.2; the code used before commit 7f3bd868 is quite different and doesn't lock up on my simple test case, nor on the submitter's dataset. Report: <20160708151747.1426.60150@wrigleys.postgresql.org> Discussion: <28685.1468246504@sss.pgh.pa.us>
-
Magnus Hagander authored
pg_xlogdump doesn't have any other mode, so it's just confusing to include this in the error message as it indicates there might be another mode.
-
- 13 Jul, 2016 4 commits
-
-
Tom Lane authored
Test the external-sort code path in CLUSTER for two different scenarios: multiple-pass external sorting, and the best case for replacement selection, where only one run is produced, so that no merge is required. This test would have caught the bug fixed in commit 1b0fc850, at least when run with valgrind enabled. In passing, add a short-circuit test in plan_cluster_use_sort() to make dead certain that it selects sorting when enable_indexscan is off. As things stand, that would happen anyway, but it seems like good future proofing for this test. Peter Geoghegan Discussion: <CAM3SWZSgxehDkDMq1FdiW2A0Dxc79wH0hz1x-TnGy=1BXEL+nw@mail.gmail.com>
-
Stephen Frost authored
Pointed out by Alexander Law
- 12 Jul, 2016 4 commits
-
-
Peter Eisentraut authored
-
Peter Eisentraut authored
-
Tom Lane authored
Since IMPORT FOREIGN SCHEMA has an INTO clause, pl/pgsql needs to be aware of that and avoid capturing the INTO as an INTO-variables clause. This isn't hard, though it's annoying to have to make IMPORT a plpgsql keyword just for this. (Fortunately, we have the infrastructure now to make it an unreserved keyword, so at least this shouldn't break any existing pl/pgsql code.) Per report from Merlin Moncure. Back-patch to 9.5 where IMPORT FOREIGN SCHEMA was introduced. Report: <CAHyXU0wpHf2bbtKGL1gtUEFATCY86r=VKxfcACVcTMQ70mCyig@mail.gmail.com>
-
Peter Eisentraut authored
From: Alexander Law <exclusion@gmail.com>
-
- 11 Jul, 2016 5 commits
-
-
Tom Lane authored
We have, for a very long time, allowed the same subplan (same member of the PlannedStmt.subplans list) to be referenced by more than one SubPlan node; this avoids problems for cases such as subplans within an IndexScan's indxqual and indxqualorig fields. However, EXPLAIN had not gotten the memo and would print each reference as though it were an independent identical subplan. To fix, track plan_ids of subplans we've printed and don't print the same plan_id twice. Per report from Pavel Stehule. BTW: the particular case of IndexScan didn't cause visible duplication in a plain EXPLAIN, only EXPLAIN ANALYZE, because in the former case we short-circuit executor startup before the indxqual field is processed by ExecInitExpr. That seems like it could easily lead to other EXPLAIN problems in future, but it's not clear how to avoid it without breaking the "EXPLAIN a plan using hypothetical indexes" use-case. For now I've left that issue alone. Although this is a longstanding bug, it's purely cosmetic (no great harm is done by the repeat printout) and we haven't had field complaints before. So I'm hesitant to back-patch it, especially since there is some small risk of ABI problems due to the need to add a new field to ExplainState. In passing, rearrange order of fields in ExplainState to be less random, and update some obsolete comments about when/where to initialize them. Report: <CAFj8pRAimq+NK-menjt+3J4-LFoodDD8Or6=Lc_stcFD+eD4DA@mail.gmail.com>
-
Tom Lane authored
Add display of proparallel (parallel-safety) when the server is >= 9.6, and display of proacl (access privileges) for all server versions. Minor tweak of column ordering to keep related columns together. Michael Paquier Discussion: <CAB7nPqTR3Vu3xKOZOYqSm-+bSZV0kqgeGAXD6w5GLbkbfd5Q6w@mail.gmail.com>
-
Peter Eisentraut authored
-
Magnus Hagander authored
-
Magnus Hagander authored
On a standby, ThisTimelineID is always 0, so we would generate a filename in timeline 0 even for other timelines. Instead, use starttli which we have retreived from the controlfile. Report by: Francesco Canovai in bug #14230 Author: Marco Nenciarini Reviewed by: Michael Paquier and Amit Kapila
-
- 10 Jul, 2016 1 commit
-
- 09 Jul, 2016 2 commits
-
-
Tom Lane authored
Change assorted places in our Perl code that did things like system("prog $path/file"); to do it more like system('prog', "$path/file"); which is safe against spaces and other special characters in the path variable. The latter was already the prevailing style, but a few bits of code hadn't gotten this memo. Back-patch to 9.4 as relevant. Michael Paquier, Kyotaro Horiguchi Discussion: <20160704.160213.111134711.horiguchi.kyotaro@lab.ntt.co.jp>
-
Tom Lane authored
Examination of the results from anole and gharial suggests that we're only managing to track the size of one of the two stacks of IA64 machines. Some googling gave the answer: on HPUX11, the register stack is reported as a page type I don't see in pstat.h on my HPUX10 box. Let's try testing for that.
-
- 08 Jul, 2016 3 commits
-
-
Tom Lane authored
After a look at preliminary results from commit 88cf37d2, I realized it'd be a good idea to spew out the maximum depth measurement seen by check_stack_depth. So add some quick-n-dirty code to do that. Like the previous commit, this will be reverted once we've gathered a set of buildfarm runs with it.
-
Tom Lane authored
Fabien Coelho, some further wordsmithing by me
-
Tom Lane authored
Alexander Law
-