- 18 Jun, 2018 6 commits
-
-
Tom Lane authored
We want, say, 2 to be transformed as 2, not \\2 which is what the original coding produced. Perl's standard seems to be to add an RV wrapper only for hash and array SVs, so do it like that. This was missed originally because the test cases only checked what came out of a round trip back to SQL, and the strip-all-dereferences loop at the top of SV_to_JsonbValue hides the extra refs from view. As a better test, print the Perl value with Data::Dumper, like the hstore_plperlu tests do. While we can't do that in the plperl test, only plperlu, that should be good enough because this code is the same for both PLs. But also add a simplistic test for extra REFs, which we can do in both. That strip-all-dereferences behavior is now a bit dubious; it's unlike what happens for other Perl-to-SQL conversions. However, the best thing to do seems to be to leave it alone and make the other conversions act similarly. That will be done separately. Dagfinn Ilmari Mannsåker, adjusted a bit by me Discussion: https://postgr.es/m/d8jlgbq66t9.fsf@dalvik.ping.uio.no
-
Tom Lane authored
ProcedureCreate formerly threw an error if the function to be created has one argument of composite type and the function name matches some column of the composite type. This was a (very non-bulletproof) defense against creating situations where f(x) and x.f are ambiguous. But we don't really need such a defense in the wake of commit b97a3465, which allows us to deal with such situations fairly cleanly. This behavior also created a dump-and-reload hazard, since a function might be rejected if a conflicting column name had been added to the input composite type later. Hence, let's just drop the check. Discussion: https://postgr.es/m/CAOW5sYa3Wp7KozCuzjOdw6PiOYPi6D=VvRybtH2S=2C0SVmRmA@mail.gmail.com
-
Tom Lane authored
Postgres has traditionally considered the syntactic forms f(x) and x.f to be equivalent, allowing tricks such as writing a function and then using it as though it were a computed-on-demand column. However, our behavior when both interpretations are feasible left something to be desired: we always chose the column interpretation. This could lead to very surprising results, as in a recent bug report from Neil Conway. It also created a dump-and-reload hazard, since what was a function call in a dumped view could get interpreted as a column reference at reload, if a matching column name had been added to the underlying table since the view was created. What seems better, in ambiguous situations, is to prefer the choice matching the syntactic form of the reference. This seems much less astonishing in general, and it fixes the dump/reload hazard. Although this could be called a bug fix, there have been few complaints and there's some small risk of breaking applications that depend on the old behavior, so no back-patch. It does seem reasonable to slip it into v11, though. Discussion: https://postgr.es/m/CAOW5sYa3Wp7KozCuzjOdw6PiOYPi6D=VvRybtH2S=2C0SVmRmA@mail.gmail.com
-
Thomas Munro authored
On Windows, it is sometimes important for corresponding malloc() and free() calls to be made from the same DLL, since some build options can result in multiple allocators being active at the same time. For that reason we already provided PQfreemem(). This commit adds a similar function for freeing string results allocated by the pgtypes library. Author: Takayuki Tsunakawa Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8AD5D6%40G01JPEXMBYT05
-
Michael Paquier authored
When a standby's WAL receiver stops reading WAL from a WAL stream, it writes data to the current WAL segment without having priorily zero'ed the page currently written to, which can cause the WAL reader to read junk data from a past recycled segment and then it would try to get a record from it. While sanity checks in place provide most of the protection needed, in some rare circumstances, with chances increasing when a record header crosses a page boundary, then the startup process could fail violently on an allocation failure, as follows: FATAL: invalid memory alloc request size XXX This is confusing for the user and also unhelpful as this requires in the worst case a manual restart of the instance, impacting potentially the availability of the cluster, and this also makes WAL data look like it is in a corrupted state. The chances of seeing failures are higher if the connection between the standby and its root node is unstable, causing WAL pages to be written in the middle. A couple of approaches have been discussed, like zero-ing new WAL pages within the WAL receiver itself but this has the disadvantage of impacting performance of any existing instances as this breaks the sequential writes done by the WAL receiver. This commit deals with the problem with a more simple approach, which has no performance impact without reducing the detection of the problem: if a record is found with a length higher than 1GB for backends, then do not try any allocation and report a soft failure which will force the standby to retry reading WAL. It could be possible that the allocation call passes and that an unnecessary amount of memory is allocated, however follow-up checks on records would just fail, making this allocation short-lived anyway. This patch owes a great deal to Tsunakawa Takayuki for reporting the failure first, and then discussing a couple of potential approaches to the problem. Backpatch down to 9.5, which is where palloc_extended has been introduced. Reported-by: Tsunakawa Takayuki Reviewed-by: Tsunakawa Takayuki Author: Michael Paquier Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8B57AD@G01JPEXMBYT05
- 17 Jun, 2018 1 commit
-
-
Tom Lane authored
Clean up four places that result in compiler warnings when using recent gcc with this warning class enabled (as seen on buildfarm members calliphoridae, skink, and others). In all these places, this is purely cosmetic, because the shift distance could not be large enough to risk a change of sign, so there's no chance of implementation-dependent behavior. Still, it's easy enough to avoid the warning by casting the shifted value to unsigned, so let's do that. Patch HEAD only, this isn't worth a back-patch.
-
- 16 Jun, 2018 7 commits
-
-
Peter Geoghegan authored
Discussing covering indexes in a chapter that is mostly about the behavior of B-Tree operator classes is unnecessary. The CREATE INDEX documentation's handling of covering indexes seems sufficient. Discussion: https://postgr.es/m/CAH2-WzmpU=L_6VjhhOAMfoyHLr-pZd1kDc+jpa3c3a8EOmtcXA@mail.gmail.com
-
Tom Lane authored
gcc 8 has started emitting some warnings that are largely useless for our purposes, particularly since they complain about code following the project-standard coding convention that path names are assumed to be shorter than MAXPGPATH. Even if we make the effort to remove that assumption in some future release, the changes wouldn't get back-patched. Hence, just suppress these warnings, on compilers that have these switches. Backpatch to all supported branches. Discussion: https://postgr.es/m/1524563856.26306.9.camel@gunduz.org
-
Tom Lane authored
Use of strncpy with a length limit based on the source, rather than the destination, is non-idiomatic and draws warnings from gcc 8. Replace with memcpy, which does exactly the same thing in these cases, but with less chance for confusion. Backpatch to all supported branches. Discussion: https://postgr.es/m/21789.1529170195@sss.pgh.pa.us
-
Tom Lane authored
This could only cause an issue if strftime returned a ridiculously long timezone name, which seems unlikely; and it wouldn't qualify as a security problem even then, since pg_waldump (nee pg_xlogdump) is a debug tool not part of the server. But gcc 8 has started issuing warnings about it, so let's use snprintf and be safe. Backpatch to 9.3 where this code was added. Discussion: https://postgr.es/m/21789.1529170195@sss.pgh.pa.us
-
Tom Lane authored
Recent additions to ParseFuncOrColumn to make it reject non-procedure functions in CALL were neither adequate nor documented. Reorganize the code to ensure uniform results for all the cases that should be rejected. Also, use ERRCODE_WRONG_OBJECT_TYPE for this case as well as the converse case of a procedure in a non-CALL context. The original coding used ERRCODE_UNDEFINED_FUNCTION which seems wrong, and is certainly inconsistent with the adjacent wrong-kind-of-routine errors. This reorganization also causes the checks for aggregate decoration with a non-aggregate function to be made in the FUNCDETAIL_COERCION case; that they were not is a long-standing oversight. Discussion: https://postgr.es/m/14497.1529089235@sss.pgh.pa.us
-
Simon Riggs authored
Issues relate only to subtransactions that hold AccessExclusiveLocks when replayed on standby. Prior to PG10, aborting subtransactions that held an AccessExclusiveLock failed to release the lock until top level commit or abort. 49bff530 fixed that. However, 49bff530 also introduced a similar bug where subtransaction commit would fail to release an AccessExclusiveLock, leaving the lock to be removed sometimes early and sometimes late. This commit fixes that bug also. Backpatch to PG10 needed. Tested by observation. Note need for multi-node isolationtester to improve test coverage for this and other HS cases. Reported-by: Simon Riggs Author: Simon Riggs
-
Tatsuo Ishii authored
Also this commit unifies some duplicated code in makeBufFile() and BufFileOpenShared() into new function makeBufFileCommon(). Author: Antonin Houska Reviewed-By: Thomas Munro, Tatsuo Ishii Discussion: https://postgr.es/m/16139.1529049566%40localhost
-
- 15 Jun, 2018 3 commits
-
-
Alvaro Herrera authored
Commit 1eb6d652 introduced zeroed alignment bytes in the GID field of commit/abort WAL records. Fixup commit cf5a1890 later changed that representation into a regular cstring with a single terminating zero byte, but it also introduced an off-by-one mistake. Fix that. Author: Nikhil Sontakke Reported-by: Nikhil Sontakke Discussion: https://postgr.es/m/CAMGcDxey6dG1DP34_tJMoWPcp5sPJUAL4K5CayUUXLQSx2GQpA@mail.gmail.com
-
Alexander Korotkov authored
PyObject returned from PySequence_GetItem() is not released. Similar code in PLyMapping_ToJsonbValue() is correct, because according to Python documentation PyList_GetItem() and PyTuple_GetItem() return a borrowed reference while PySequence_GetItem() returns new reference. contrib/jsonb_plpython is new in PostgreSQL 11, no backpatch is needed. Author: Nikita Glukhov Discussion: https://postgr.es/m/6001af16-b242-2527-bc7e-84b8a959163b%40postgrespro.ru
-
Tatsuo Ishii authored
Memory is allocated twice for "file" and "files" variables in BufFileOpenShared(). Author: Antonin Houska Discussion: https://postgr.es/m/11329.1529045692%40localhost
-
- 14 Jun, 2018 3 commits
-
-
Alvaro Herrera authored
They already fail anyway, but prior to this patch they raise an ugly error message about a lock that cannot be acquired. This just improves the message. Author: Masahiko Sawada Reported-by: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoBZau4g4_NUf3BKNd=CdYK+xaPdtJCzvOC1TxGdTiJx_Q@mail.gmail.com Reviewed-by: Kuntal Ghosh, Alexander Korotkov, Simon Riggs, Michaël Paquier, Álvaro Herrera
-
Simon Riggs authored
GetRunningTransactionData() suggested that subxids were not worth optimizing away if overflowed, yet they have already been removed for that case. Changes to LogAccessExclusiveLock() API forgot to remove the prior comment when it was copied to LockAcquire().
-
Simon Riggs authored
32ac7a11 tried to fix a Hot Standby issue reported by Greg Stark, but in doing so caused a different bug to appear, noted by Andres Freund. Revoke the core changes from 32ac7a11, leaving in its place a minor change in code ordering and comments to explain for the future.
-
- 13 Jun, 2018 4 commits
-
-
Tom Lane authored
Fix inconsistent decisions about NOMATCH vs UNSUPPORTED result codes. If we're going to cater for partkeys that have the same expression and different collations, surely we should also support partkeys with the same expression and different opclasses. Clean up shaky handling of commuted opclauses, eg checking the wrong operator to see what its negator is. This wouldn't cause any actual bugs given a sane opclass definition, but it doesn't seem helpful to expend more code to be less correct. Improve handling of null elements in ScalarArrayOp arrays: in the "op ALL" case, we can conclude they result in an unsatisfiable clause. Minor cosmetic changes and comment improvements.
-
Tom Lane authored
"compute_hash_value" is particularly gratuitously generic, but IMO all of these ought to have names clearly related to partitioning.
-
Tom Lane authored
The previous coding saved pointers into the partitioned table's relcache entry, but then closed the relcache entry, causing those pointers to nominally become dangling. Actual trouble would be seen in the field only if a relcache flush occurred mid-query, but that's hardly out of the question. While we could fix this by copying all the data in question at query start, it seems better to just hold the relcache entry open for the whole query. While at it, improve the handling of support-function lookups: do that once per query not once per pruning test. There's still something to be desired here, in that we fail to exploit the possibility of caching data across queries in the fn_extra fields of the relcache's FmgrInfo structs, which could happen if we just used those structs in-place rather than copying them. However, combining that with the possibility of per-query lookups of cross-type comparison functions seems to require changes in the APIs of a lot of the pruning support functions, so it's too invasive to consider as part of this patch. A win would ensue only for complex partition key data types (e.g. arrays), so it may not be worth the trouble. David Rowley and Tom Lane Discussion: https://postgr.es/m/17850.1528755844@sss.pgh.pa.us
-
Alexander Korotkov authored
Documentation of word_similarity() and strict_word_similarity() functions contains some vague wordings which could confuse users. This patch makes those wordings more clear. word_similarity() was introduced in PostgreSQL 9.6, and corresponding part of documentation needs to be backpatched. Author: Bruce Momjian, Alexander Korotkov Discussion: https://postgr.es/m/20180526165648.GB12510%40momjian.us Backpatch: 9.6, where word_similarity() was introduced
-
- 12 Jun, 2018 5 commits
-
-
Andrew Dunstan authored
The .git directory might contain perl files, as hooks, for example. Since we have no control over these they should be excluded from things like our perlcritic checks. Per offline report from Mike Blackwell.
-
Andres Freund authored
When vacuum processes a relation it uses the corresponding relcache entry's relfrozenxid / relminmxid as a cutoff for when to remove tuples etc. Unfortunately for nailed relations (i.e. critical system catalogs) bugs could frequently lead to the corresponding relcache entry being stale. This set of bugs could cause actual data corruption as vacuum would potentially not remove the correct row versions, potentially reviving them at a later point. After 699bf7d0 some corruptions in this vein were prevented, but the additional error checks could also trigger spuriously. Examples of such errors are: ERROR: found xmin ... from before relfrozenxid ... and ERROR: found multixact ... from before relminmxid ... To be caused by this bug the errors have to occur on system catalog tables. The two bugs are: 1) Invalidations for nailed relations were ignored, based on the theory that the relcache entry for such tables doesn't change. Which is largely true, except for fields like relfrozenxid etc. This means that changes to relations vacuumed in other sessions weren't picked up by already existing sessions. Luckily autovacuum doesn't have particularly longrunning sessions. 2) For shared *and* nailed relations, the shared relcache init file was never invalidated while running. That means that for such tables (e.g. pg_authid, pg_database) it's not just already existing sessions that are affected, but even new connections are as well. That explains why the reports usually were about pg_authid et. al. To fix 1), revalidate the rd_rel portion of a relcache entry when invalid. This implies a bit of extra complexity to deal with bootstrapping, but it's not too bad. The fix for 2) is simpler, simply always remove both the shared and local init files. Author: Andres Freund Reviewed-By: Alvaro Herrera Discussion: https://postgr.es/m/20180525203736.crkbg36muzxrjj5e@alap3.anarazel.de https://postgr.es/m/CAMa1XUhKSJd98JW4o9StWPrfS=11bPgG+_GDMxe25TvUY4Sugg@mail.gmail.com https://postgr.es/m/CAKMFJucqbuoDRfxPDX39WhA3vJyxweRg_zDVXzncr6+5wOguWA@mail.gmail.com https://postgr.es/m/CAGewt-ujGpMLQ09gXcUFMZaZsGJC98VXHEFbF-tpPB0fB13K+A@mail.gmail.com Backpatch: 9.3-
-
Peter Eisentraut authored
-
Peter Eisentraut authored
We normally use the default line mode in examples.
-
Peter Eisentraut authored
The previous wording suggested only Slony, and there are more options available.
-
- 11 Jun, 2018 11 commits
-
-
Tom Lane authored
It might be impossible for this to cause a problem in non-debug builds, since there'd be no opportunity for the relcache entry to get recycled before the fetch. It blows up nicely with -DRELCACHE_FORCE_RELEASE plus valgrind, though. Evidently introduced by careless refactoring in commit f0e44751. Back-patch accordingly. Discussion: https://postgr.es/m/27543.1528758304@sss.pgh.pa.us
-
Michael Paquier authored
Calling an external function while a pin-lock is held is a bad idea as those are designed to be short-lived. The stress of a first commit into a large git history may contribute to that. Reported-by: Andres Freund Discussion: https://postgr.es/m/20180611164952.vmxdpdpirdtkdsz6@alap3.anarazel.de
-
Tom Lane authored
We don't need two passes if we scan child partitions before parents, as that way the children's present_parts are up to date before they're needed. I (tgl) think there's actually a bug being fixed here, for the case of an intermediate partitioned table with no direct leaf children, but haven't attempted to construct a test case to prove it. David Rowley Discussion: https://postgr.es/m/CAKJS1f-6GODRNgEtdPxCnAPme2h2hTztB6LmtfdmcYAAOE0kQg@mail.gmail.com
-
Tom Lane authored
No code changes except for a couple of new Asserts. David Rowley and Tom Lane Discussion: https://postgr.es/m/CAKJS1f-6GODRNgEtdPxCnAPme2h2hTztB6LmtfdmcYAAOE0kQg@mail.gmail.com
-
Peter Eisentraut authored
Makes it look more similar to other ones, and avoids the need for pluralization.
-
Alvaro Herrera authored
Starting with commit f0e44751, ExecConstraints was in charge of running the partition constraint; commit 19c47e7c modified that so that caller could request to skip that checking depending on some conditions, but that commit and 15ce775f together introduced a small bug there which caused ExecInsert to request skipping the constraint check but have this not be honored -- in effect doing the check twice. This could have been fixed in a very small patch, but on further analysis of the involved function and its callsites, it turns out to be simpler to give the responsibility of checking the partition constraint fully to the caller, and return ExecConstraints to its original (pre-partitioning) shape where it only checked tuple descriptor-related constraints. Each caller must do partition constraint checking on its own schedule, which is more convenient after commit 2f178441 anyway. Reported-by: David Rowley Author: David Rowley, Álvaro Herrera Reviewed-by: Amit Langote, Amit Khandekar, Simon Riggs Discussion: https://postgr.es/m/CAKJS1f8w8+awsxgea8wt7_UX8qzOQ=Tm1LD+U1fHqBAkXxkW2w@mail.gmail.com
-
Andrew Dunstan authored
-
Andrew Dunstan authored
Also add a function that centralizes the logic for locating all our perl files and use it in pgperlcritic and pgperltidy as well as the new pgperlcheck.
-
Tom Lane authored
The previous coding just ignored pruning constraints that compare a partition key to a null-valued expression. This is silly, since really what we can do there is conclude that all partitions are rejected: the pruning operator is known strict so the comparison must always fail. This also fixes the logic to not ignore constisnull for a Const comparison value. That's probably an unreachable case, since the planner would normally have simplified away a strict operator with a constant-null input. But this code has no business assuming that. David Rowley, per a gripe from me Discussion: https://postgr.es/m/26279.1528670981@sss.pgh.pa.us
-
Peter Eisentraut authored
-