- 21 Jul, 2020 4 commits
-
-
Tom Lane authored
The bug fixed in commit 72eab84a would not have occurred if initdb had a less surprising rule about which columns should be marked NOT NULL by default. Let's make that rule be strictly that the column must be fixed-width and its predecessors must be fixed-width and NOT NULL, removing the hacky and unsafe exceptions for oidvector and int2vector. Since we do still want all existing oidvector and int2vector columns to be marked NOT NULL, we have to put BKI_FORCE_NOT_NULL labels on them. But making this less magic and more documented seems like a good idea, even if it's a shade more verbose. I didn't bump catversion since the initial catalog contents are not actually changed by this patch. Note however that the contents of postgres.bki do change, and feeding an old copy of that to a new backend will produce wrong results. Discussion: https://postgr.es/m/204760.1595181800@sss.pgh.pa.us
-
Tom Lane authored
The executor checks for this error, and so does the bootstrap catalog loader, but we never checked for it in retail catalog manipulations. The folly of that has now been exposed, so let's add assertions checking it. Checking in CatalogTupleInsert[WithInfo] and CatalogTupleUpdate[WithInfo] should be enough to cover this. Back-patch to v10; the aforesaid functions didn't exist before that, and it didn't seem worth adapting the patch to the oldest branches. But given the risk of JIT crashes, I think we certainly need this as far back as v11. Pre-v13, we have to explicitly exclude pg_subscription.subslotname and pg_subscription_rel.srsublsn from the checks, since they are mismarked. (Even if we change our mind about applying BKI_FORCE_NULL in the branch tips, it doesn't seem wise to have assertions that would fire in existing databases.) Discussion: https://postgr.es/m/298837.1595196283@sss.pgh.pa.us
-
Michael Paquier authored
This corrects and simplifies $subject in a number of ways: - Remove from the completion the pre-9.0 grammar still supported for compatibility purposes. This simplifies the code, and allows to extend it more easily with new patterns. - Add completion for the options of FORMAT within a WITH clause. - Complete WHERE and WITH clauses correctly depending on if TO or FROM are used, WHERE being only available with COPY FROM. Author: Vignesh C, Michael Paquier Reviewed-by: Ahsan Hadi Discussion: https://postgr.es/m/CALDaNm3zWr=OmxeNqOqfT=uZTSdam_j-gkX94CL8eTNfgUtf6A@mail.gmail.com
-
Tom Lane authored
Many situations where the offset is infinity were not handled sanely. We should generally allow the val versus base +/- offset comparison to proceed according to the normal rules of IEEE arithmetic; however, we must do something special for the corner cases where base +/- offset would produce NaN due to subtracting two like-signed infinities. That corresponds to asking which values infinitely precede +inf or infinitely follow -inf, which should certainly be true of any finite value or of the opposite-signed infinity. After some discussion it seems that the best decision is to make it true of the same-signed infinity as well, ie, just return constant TRUE if the calculation would produce a NaN. (We could write this with a bit less code by subtracting anyway, and then checking for a NaN result. However, I prefer this formulation because it'll be easier to transpose into numeric.c.) Although this seems like clearly a bug fix with respect to finite values, it is less obviously correct for infinite values. Between that and the fact that the whole issue only arises for very strange window specifications (e.g. RANGE BETWEEN 'inf' PRECEDING AND 'inf' PRECEDING), I'll desist from back-patching. Noted by Dean Rasheed. Discussion: https://postgr.es/m/3393130.1594925893@sss.pgh.pa.us
-
- 20 Jul, 2020 9 commits
-
-
Tom Lane authored
This is more consistent with the IEEE 754 spec and our treatment of NaNs elsewhere; in particular, the case has always acted that way in "numeric" arithmetic. Noted by Dean Rasheed. Discussion: https://postgr.es/m/3421746.1594927785@sss.pgh.pa.us
-
Peter Geoghegan authored
Strengthen the LockBuffer() assertion that verifies BufferIsValid() by making it verify BufferIsPinned() instead. Do the same in nearby related functions. There is probably not much chance that anybody will try to lock a buffer that is not already pinned, but we might as well make sure of that.
-
Tom Lane authored
The code has always set this column to NULL when it's not valid, but the catalog header's description failed to reflect that, as did the SGML docs, as did some of the code. To prevent future coding errors of the same ilk, let's hide the field from C code as though it were variable-length (which, in a sense, it is). As with commit 72eab84a, we can only fix this cleanly in HEAD and v13; the problem extends further back but we'll need some klugery in the released branches. Discussion: https://postgr.es/m/367660.1595202498@sss.pgh.pa.us
-
Tom Lane authored
Commit b9c130a1 failed to apply the publisher-to-subscriber column mapping while checking which columns were updated. Perhaps less significantly, it didn't exclude dropped columns either. This could result in an incorrect updated-columns bitmap and thus wrong decisions about whether to fire column-specific triggers on the subscriber while applying updates. In HEAD (since commit 9de77b54), it could also result in accesses off the end of the colstatus array, as detected by buildfarm member skink. Fix the logic, and adjust 003_constraints.pl so that the problem is exposed in unpatched code. In HEAD, also add some assertions to check that we don't access off the ends of these newly variable-sized arrays. Back-patch to v10, as b9c130a1 was. Discussion: https://postgr.es/m/CAH2-Wz=79hKQ4++c5A060RYbjTHgiYTHz=fw6mptCtgghH2gJA@mail.gmail.com
-
Alexander Korotkov authored
All functions provided by this extension are PARALLEL SAFE. Discussion: https://postgr.es/m/AM5PR0901MB1587E47B1ACF23C6089DFCA3FD9B0%40AM5PR0901MB1587.eurprd09.prod.outlook.com Author: Steven Winfield
-
Fujii Masao authored
max_slot_wal_keep_size that was added in v13 and wal_keep_segments are the GUC parameters to specify how much WAL files to retain for the standby servers. While max_slot_wal_keep_size accepts the number of bytes of WAL files, wal_keep_segments accepts the number of WAL files. This difference of setting units between those similar parameters could be confusing to users. To alleviate this situation, this commit renames wal_keep_segments to wal_keep_size, and make users specify the WAL size in it instead of the number of WAL files. There was also the idea to rename max_slot_wal_keep_size to max_slot_wal_keep_segments, in the discussion. But we have been moving away from measuring in segments, for example, checkpoint_segments was replaced by max_wal_size. So we concluded to rename wal_keep_segments to wal_keep_size. Back-patch to v13 where max_slot_wal_keep_size was added. Author: Fujii Masao Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, David Steele Discussion: https://postgr.es/m/574b4ea3-e0f9-b175-ead2-ebea7faea855@oss.nttdata.com
-
Amit Kapila authored
The logical decoding infrastructure needs to know which top-level transaction the subxact belongs to, in order to decode all the changes. Until now that might be delayed until commit, due to the caching (GPROC_MAX_CACHED_SUBXIDS), preventing features requiring incremental decoding. So we also write the assignment info into WAL immediately, as part of the next WAL record (to minimize overhead) only when wal_level=logical. We can not remove the existing XLOG_XACT_ASSIGNMENT WAL as that is required for avoiding overflow in the hot standby snapshot. Bump XLOG_PAGE_MAGIC, since this introduces XLR_BLOCK_ID_TOPLEVEL_XID. Author: Tomas Vondra, Dilip Kumar, Amit Kapila Reviewed-by: Amit Kapila Tested-by: Neha Sharma and Mahendra Singh Thalor Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com
-
Fujii Masao authored
There was no easy way to find how many times generic and custom plans have been executed for a prepared statement. This commit exposes those numbers of times in pg_prepared_statements view. Author: Atsushi Torikoshi, Kyotaro Horiguchi Reviewed-by: Tatsuro Yamada, Masahiro Ikeda, Fujii Masao Discussion: https://postgr.es/m/CACZ0uYHZ4M=NZpofH6JuPHeX=__5xcDELF8hT8_2T+R55w4RQw@mail.gmail.com
-
Amit Kapila authored
Author: Vignesh C Reviewed-by: James Coleman Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/CALDaNm0WjZqRvdeL59ZfYH0o4mLbKQ23jm-bnjXcFzgpANx55g@mail.gmail.com
-
- 19 Jul, 2020 4 commits
-
-
Peter Geoghegan authored
Valgrind builds with assertions enabled sometimes perform a theoretically unsafe page access inside an assertion in heapam_tuple_lock(). This happened when the eval-plan-qual isolation test ran one of the permutations added by commit a2418f9e. Avoid complaints from Valgrind by moving the assertion ever so slightly. This is minor cleanup for commit 1e0dfd16, which added Valgrind buffer access instrumentation. No backpatch, since this only happens within an assertion, and seems very unlikely to cause any real problems even with assert-enabled builds.
-
Peter Geoghegan authored
Make PinBuffer() mark buffers as defined to Valgrind unconditionally, including when the buffer header spinlock must be acquired. Failure to handle that case could lead to false positive reports from Valgrind. This theoretically creates a risk that we'll mark buffers defined even when external callers don't end up with a buffer pin. That seems perfectly acceptable, though, since in general we make no guarantees about buffers that are unsafe to access being reliably marked as unsafe. Oversight in commit 1e0dfd16, which added valgrind buffer access instrumentation.
-
Tom Lane authored
Due to the layout of this catalog, subslotname has to be explicitly marked BKI_FORCE_NULL, else initdb will default to the assumption that it's non-nullable. Since, in fact, CREATE/ALTER SUBSCRIPTION will store null values there, the existing marking is just wrong, and has been since this catalog was invented. We haven't noticed because not much in the system actually depends on attnotnull being truthful. However, JIT'ed tuple deconstruction does depend on that in some cases, allowing crashes or wrong answers in queries that inspect pg_subscription. Commit 9de77b54 quite accidentally exposed this on the buildfarm members that force JIT activation. Back-patch to v13. The problem goes further back, but we cannot force initdb in released branches, so some klugier solution will be needed there. Before working on that, push this simple fix to try to get the buildfarm back to green. Discussion: https://postgr.es/m/4118109.1595096139@sss.pgh.pa.us
-
Peter Eisentraut authored
This avoids deprecation warnings from newer OpenSSL versions (3.0.0 in particular). Discussion: https://www.postgresql.org/message-id/flat/FEF81714-D479-4512-839B-C769D2605F8A%40yesql.se
-
- 18 Jul, 2020 8 commits
-
-
Tom Lane authored
This header hasn't changed recently, so the fact that it now fails headerscheck/cpluspluscheck testing must be due to changes in what it includes. Probably f2191679 is to blame, but I didn't try to verify that. Discussion: https://postgr.es/m/3699703.1595016554@sss.pgh.pa.us
-
Tom Lane authored
This patch adds a "binary" option to CREATE/ALTER SUBSCRIPTION. When that's set, the publisher will send data using the data type's typsend function if any, rather than typoutput. This is generally faster, if slightly less robust. As committed, we won't try to transfer user-defined array or composite types in binary, for fear that type OIDs won't match at the subscriber. This might be changed later, but it seems like fit material for a follow-on patch. Dave Cramer, reviewed by Daniel Gustafsson, Petr Jelinek, and others; adjusted some by me Discussion: https://postgr.es/m/CADK3HH+R3xMn=8t3Ct+uD+qJ1KD=Hbif5NFMJ+d5DkoCzp6Vgw@mail.gmail.com
-
Michael Paquier authored
This updates some URLs that are redirections, mostly to an equivalent using https. One URL referring to generalized partial indexes was outdated. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20200717.121308.1369606287593685396.horikyota.ntt@gmail.com Backpatch-through: 9.5
-
Amit Kapila authored
Author: Dave Cramer Reviewed-by: David G. Johnston Discussion: https://postgr.es/m/CADK3HHL8do4Fp1bsymgNasx375njV3AR7zY3UgYwzbL_Dx-n2Q@mail.gmail.com
-
Amit Kapila authored
Author: Dave Cramer Reviewed-by: David G. Johnston Discussion: https://postgr.es/m/CADK3HHL8do4Fp1bsymgNasx375njV3AR7zY3UgYwzbL_Dx-n2Q@mail.gmail.com
-
Michael Paquier authored
The WHERE clause introduced by 31f38174 was not described. While on it, split the grammar of \copy FROM and TO into two distinct parts for clarity as they support different set of options. Author: Vignesh C Discussion: https://postgr.es/m/CALDaNm3zWr=OmxeNqOqfT=uZTSdam_j-gkX94CL8eTNfgUtf6A@mail.gmail.com Backpatch-through: 12
-
Peter Geoghegan authored
The term "hash_mem" will take on new significance when pending work to add a new hash_mem_multiplier GUC is committed. Rename a local variable that happens to have been called hash_mem now to avoid confusion.
-
Peter Geoghegan authored
Teach Valgrind memcheck to maintain the "defined-ness" of each shared buffer based on whether the backend holds at least one pin at the point it is accessed by access method code. Bugs like the one fixed by commit b0229f26 can be detected using this new instrumentation. Note that backends running with Valgrind naturally have their own independent ideas about whether any given byte in shared memory is safe or unsafe to access. There is no risk that concurrent access by multiple backends to the same shared memory will confuse Valgrind's instrumentation, because everything already works at the process level (or at the memory mapping level, if you prefer). Author: Álvaro Herrera, Peter Geoghegan Reviewed-By: Anastasia Lubennikova Discussion: https://postgr.es/m/20150723195349.GW5596@postgresql.org Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com
-
- 17 Jul, 2020 9 commits
-
-
Tom Lane authored
pg_dump produces custom-format archive files that lack data offsets when it is unable to seek its output. Up to now that's been a hazard for pg_restore. But if pg_restore is able to seek in the archive file, there is no reason to throw up our hands when asked to restore data blocks out of order. Instead, whenever we are searching for a data block, record the locations of the blocks we passed over (that is, fill in the missing data-offset fields in our in-memory copy of the TOC data). Then, when we hit a case that requires going backwards, we can just seek back. Also track the furthest point that we've searched to, and seek back to there when beginning a search for a new data block. This avoids possible O(N^2) time consumption, by ensuring that each data block is examined at most twice. (On Unix systems, that's at most twice per parallel-restore job; but since Windows uses threads here, the threads can share block location knowledge, reducing the amount of duplicated work.) We can also improve the code a bit by using fseeko() to skip over data blocks during the search. This is all of some use even in simple restores, but it's really significant for parallel pg_restore. In that case, we require seekability of the input already, and we will very probably need to do out-of-order restores. Back-patch to v12, as this fixes a regression introduced by commit 548e5097. Before that, parallel restore avoided requesting out-of-order restores, so it would work on a data-offset-less archive. Now it will again. Ideally this patch would include some test coverage, but there are other open bugs that need to be fixed before we can extend our coverage of parallel restore very much. Plan to revisit that later. David Gilman and Tom Lane; reviewed by Justin Pryzby Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
-
Tom Lane authored
We do not really need to track the file position by hand. We were already relying on ftello() whenever the archive file is seekable, while if it's not seekable we don't need the file position info anyway because we're not going to be able to re-write the TOC. Moreover, that tracking was buggy since it failed to account for the effects of fseeko(). Somewhat remarkably, that seems not to have made for any live bugs up to now. We could fix the oversights, but it seems better to just get rid of the whole error-prone mess. In itself this is merely code cleanup. However, it's necessary infrastructure for an upcoming bug-fix patch (because that code *does* need valid file position after fseeko). The bug fix needs to go back as far as v12; hence, back-patch that far. Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
-
Peter Geoghegan authored
There is no advantage to attempting deduplication for a unique index during CREATE INDEX, since there cannot possibly be any duplicates. Doing so wastes cycles due to unnecessary copying. Make sure that we avoid it consistently. We already avoided unique index deduplication in the case where there were some spool2 tuples to merge. That didn't account for the fact that spool2 is removed early/unset in the common case where it has no tuples that need to be merged (i.e. it failed to account for the "spool2 turns out to be unnecessary" optimization in _bt_spools_heapscan()). Oversight in commit 0d861bbb, which added nbtree deduplication Backpatch: 13-, where nbtree deduplication was introduced.
-
Tom Lane authored
We had two occurrences of "Mitteleuropäische Zeit" in Europe.txt, though the corresponding entries in Default were spelled "Mitteleuropaeische Zeit". Standardize on the latter spelling to avoid questions of which encoding to use. While here, correct a couple of other trivial inconsistencies between the Default file and the supposedly-matching entries in the *.txt files, as exposed by some checking with comm(1). Also, add BDST to the Europe.txt file; it previously was only listed in Default. None of this has any direct functional effect. Per complaint from Christoph Berg. As usual for timezone data patches, apply to all branches. Discussion: https://postgr.es/m/20200716100743.GE3534683@msg.df7cb.de
-
Peter Eisentraut authored
-
Peter Eisentraut authored
-
Amit Kapila authored
Commit 1e53fe0e has unified the usage of the config-file reload flag by using the same signal handler function for the SIGHUP signal at many places in the code. By mistake, it used the wrong SIGNAL in apply launcher process for the SIGHUP signal handler function. Author: Bharath Rupireddy Reviewed-by: Dilip Kumar Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/CALj2ACVzHCRnS20bOiEHaLtP5PVBENZQn4khdsSJQgOv_GM-LA@mail.gmail.com
-
Thomas Munro authored
This representation saves 8 bytes per tuple compared to HeapTuple, and avoids the need to allocate, copy and free on the receiving side. Gather can emit the returned MinimalTuple directly, but GatherMerge now needs to make an explicit copy because it buffers multiple tuples at a time. That should be no worse than before. Reviewed-by: Soumyadeep Chakraborty <soumyadeep2007@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2B8T_ggoUTAE-U%3DA%2BOcPc4%3DB0nPPHcSfffuQhvXXjML6w%40mail.gmail.com
-
Thomas Munro authored
This allows the huge page size to be set explicitly. The default is 0, meaning it will use the system default, as before. Author: Odin Ugedal <odin@ugedal.com> Discussion: https://postgr.es/m/20200608154639.20254-1-odin%40ugedal.com
-
- 16 Jul, 2020 2 commits
-
-
Andrew Dunstan authored
Windows has junction points which function as symbolic links for directories. This patch introduces a new function TestLib::dir_symlink() which creates a junction point on Windows and a standard Unix type symbolic link elsewhere. The function TestLib::perl2host is also modified, first to use cygpath where it's available (e.g. msys2) and second to allow it to succeed if the gandparent directory exists but the parent does not. Given these changes the only symlink tests that need to be skipped on Windows are those related to permissions or to use of readlink. The relevant tests for pg_basebackup and pg_rewind are therefore adjusted accordingly. Andrew Dunstan, reviewed by Peter Eisentraut and Michael Paquier. Discussion: https://postgr.es/m/c50a646c-d9bb-7c62-a4bf-8256ff6ff338@2ndquadrant.com
-
Michael Paquier authored
pg_test_fsync has always opened files using the text mode on Windows, as this is the default mode used if not enforced by _setmode(). This fixes a failure when running pg_test_fsync down to 12 because O_DSYNC and the text mode are not able to work together nicely. We fixed the handling of O_DSYNC in 12~ for the tool by switching to the concurrent-safe version of fopen() in src/port/ with 0ba06e0b. And 40cfe860, by enforcing the text mode for compatibility reasons if O_TEXT or O_BINARY are not specified by the caller, broke pg_test_fsync. For all versions, this avoids any translation overhead, and pg_test_fsync should test binary writes, so it is a gain in all cases. Note that O_DSYNC is still not handled correctly in ~11, leading to pg_test_fsync to show insanely high numbers for open_datasync() (using this property it is easy to notice that the binary mode is much faster). This would require a backpatch of 0ba06e0b and 40cfe860, which could potentially break existing applications, so this is left out. There are no TAP tests for this tool yet, so I have checked all builds manually using MSVC. We could invent a new option to run a single transaction instead of using a duration of 1s to make the tests a maximum short, but this is left as future work. Thanks to Bruce Momjian for the discussion. Reported-by: Jeff Janes Author: Michael Paquier Discussion: https://postgr.es/m/16526-279ded30a230d275@postgresql.org Backpatch-through: 9.5
-
- 15 Jul, 2020 4 commits
-
-
Peter Eisentraut authored
Similar to daa9fe8a, instead of repeating the almost same large query in each version branch, use one query and add a few columns to the SELECT list depending on the version. This saves a lot of duplication. Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr> Discussion: https://www.postgresql.org/message-id/flat/6594334b-40fd-14f1-6bc5-877afa3feed5@2ndquadrant.com
-
Michael Paquier authored
When working with an online source cluster, pg_rewind gets a list of all the files in the source data directory using a WITH RECURSIVE query, returning a NULL result for a file's metadata if it gets removed between the moment it is listed in a directory and the moment its metadata is obtained with pg_stat_file() (say a recycled WAL segment). The query result was processed in such a way that for each tuple we checked only that the first file's metadata was NULL. This could have two consequences, both resulting in a failure of the rewind: - If the first tuple referred to a removed file, all files from the source would be ignored. - Any file actually missing would not be considered as such. While on it, rework slightly the code so as no values are saved if we know that a file is going to be skipped. Issue introduced by b36805f3, so backpatch down to 9.5. Author: Justin Pryzby, Michael Paquier Reviewed-by: Daniel Gustafsson, Masahiko Sawada Discussion: https://postgr.es/m/20200713061010.GC23581@telsasoft.com Backpatch-through: 9.5
-
Michael Paquier authored
One change for getObjectIdentity() has been missed in 2a10fdc4, causing the module to not compile properly. This was actually the only problem, and it happens that it is easy enough to check the compilation of the module on Debian after installing libselinux1-dev. Per buildfarm member rhinoceros.
-
Michael Paquier authored
When using the following functions, users could see various types of errors of the type "cache lookup failed for OID XXX" with elog(), that can only be used for internal errors: * pg_describe_object() * pg_identify_object() * pg_identify_object_as_address() The set of APIs managing object addresses for all object types are made smarter by gaining a new argument "missing_ok" that allows any caller to control if an error is raised or not on an undefined object. The SQL functions listed above are changed to handle the case where an object is missing. Regression tests are added for all object types for the cases where these are undefined. Before this commit, these cases failed with cache lookup errors, and now they basically return NULL (minus the name of the object type requested). Author: Michael Paquier Reviewed-by: Aleksander Alekseev, Dmitry Dolgov, Daniel Gustafsson, Álvaro Herrera, Kyotaro Horiguchi Discussion: https://postgr.es/m/CAB7nPqSZxrSmdHK-rny7z8mi=EAFXJ5J-0RbzDw6aus=wB5azQ@mail.gmail.com
-