- 17 Jul, 2020 3 commits
-
-
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
-
- 14 Jul, 2020 7 commits
-
-
Tom Lane authored
reparameterize_path_by_child() failed to reparameterize BitmapAnd and BitmapOr paths. This matters only if such a path is chosen as the inside of a nestloop partition-wise join, where we have to pass in parameters from the outside of the nestloop. If that did happen, we generated a bad plan that would likely lead to crashes at execution. This is not entirely reparameterize_path_by_child()'s fault though; it's the victim of an ancient decision (my ancient decision, I think) to not bother filling in param_info in BitmapAnd/Or path nodes. That caused the function to believe that such nodes and their children contain no parameter references and so need not be processed. In hindsight that decision looks pretty penny-wise and pound-foolish: while it saves a few cycles during path node setup, we do commonly need the information later. In particular, by reversing the decision and requiring valid param_info data in all nodes of a bitmap path tree, we can get rid of indxpath.c's get_bitmap_tree_required_outer() function, which computed the data on-demand. It's not unlikely that that nets out as a savings of cycles in many scenarios. A couple of other things in indxpath.c can be simplified as well. While here, get rid of some cases in reparameterize_path_by_child() that are visibly dead or useless, given that we only care about reparameterizing paths that can be on the inside of a parameterized nestloop. This case reminds one of the maxim that untested code probably does not work, so I'm unwilling to leave unreachable code in this function. (I did leave the T_Gather case in place even though it's not reached in the regression tests. It's not very clear to me when the planner might prefer to put Gather below rather than above a nestloop, but at least in principle the case might be interesting.) Per bug #16536, originally from Arne Roland but with a test case by Andrew Gierth. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/16536-2213ee0b3aad41fd@postgresql.org
-
Peter Eisentraut authored
Three groups of issues needed to be addressed: load_external_function() and related functions returned PGFunction, even though not necessarily all callers are looking for a function of type PGFunction. Since these functions are really just wrappers around dlsym(), change to return void * just like dlsym(). In dynahash.c, we are using strlcpy() where a function with a signature like memcpy() is expected. This should be safe, as the new comment there explains, but the cast needs to be augmented to avoid the warning. In PL/Python, methods all need to be cast to PyCFunction, per Python API, but this now runs afoul of these warnings. (This issue also exists in core CPython.) To fix the second and third case, we add a new type pg_funcptr_t that is defined specifically so that gcc accepts it as a special function pointer that can be cast to any other function pointer without the warning. Also add -Wcast-function-type to the standard warning flags, subject to configure check. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/1e97628e-6447-b4fd-e230-d109cec2d584%402ndquadrant.com
-
David Rowley authored
Removing the unused 'miinfo' parameter has been raised a couple of times now. It was decided in the 2nd discussion below that we're going to leave it alone. It seems like it might be useful to add a comment to mention this fact so that nobody wastes any time in the future proposing its removal again. Discussion: https://postgr.es/m/CAApHDvpCf-qR5HC1rXskUM4ToV+3YDb4-n1meY=vpAHsRS_1PA@mail.gmail.com Discussion: https://postgr.es/m/CAE9k0P%3DFvcDswnSVtRpSyZMpcAWC%3DGp%3DifZ0HdfPaRQ%3D__LBtw%40mail.gmail.com
-
David Rowley authored
An ALTER TABLE to validate a foreign key in which another subcommand already caused a pending table rewrite could fail due to ALTER TABLE attempting to validate the foreign key before the actual table rewrite takes place. This situation could result in an error such as: ERROR: could not read block 0 in file "base/nnnnn/nnnnn": read only 0 of 8192 bytes The failure here was due to the SPI call which validates the foreign key trying to access an index which is yet to be rebuilt. Similarly, we also incorrectly tried to validate CHECK constraints before the heap had been rewritten. The fix for both is to delay constraint validation until phase 3, after the table has been rewritten. For CHECK constraints this means a slight behavioral change. Previously ALTER TABLE VALIDATE CONSTRAINT on inheritance tables would be validated from the bottom up. This was different from the order of evaluation when a new CHECK constraint was added. The changes made here aligns the VALIDATE CONSTRAINT evaluation order for inheritance tables to be the same as ADD CONSTRAINT, which is generally top-down. Reported-by: Nazli Ugur Koyluoglu, using SQLancer Discussion: https://postgr.es/m/CAApHDvp%3DZXv8wiRyk_0rWr00skhGkt8vXDrHJYXRMft3TjkxCA%40mail.gmail.com Backpatch-through: 9.5 (all supported versions)
-
Michael Paquier authored
The following header files missed the shot: - jsonfuncs.h, as of ce0425b1. - jsonapi.h, as of beb46990. - llvmjit_emit.h as of 7ec0d80c. - partdesc.h, as of 1bb5e782. Author: Jesse Zhang Discussion: https://postgr.es/m/CAGf+fX4-8xULEOz09DE2dZGjT+q8VJ--rqfTpvcFwc+A4fc-3Q@mail.gmail.com
-
Michael Paquier authored
Incorrect function names were referenced. As this fixes some portions of tableam.h, that is mentioned in the docs as something to look at when implementing a table AM, backpatch down to 12 where this has been introduced. Author: Hironobu Suzuki Discussion: https://postgr.es/m/8fe6d672-28dd-3f1d-7aed-ac2f6d599d3f@interdb.jp Backpatch-through: 12
-
Tom Lane authored
The qual pushdown logic assumed that all Vars in a restriction clause must be Vars referencing subquery outputs; but since we introduced LATERAL, it's possible for such a Var to be a lateral reference instead. This led to an assertion failure in debug builds. In a non-debug build, there might be no ill effects (if qual_is_pushdown_safe decided the qual was unsafe anyway), or we could get failures later due to construction of an invalid plan. I've not gone to much length to characterize the possible failures, but at least segfaults in the executor have been observed. Given that this has been busted since 9.3 and it took this long for anybody to notice, I judge that the case isn't worth going to great lengths to optimize. Hence, fix by just teaching qual_is_pushdown_safe that such quals are unsafe to push down, matching the previous behavior when it accidentally didn't fail. Per report from Tom Ellis. Back-patch to all supported branches. Discussion: https://postgr.es/m/20200713175124.GQ8220@cloudinit-builder
-
- 13 Jul, 2020 7 commits
-
-
Alvaro Herrera authored
Remove previous hack in KeepLogSeg that added a case to deal with a (badly represented) invalid segment number. This was added for the sake of GetWALAvailability. But it's not needed if in that function we initialize the segment number to be retreated to the currently being written segment, so do that instead. Per valgrind-running buildfarm member skink, and some sparc64 animals. Discussion: https://postgr.es/m/1724648.1594230917@sss.pgh.pa.us
-
Tom Lane authored
GSS-related resources should be cleaned up in pqDropConnection, not freePGconn, else the wrong things happen when resetting a connection or trying to switch to a different server. It's also critical to reset conn->gssenc there. During connection setup, initialize conn->try_gss at the correct place, else switching to a different server won't work right. Remove now-redundant cleanup of GSS resources around one (and, for some reason, only one) pqDropConnection call in connectDBStart. Per report from Kyotaro Horiguchi that psql would freeze up, rather than successfully resetting a GSS-encrypted connection after a server restart. This is YA oversight in commit b0b39f72, so back-patch to v12. Discussion: https://postgr.es/m/20200710.173803.435804731896516388.horikyota.ntt@gmail.com
-
Alexander Korotkov authored
* Strategy number and purpose are essential information for opfamily operator. So, show those columns in non-verbose output. * "Left/right arg type" \dAp column names are confusing, because those type don't necessary match to function arguments. Rename them to "Registered left/right type". * Replace manual assembling of operator/procedure names with casts to regoperator/regprocedure. * Add schema-qualification for pg_catalog functions and tables. Reported-by: Peter Eisentraut, Tom Lane Reviewed-by: Tom Lane Discussion: https://postgr.es/m/2edc7b27-031f-b2b6-0db2-864241c91cb9%402ndquadrant.com Backpatch-through: 13
-
Jeff Davis authored
This is a replacement for 4cad2534. Instead of projecting all tuples going into a HashAgg, only remove unnecessary attributes when actually spilling. This avoids the regression for the in-memory case. Discussion: https://postgr.es/m/a2fb7dfeb4f50aa0a123e42151ee3013933cb802.camel%40j-davis.com Backpatch-through: 13
-
Jeff Davis authored
This reverts commit 4cad2534 due to a performance regression. It will be replaced by a new approach in an upcoming commit. Reported-by: Andres Freund Discussion: https://postgr.es/m/20200614181418.mx4bvljmfkkhoqzl@alap3.anarazel.de Backpatch-through: 13
-
Amit Kapila authored
The stats with this commit was available only for WALSenders, however, users might want to see for backends doing logical decoding via SQL API. Then, users might want to reset and access these stats across server restart which was not possible with the current patch. List of commits reverted: caa3c424 Don't call elog() while holding spinlock. e641b2a9 Doc: Update the documentation for spilled transaction statistics. 5883f5fe Fix unportable printf format introduced in commit 9290ad19. 9290ad19 Track statistics for spilling of changes from ReorderBuffer. Additionaly, remove the release notes entry for this feature. Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
-
Michael Paquier authored
Switching the regression tests to use tstzrange() has proved to not be a good idea for environments where the timestamp precision is low, as internal range checks exclude the upper bound. So, if the commit timestamp of a transaction matched with now() from the next query, the test would fail. This changes to use two bound checks instead of the range function, where the upper bound is inclusive. Per buildfarm member jacana. Discussion: https://postgr.es/m/20200712122507.GD21680@paquier.xyz
-
- 12 Jul, 2020 2 commits
-
-
Michael Paquier authored
Replication origins created by regression tests should have names starting with "regress_", and the test introduced in b1e48bbe for commit timestamps did not do that. Per buildfarm member longfin. Discussion: https://postgr.es/m/20200712122507.GD21680@paquier.xyz
-
Michael Paquier authored
This includes two changes: - Addition of a new function pg_xact_commit_timestamp_origin() able, for a given transaction ID, to return the commit timestamp and replication origin of this transaction. An equivalent function existed in pglogical. - Addition of the replication origin to pg_last_committed_xact(). The commit timestamp manager includes already APIs able to return the replication origin of a transaction on top of its commit timestamp, but the code paths for replication origins were never stressed as those functions have never looked for a replication origin, and the SQL functions available have never included this information since their introduction in 73c986ad. While on it, refactor a test of modules/commit_ts/ to use tstzrange() to check that a transaction timestamp is within the wanted range, making the test a bit easier to read. Bump catalog version. Author: Movead Li Reviewed-by: Madan Kumar, Michael Paquier Discussion: https://postgr.es/m/2020051116430836450630@highgo.ca
-
- 11 Jul, 2020 6 commits
-
-
Tom Lane authored
The raw_buf and line_buf buffers aren't used when reading binary format, so skip allocating them. raw_buf is 64K so that seems like a worthwhile savings. An unused line_buf only wastes 1K, but as long as we're checking it's free to avoid allocating that too. Bharath Rupireddy, tweaked a bit by me Discussion: https://postgr.es/m/CALj2ACXcCKaGPY0whowqrJ4OPJvDnTssgpGCzvuFQu5z0CXb-g@mail.gmail.com
-
Tom Lane authored
Parallel pg_restore has always supposed that ACL items for different objects are independent and can be restored in parallel without conflicts. However, there is one case where this fails: because REVOKE on a table is defined to also revoke the privilege(s) at column level, we can't restore per-column ACLs till after we restore any table-level privileges on their table. Failure to honor this restriction can lead to "tuple concurrently updated" errors during parallel restore, or even to the per-column ACLs silently disappearing because the table-level REVOKE is executed afterwards. To fix, add a dependency from each column-level ACL item to its table's ACL item, if there is one. Note that this doesn't fix the hazard for pre-existing archive files, only for ones made with a corrected pg_dump. Given that the bug's been there quite awhile without field reports, I think this is acceptable. This requires changing the API of pg_dump's dumpACL() function. To keep its argument list from getting even longer, I removed the "CatalogId objCatId" argument, which has been unused for ages. Per report from Justin Pryzby. Back-patch to all supported branches. Discussion: https://postgr.es/m/20200706050129.GW4107@telsasoft.com
-
Peter Eisentraut authored
Reported-by: Lee Dong Wook <sh95119@gmail.com>
-
Michael Paquier authored
"relkind" normally refers to the char field from pg_class. However, in the parse nodes AlterTableStmt and CreateTableAsStmt, "relkind" was used for a field of type enum ObjectType, that could refer to other object types than those possible for a relkind. Such fields being usually named "objtype", switch the name in both structures to make things more consistent. Note that this led to some confusion in functions that also operate on a RangeTableEntry object, which also has a field named "relkind". This naming goes back to commit 09d4e96d, where only OBJECT_TABLE and OBJECT_INDEX were used. This got extended later to use as well OBJECT_TYPE with e440e12c, not really a relation kind. Author: Mark Dilger Reviewed-by: Daniel Gustafsson, Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/609181AE-E399-47C7-9221-856E0F96BF93@enterprisedb.com
-
Alexander Korotkov authored
SQL standard doesn't define numeric Inf or NaN values. It appears even more ridiculous to support then in jsonpath assuming JSON doesn't support these values as well. This commit forbids returning NaN from .double(), which was previously allowed. NaN can't be result of inner-jsonpath computation over non-NaNs. So, we can not expect NaN in the jsonpath output. Reported-by: Tom Lane Discussion: https://postgr.es/m/203949.1591879542%40sss.pgh.pa.us Author: Alexander Korotkov Reviewed-by: Tom Lane Backpatch-through: 12
-
Alexander Korotkov authored
When jsonpath .double() method detects that numeric or string can't be converted to double precision, it throws an error. This commit makes these errors explicitly express the reason of failure. Discussion: https://postgr.es/m/CAPpHfdtqJtiSXkP7tOXez18NxhLUH_-75bL8%3DOce4Ki%2Bbv7V6Q%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Tom Lane Backpatch-through: 12
-
- 10 Jul, 2020 5 commits
-
-
Tom Lane authored
Re-point comp.ai.genetic FAQ link to a more stable address. Remove stale links to AIX documentation; we don't really need to tell AIX users how to use their systems. Remove stale links to HP documentation about SSL. We've had to update those twice before, making it increasingly obvious that HP does not intend them to be stable landing points. They're not particularly authoritative, either. (This change effectively reverts bbd3bdba.) Daniel Gustafsson and Álvaro Herrera, per a gripe from Kyotaro Horiguchi. Back-patch, since these links are just as dead in the back branches. Discussion: https://postgr.es/m/20200709.161226.204639179120026914.horikyota.ntt@gmail.com
-
Peter Eisentraut authored
Most of them already have this, but some were missing. Author: Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://www.postgresql.org/message-id/flat/87o8pco34z.fsf%40wibble.ilmari.org
-
Michael Paquier authored
This reverts commit 2b2a070d, that moved the reset of path "testtablespace" used by the regression tests as a path for tablespaces (via --outputdir) from pg_regress to the MSVC script vcregress.pl, as this broke the behavior added by ce5d3424 to be able to safely run the regression test suite with an administrative Windows account using a restricted token. Note that before 2b2a070d, the code doing the reset in pg_regress.c included a comment telling that we had better move that out to a different place, leading to the mistake done in 2b2a070d. Fix this comment, and document instead that we had better never remove this code, for the sake of not breaking again the behavior we expect on Windows. Thanks to Thomas Munro and Andrew Dunstan for the discussion. Discussion: https://postgr.es/m/6d9eee97-54c8-e14a-48f7-3194e712f54f@2ndQuadrant.com Discussion: https://postgr.es/m/CA+hUKGLiieEzfrdWxWFE+_wnXho_F5Smx972X1wEubhS7v1q9g@mail.gmail.com
-
Peter Eisentraut authored
This order makes more sense because the location is effectively at the lowest level of the backtrace. Discussion: https://www.postgresql.org/message-id/flat/90f5fa04-c410-a54e-9449-aa3749fb7972%402ndquadrant.com
-
Alvaro Herrera authored
This message was being emitted on the grounds that only crashed summarization could cause it, but in reality even an aborted vacuum could do it ... which makes it way too noisy, particularly since it shows up in regression tests and makes them die. Reported by Tom Lane. Discussion: https://postgr.es/m/489091.1593534251@sss.pgh.pa.us
-
- 09 Jul, 2020 4 commits
-
-
Tom Lane authored
Due to not having our signals straight about CRLF vs. LF line termination, the output of pg_current_logfile() included a trailing \r on Windows. To fix, force the file descriptor it uses into text mode. While here, move a couple of local variable declarations to make the function's logic clearer. In v12 and v13, also back-patch the test added by 1c4e88e2 so that this function has some test coverage. However, the 004_logrotate.pl test script doesn't exist before v12, and it didn't seem worth adding to older branches just for this. Per report from Thomas Kellerer. Back-patch to v10 where this function was added. Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net
-
Tom Lane authored
Buildfarm results now imply that Perl's IPC::Run does CRLF conversion for us if we're using native Perl, but not when using MSys Perl. Restrict the conversions done by PostgresNode.pm to act only in the latter case. (Similar conversions done in TestLib.pm and RewindTest.pm were already handled this way.) Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net
-
Peter Eisentraut authored
After further discussion after daa9fe8a, reorder the version-specific sections from oldest to newest. Also, remove the variable assignments from PQfnumber() to reduce vertical space. Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr> Discussion: https://www.postgresql.org/message-id/flat/6594334b-40fd-14f1-6bc5-877afa3feed5@2ndquadrant.com
-
Fujii Masao authored
pg_stat_activity.query text is truncated at 1024 bytes. But previously the document described that it's truncated at 1024 characters. This was not accurate when considering multibyte characters. Back-patch to v10 where this inaccurate description was added. Author: Atsushi Torikoshi Reviewed-by: Daniel Gustafsson, Fujii Masao Discussion: https://postgr.es/m/cd5b49a5a14e887542f5f569c1c6bde2@oss.nttdata.com
-