aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Add CheckBuffer() to check on-disk pages without shared buffer loadingMichael Paquier2020-10-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | CheckBuffer() is designed to be a concurrent-safe function able to run sanity checks on a relation page without loading it into the shared buffers. The operation is done using a lock on the partition involved in the shared buffer mapping hashtable and an I/O lock for the buffer itself, preventing the risk of false positives due to any concurrent activity. The primary use of this function is the detection of on-disk corruptions for relation pages. If a page is found in shared buffers, the on-disk page is checked if not dirty (a follow-up checkpoint would flush a valid version of the page if dirty anyway), as it could be possible that a page was present for a long time in shared buffers with its on-disk version corrupted. Such a scenario could lead to a corrupted cluster if a host is plugged off for example. If the page is not found in shared buffers, its on-disk state is checked. PageIsVerifiedExtended() is used to apply the same sanity checks as when a page gets loaded into shared buffers. This function will be used by an upcoming patch able to check the state of on-disk relation pages using a SQL function. Author: Julien Rouhaud, Michael Paquier Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAOBaU_aVvMjQn=ge5qPiJOPMmOj5=ii3st5Q0Y+WuLML5sR17w@mail.gmail.com
* Makefile comment: remove reference to tools/thread/thread_testBruce Momjian2020-10-27
| | | | | | | | | | You can't compile thread_test alone anymore, and the location moved too. Reported-by: Tom Lane Discussion: https://postgr.es/m/1062278.1603819969@sss.pgh.pa.us Backpatch-through: 9.5
* pg_dump: Lock all relations, not just plain tablesAlvaro Herrera2020-10-27
| | | | | | | | | | | | | | | | Now that LOCK TABLE can take any relation type, acquire lock on all relations that are to be dumped. This prevents schema changes or deadlock errors that could cause a dump to fail after expending much effort. The server is tested to have the capability and the feature disabled if it doesn't, so that a patched pg_dump doesn't fail when connecting to an unpatched server. Backpatch to 9.5. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Wells Oliver <wells.oliver@gmail.com> Discussion: https://postgr.es/m/20201021200659.GA32358@alvherre.pgsql
* Add select_common_typmod()Peter Eisentraut2020-10-27
| | | | | | | | | | | | | | | This accompanies select_common_type() and select_common_collation(). Typmods were previously combined using hand-coded logic in several places. The logic in select_common_typmod() isn't very exciting, but it makes the code more compact and readable in a few locations, and in the future we can perhaps do more complicated things if desired. As a small enhancement, the type unification of the direct and aggregate arguments of hypothetical-set aggregates now unifies the typmod as well using this new function, instead of just dropping it. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/97df3af9-8b5e-fb7f-a029-3eb7e80d7af9@2ndquadrant.com
* Accept relations of any kind in LOCK TABLEAlvaro Herrera2020-10-27
| | | | | | | | | | | | | | | The restriction that only tables and views can be locked by LOCK TABLE is quite arbitrary, since the underlying mechanism can lock any relation type. Drop the restriction so that programs such as pg_dump can lock all relations they're interested in, preventing schema changes that could cause a dump to fail after expending much effort. Backpatch to 9.5. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Wells Oliver <wells.oliver@gmail.com> Discussion: https://postgr.es/m/20201021200659.GA32358@alvherre.pgsql
* Fix enum errdetail to mention bytes, not charsPeter Eisentraut2020-10-27
| | | | | | | | The enum label length is in terms of bytes, not charactes. Author: Ian Lawrence Barwick <barwick@gmail.com> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAB8KJ=itZEJ7C9BacTHSYgeUysH4xx8wDiOnyppnSLyn6-g+Bw@mail.gmail.com
* Make procedure OUT parameters work with JDBCPeter Eisentraut2020-10-27
| | | | | | | | | | The JDBC driver sends OUT parameters with type void. This makes sense when calling a function, so that the parameters are ignored in ParseFuncOrColumn(). For a procedure call we want to treat them as unknown. Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://www.postgresql.org/message-id/flat/d7e49540-ea92-b4e2-5fff-42036102f968%402ndquadrant.com
* In INSERT/UPDATE, use the table's real tuple descriptor as target.Tom Lane2020-10-26
| | | | | | | | | | | | | | | | | | | | | | | Previously, ExecInitModifyTable relied on ExecInitJunkFilter, and thence ExecCleanTypeFromTL, to build the target descriptor from the query tlist. While we just checked (in ExecCheckPlanOutput) that the tlist produces compatible output, this is not a great substitute for the relation's actual tuple descriptor that's available from the relcache. For one thing, dropped columns will not be correctly marked attisdropped; it's a bit surprising that we've gotten away with that this long. But the real reason for being concerned with this is that using the table's descriptor means that the slot will have correct attrmissing data, allowing us to revert the klugy fix of commit ba9f18abd. (This commit undoes that one's changes in trigger.c, but keeps the new test case.) Thus we can solve the bogus-trigger-tuple problem with fewer cycles rather than more. No back-patch, since this doesn't fix any additional bug, and it seems somewhat more likely to have unforeseen side effects than ba9f18abd's narrow fix. Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
* Extend PageIsVerified() to handle more custom optionsMichael Paquier2020-10-26
| | | | | | | | | | | | | | | | This is useful for checks of relation pages without having to load the pages into the shared buffers, and two cases can make use of that: page verification in base backups and the online, lock-safe, flavor. Compatibility is kept with past versions using a macro that calls the new extended routine with the set of options compatible with the original version. Extracted from a larger patch by the same author. Author: Anastasia Lubennikova Reviewed-by: Michael Paquier, Julien Rouhaud Discussion: https://postgr.es/m/608f3476-0598-2514-2c03-e05c7d2b0cbd@postgrespro.ru
* Fix corner case for a BEFORE ROW UPDATE trigger returning OLD.Tom Lane2020-10-25
| | | | | | | | | | | | | | | | | | | | | | | | | If the old row has any "missing" attributes that are supposed to be retrieved from an associated tuple descriptor, the wrong things happened because the trigger result is shoved directly into an executor slot that lacks the missing-attribute data. Notably, CHECK-constraint verification would incorrectly see those columns as NULL, and so would RETURNING-list evaluation. Band-aid around this by forcibly expanding the tuple before passing it to the trigger function. (IMO it was a fundamental misdesign to put the missing-attribute data into tuple constraints, which so much of the system considers to be optional. But we're probably stuck with that now, and will have to continue to apply band-aids as we find other places with similar issues.) Back-patch to v12. v11 would also have the issue, except that commit 920311ab1 already applied a similar band-aid. That forced expansion in more cases than seem really necessary, though, so this isn't a directly equivalent fix. Amit Langote, with some cosmetic changes by me Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
* Fix incorrect parameter name in a function header commentDavid Rowley2020-10-25
| | | | | | Author: Zhijie Hou Discussion: https://postgr.es/m/14cd74ea00204cc8a7ea5d738ac82cd1@G08CNEXMBPEKD05.g08.fujitsu.local Backpatch-through: 12, where the mistake was introduced
* Fix ancient bug in ecpg's pthread_once() emulation for Windows.Tom Lane2020-10-24
| | | | | | | | | | | | | | | | We must not set the "done" flag until after we've executed the initialization function. Otherwise, other threads can fall through the initial unlocked test before initialization is really complete. This has been seen to cause rare failures of ecpg's thread/descriptor test, and it could presumably cause other sorts of misbehavior in threaded ECPG-using applications, since ecpglib relies on pthread_once() in several places. Diagnosis and patch by me, based on investigation by Alexander Lakhin. Back-patch to all supported branches (the bug dates to 2007). Discussion: https://postgr.es/m/16685-d6cd241872c101d3@postgresql.org
* Fix issue with --enable-coverage and the new unicode {de,re}composition codeMichael Paquier2020-10-24
| | | | | | | | | | | | | | | | | | | genhtml has been generating the following warning with this new code: WARNING: function data mismatch at /path/src/common/unicode_norm.c:102 HTML coverage reports care about the uniqueness of functions defined in source files, ignoring any assumptions around CFLAGS. 783f0cc introduced a duplicated definition of get_code_entry(), leading to a warning and potentially some incorrect data generated in the reports. This refactors the code so as the code has only one function declaration, fixing the warning. Oversight in 783f0cc. Reported-by: Tom Lane Author: Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/207789.1603469272@sss.pgh.pa.us
* Add tab completion for ALTER TABLE .. FORCE ROW LEVEL SECURITY in psqlMichael Paquier2020-10-24
| | | | | | | | This completes both the FORCE and NO FORCE options, NO INHERIT needing a small adjustment. Author: Li Japin Discussion: https://postgr.es/m/15B10F9F-5847-4F5E-BD66-8E25AA473C95@hotmail.com
* Allow psql to re-use connection parameters after a connection loss.Tom Lane2020-10-23
| | | | | | | | | | | | | | | | | | | | | | Instead of immediately PQfinish'ing a dead connection, save it aside so that we can still extract its parameters for \connect attempts. (This works because PQconninfo doesn't care whether the PGconn is in CONNECTION_BAD state.) This allows developers to reconnect with just \c after a database crash and restart. It's tempting to use the same approach instead of closing the old connection after a failed non-interactive \connect command. However, that would not be very safe: consider a script containing \c db1 user1 live_server \c db2 user2 dead_server \c db3 The script would be expecting to connect to db3 at dead_server, but if we re-use parameters from the first connection then it might successfully connect to db3 at live_server. This'd defeat the goal of not letting a script accidentally execute commands against the wrong database. Discussion: https://postgr.es/m/38464.1603394584@sss.pgh.pa.us
* Fix broken XML formatting in EXPLAIN output for incremental sorts.Tom Lane2020-10-23
| | | | | | | | | | The ExplainCloseGroup arguments for incremental sort usage data didn't match the corresponding ExplainOpenGroup. This only matters for XML-format output, which is probably why we'd not noticed. Daniel Gustafsson, per bug #16683 from Frits Jalvingh Discussion: https://postgr.es/m/16683-8005033324ad34e9@postgresql.org
* Fix initialization of es_result_relations in EvalPlanQualStart().Heikki Linnakangas2020-10-23
| | | | | | | | | | | | Thinko in commit 1375422c782. EvalPlanQualStart() was mistakenly resetting the parent EState's es_result_relations, when it should initialize the field in the child EPQ EState it just created. That was clearly wrong, but it didn't cause any ill effects, because es_result_relations is currently not used after the ExecInit* phase. Author: Amit Langote Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFEuq8AAAmxXsTDVZ1r38cHbfYuiPQx_%3DYyKe2DC-6q4A%40mail.gmail.com
* Improve performance of Unicode {de,re}composition in the backendMichael Paquier2020-10-23
| | | | | | | | | | | | | | | | | | | | | | | | | This replaces the existing binary search with two perfect hash functions for the composition and the decomposition in the backend code, at the cost of slightly-larger binaries there (35kB in libpgcommon_srv.a). Per the measurements done, this improves the speed of the recomposition and decomposition by up to 30~40 times for the NFC and NFKC conversions, while all other operations get at least 40% faster. This is not as "good" as what libicu has, but it closes the gap a lot as per the feedback from Daniel Verite. The decomposition table remains the same, getting used for the binary search in the frontend code, where we care more about the size of the libraries like libpq over performance as this gets involved only in code paths related to the SCRAM authentication. In consequence, note that the perfect hash function for the recomposition needs to use a new inverse lookup array back to to the existing decomposition table. The size of all frontend deliverables remains unchanged, even with --enable-debug, including libpq. Author: John Naylor Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/CAFBsxsHUuMFCt6-pU+oG-F1==CmEp8wR+O+bRouXWu6i8kXuqA@mail.gmail.com
* Update time zone data files to tzdata release 2020d.Tom Lane2020-10-22
| | | | | DST law changes in Palestine, with a whopping 120 hours' notice. Also some historical corrections for Palestine.
* Sync our copy of the timezone library with IANA release tzcode2020d.Tom Lane2020-10-22
| | | | | | | | There's no functional change at all here, but I'm curious to see whether this change successfully shuts up Coverity's warning about a useless strcmp(), which appeared with the previous update. Discussion: http://mm.icann.org/pipermail/tz/2020-October/029370.html
* Add documentation and tests for quote marks in ECPG literal queries.Tom Lane2020-10-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ECPG's PREPARE ... FROM and EXECUTE IMMEDIATE can optionally take the target query as a simple literal, rather than the more usual string-variable reference. This was previously documented as being a C string literal, but that's a lie in one critical respect: you can't write a data double quote as \" in such literals. That's because the lexer is in SQL mode at this point, so it'll parse double-quoted strings as SQL identifiers, within which backslash is not special, so \" ends the literal. I looked into making this work as documented, but getting the lexer to switch behaviors at just the right point is somewhere between very difficult and impossible. It's not really worth the trouble, because these cases are next to useless: if you have a fixed SQL statement to execute or prepare, you might as well write it as a direct EXEC SQL, saving the messiness of converting it into a string literal and gaining the opportunity for compile-time SQL syntax checking. Instead, let's just document (and test) the workaround of writing a double quote as an octal escape (\042) in such cases. There's no code behavioral change here, so in principle this could be back-patched, but it's such a niche case I doubt it's worth the trouble. Per report from 1250kv. Discussion: https://postgr.es/m/673825.1603223178@sss.pgh.pa.us
* Avoid premature de-doubling of quote marks in ECPG strings.Tom Lane2020-10-22
| | | | | | | | | | | | | | | | | | | | | | If you write the literal 'abc''def' in an EXEC SQL command, that will come out the other end as 'abc'def', triggering a syntax error in the backend. Likewise, "abc""def" is reduced to "abc"def" which is wrong syntax for a quoted identifier. The cause is that the lexer thinks it should emit just one quote mark, whereas what it really should do is keep the string as-is. Add some docs and test cases, too. Although this seems clearly a bug, I fear users wouldn't appreciate changing it in minor releases. Some may well be working around it by applying an extra doubling of affected quotes, as for example sql/dyntest.pgc has been doing. Per investigation of a report from 1250kv, although this isn't exactly what he/she was on about. Discussion: https://postgr.es/m/673825.1603223178@sss.pgh.pa.us
* Clean up some unpleasant behaviors in psql's \connect command.Tom Lane2020-10-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The check for whether to complain about not having an old connection to get parameters from was seriously out of date: it had not been rethought when we invented connstrings, nor when we invented the -reuse-previous option. Replace it with a check that throws an error if reuse-previous is active and we lack an old connection to reuse. While that doesn't move the goalposts very far in terms of easing reconnection after a server crash, at least it's consistent. If the user specifies a connstring plus additional parameters (which is invalid per the documentation), the extra parameters were silently ignored. That seems like it could be really confusing, so let's throw a syntax error instead. Teach the connstring code path to re-use the old connection's password in the same cases as the old-style-syntax code path would, ie if we are reusing parameters and the values of username, host/hostaddr, and port are not being changed. Document this behavior, too, since it was unmentioned before. Also simplify the implementation a bit, giving rise to two new and useful properties: if there's a "password=xxx" in the connstring, we'll use it not ignore it, and by default (i.e., except with --no-password) we will prompt for a password if the re-used password or connstring password doesn't work. The previous code just failed if the re-used password didn't work. Given the paucity of field complaints about these issues, I don't think that they rise to the level of back-patchable bug fixes, and in any case they might represent undesirable behavior changes in minor releases. So no back-patch. Discussion: https://postgr.es/m/235210.1603321144@sss.pgh.pa.us
* Extend amcheck to check heap pages.Robert Haas2020-10-22
| | | | | | | | Mark Dilger, reviewed by Peter Geoghegan, Andres Freund, Álvaro Herrera, Michael Paquier, Amul Sul, and by me. Some last-minute cosmetic revisions by me. Discussion: http://postgr.es/m/12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com
* Use croak instead of die in Perl code when appropriatePeter Eisentraut2020-10-22
|
* Optimize a few list_delete_ptr callsDavid Rowley2020-10-22
| | | | | | | | | | | | | | | | | | | | | There is a handful of places where we called list_delete_ptr() to remove some element from a List. In many of these places we know, or with very little additional effort know the index of the ListCell that we need to remove. Here we change all of those places to instead either use one of; list_delete_nth_cell(), foreach_delete_current() or list_delete_last(). Each of these saves from having to iterate over the list to search for the element to remove by its pointer value. There are some small performance gains to be had by doing this, but in the general case, none of these lists are likely to be very large, so the lookup was probably never that expensive anyway. However, some of the calls are in fairly hot code paths, e.g process_equivalence(). So any small gains there are useful. Author: Zhijie Hou and David Rowley Discussion: https://postgr.es/m/b3517353ec7c4f87aa560678fbb1034b@G08CNEXMBPEKD05.g08.fujitsu.local
* Fix connection string handling in psql's \connect command.Tom Lane2020-10-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | psql's \connect claims to be able to re-use previous connection parameters, but in fact it only re-uses the database name, user name, host name (and possibly hostaddr, depending on version), and port. This is problematic for assorted use cases. Notably, pg_dump[all] emits "\connect databasename" commands which we would like to have re-use all other parameters. If such a script is loaded in a psql run that initially had "-d connstring" with some non-default parameters, those other parameters would be lost, potentially causing connection failure. (Thus, this is the same kind of bug addressed in commits a45bc8a4f and 8e5793ab6, although the details are much different.) To fix, redesign do_connect() so that it pulls out all properties of the old PGconn using PQconninfo(), and then replaces individual properties in that array. In the case where we don't wish to re-use anything, get libpq's default settings using PQconndefaults() and replace entries in that, so that we don't need different code paths for the two cases. This does result in an additional behavioral change for cases where the original connection parameters allowed multiple hosts, say "psql -h host1,host2", and the \connect request allows re-use of the host setting. Because the previous coding relied on PQhost(), it would only permit reconnection to the same host originally selected. Although one can think of scenarios where that's a good thing, there are others where it is not. Moreover, that behavior doesn't seem to meet the principle of least surprise, nor was it documented; nor is it even clear it was intended, since that coding long pre-dates the addition of multi-host support to libpq. Hence, this patch is content to drop it and re-use the host list as given. Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
* Use fast checkpoint in PostgresNode::backup()Alvaro Herrera2020-10-21
| | | | Should cause tests to be a bit faster
* Remove the option to build thread_test.c outside configure.Tom Lane2020-10-21
| | | | | | | | | | | | | | | | | | | Theoretically one could go into src/test/thread and build/run this program there. In practice, that hasn't worked since 96bf88d52, and probably much longer on some platforms (likely including just the sort of hoary leftovers where this test might be of interest). While it wouldn't be too hard to repair the breakage, the fact that nobody has noticed for two years shows that there is zero usefulness in maintaining this build pathway. Let's get rid of it and decree that thread_test.c is *only* meant to be built/used in configure. Given that decision, it makes sense to put thread_test.c under config/ and get rid of src/test/thread altogether, so that's what I did. In passing, update src/test/README, which had been ignored by some not-so-recent additions of subdirectories. Discussion: https://postgr.es/m/227659.1603041612@sss.pgh.pa.us
* Remove obsolete ifdefsPeter Eisentraut2020-10-21
| | | | | | | | | | | Commit 8dace66e0735ca39b779922d02c24ea2686e6521 added #ifdefs for a number of errno symbols because they were not present on Windows. Later, commit 125ad539a275db5ab8f4647828b80a16d02eabd2 added replacement #defines for some of those symbols. So some of the changes from the first commit are made dead code by the second commit and can now be removed. Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
* Fix -Wcast-function-type warnings on Windows/MinGWPeter Eisentraut2020-10-21
| | | | | | | | After de8feb1f3a23465b5737e8a8c160e8ca62f61339, some warnings remained that were only visible when using GCC on Windows. Fix those as well. Note that the ecpg test source files don't use the full pg_config.h, so we can't use pg_funcptr_t there but have to do it the long way.
* Review format of code generated by PerfectHash.pmMichael Paquier2020-10-21
| | | | | | | | | | | | | | | 80f8eb7 has added to the normalization quick check headers some code generated by PerfectHash.pm that is incompatible with the settings of gitattributes for this repository, as whitespaces followed a set of tabs for the first element of a line in the table. Instead of adding a new exception to gitattributes, rework the format generated so as a right padding with spaces is used instead of a left padding. This keeps the table generated in a readable shape with its set of columns, making unnecessary an update of gitattributes. Reported-by: Peter Eisentraut Author: John Naylor Discussion: https://postgr.es/m/d601b3b5-a3c7-5457-2f84-3d6513d690fc@2ndquadrant.com
* Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursionAlvaro Herrera2020-10-20
| | | | | | | | | | | | | | | | More precisely, correctly handle the ONLY flag indicating not to recurse. This was implemented in 86f575948c77 by recursing in trigger.c, but that's the wrong place; use ATSimpleRecursion instead, which behaves properly. However, because legacy inheritance has never recursed in that situation, make sure to do that only for new-style partitioning. I noticed this problem while testing a fix for another bug in the vicinity. This has been wrong all along, so backpatch to 11. Discussion: https://postgr.es/m/20201016235925.GA29829@alvherre.pgsql
* Change the attribute name in pg_stat_replication_slots view.Amit Kapila2020-10-20
| | | | | | | | | | | | | | | Change the attribute 'name' to 'slot_name' in pg_stat_replication_slots view to make it clear and that way we will be consistent with the other places like pg_stat_wal_receiver view where we display the same attribute. In the passing, fix the typo in one of the macros in the related code. Bump the catversion as we have modified the name in the catalog as well. Reported-by: Noriyoshi Shinoda Author: Noriyoshi Shinoda Reviewed-by: Sawada Masahiko and Amit Kapila Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
* Fix connection string handling in src/bin/scripts/ programs.Tom Lane2020-10-19
| | | | | | | | | | | | | | | | | | | | | When told to process all databases, clusterdb, reindexdb, and vacuumdb would reconnect by replacing their --maintenance-db parameter with the name of the target database. If that parameter is a connstring (which has been allowed for a long time, though we failed to document that before this patch), we'd lose any other options it might specify, for example SSL or GSS parameters, possibly resulting in failure to connect. Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and pg_restore. We can fix it in the same way, by using libpq's rules for handling multiple "dbname" parameters to add the target database name separately. I chose to apply the same refactoring approach as in that patch, with a struct to handle the command line parameters that need to be passed through to connectDatabase. (Maybe someday we can unify the very similar functions here and in pg_dump/pg_restore.) Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
* Fix list-munging bug that broke SQL function result coercions.Tom Lane2020-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 913bbd88d, check_sql_fn_retval() can either insert type coercion steps in-line in the Query that produces the SQL function's results, or generate a new top-level Query to perform the coercions, if modifying the Query's output in-place wouldn't be safe. However, it appears that the latter case has never actually worked, because the code tried to inject the new Query back into the query list it was passed ... which is not the list that will be used for later processing when we execute the SQL function "normally" (without inlining it). So we ended up with no coercion happening at run-time, leading to wrong results or crashes depending on the datatypes involved. While the regression tests look like they cover this area well enough, through a huge bit of bad luck all the test cases that exercise the separate-Query path were checking either inline-able cases (which accidentally didn't have the bug) or cases that are no-ops at runtime (e.g., varchar to text), so that the failure to perform the coercion wasn't obvious. The fact that the cases that don't work weren't allowed at all before v13 probably contributed to not noticing the problem sooner, too. To fix, get rid of the separate "flat" list of Query nodes and instead pass the real two-level list that is going to be used later. I chose to make the same change in check_sql_fn_statements(), although that has no actual bug, just so that we don't need that data structure at all. This is an API change, as evidenced by the adjustments needed to callers outside functions.c. That's a bit scary to be doing in a released branch, but so far as I can tell from a quick search, there are no outside callers of these functions (and they are sufficiently specific to our semantics for SQL-language functions that it's not apparent why any extension would need to call them). In any case, v13 already changed the API of check_sql_fn_retval() compared to prior branches. Per report from pinker. Back-patch to v13 where this code came in. Discussion: https://postgr.es/m/1603050466566-0.post@n3.nabble.com
* Remove PartitionRoutingInfo struct.Heikki Linnakangas2020-10-19
| | | | | | | | | | The extra indirection neeeded to access its members via its enclosing ResultRelInfo seems pointless. Move all the fields from PartitionRoutingInfo to ResultRelInfo. Author: Amit Langote Reviewed-by: Alvaro Herrera Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFViT47Zbr_ASBejiK7iDG8%3DQ1swQ-tjM6caRPQ67pT%3Dw%40mail.gmail.com
* Revise child-to-root tuple conversion map management.Heikki Linnakangas2020-10-19
| | | | | | | | | | | | | | | | | | | | | | | Store the tuple conversion map to convert a tuple from a child table's format to the root format in a new ri_ChildToRootMap field in ResultRelInfo. It is initialized if transition tuple capture for FOR STATEMENT triggers or INSERT tuple routing on a partitioned table is needed. Previously, ModifyTable kept the maps in the per-subplan ModifyTableState->mt_per_subplan_tupconv_maps array, or when tuple routing was used, in ResultRelInfo->ri_Partitioninfo->pi_PartitionToRootMap. The new field replaces both of those. Now that the child-to-root tuple conversion map is always available in ResultRelInfo (when needed), remove the TransitionCaptureState.tcs_map field. The callers of Exec*Trigger() functions no longer need to set or save it, which is much less confusing and bug-prone. Also, as a future optimization, this will allow us to delay creating the map for a given result relation until the relation is actually processed during execution. Author: Amit Langote Discussion: https://www.postgresql.org/message-id/CA%2BHiwqHtCWLdK-LO%3DNEsvOdHx%2B7yv4mE_zYK0i3BH7dXb-wxog%40mail.gmail.com
* Clean up code to resolve the "root target relation" in nodeModifyTable.cHeikki Linnakangas2020-10-19
| | | | | | | | | | | | | | | | | When executing DDL on a partitioned table or on a table with inheritance children, statement-level triggers must be fired against the table given in the original statement. The code to look that up was a bit messy and duplicative. Commit 501ed02cf6 added a helper function, getASTriggerResultRelInfo() (later renamed to getTargetResultRelInfo()) for it, but for some reason it was only used when firing AFTER STATEMENT triggers and the code to fire BEFORE STATEMENT triggers duplicated the logic. Determine the target relation in ExecInitModifyTable(), and set it always in ModifyTableState. Code that used to call getTargetResultRelInfo() can now use ModifyTableState->rootResultRelInfo directly. Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFViT47Zbr_ASBejiK7iDG8%3DQ1swQ-tjM6caRPQ67pT%3Dw%40mail.gmail.com
* Avoid invalid alloc size error in shm_mqPeter Eisentraut2020-10-19
| | | | | | | | | | | | In shm_mq_receive(), a huge payload could trigger an unjustified "invalid memory alloc request size" error due to the way the buffer size is increased. Add error checks (documenting the upper limit) and avoid the error by limiting the allocation size to MaxAllocSize. Author: Markus Wanner <markus.wanner@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/3bb363e7-ac04-0ac4-9fe8-db1148755bfa%402ndquadrant.com
* Improve whitespacePeter Eisentraut2020-10-19
| | | | | The SQL file was using a mix of tabs and spaces inconsistently. Convert all to spaces and adjust indentation accordingly.
* Prevent overly large and NaN row estimates in relationsDavid Rowley2020-10-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Given a query with enough joins, it was possible that the query planner, after multiplying the row estimates with the join selectivity that the estimated number of rows would exceed the limits of the double data type and become infinite. To give an indication on how extreme a case is required to hit this, the particular example case reported required 379 joins to a table without any statistics, which resulted in the 1.0/DEFAULT_NUM_DISTINCT being used for the join selectivity. This eventually caused the row estimates to go infinite and resulted in an assert failure in initial_cost_mergejoin() where the infinite row estimated was multiplied by an outerstartsel of 0.0 resulting in NaN. The failing assert verified that NaN <= Inf, which is false. To get around this we use clamp_row_est() to cap row estimates at a maximum of 1e100. This value is thought to be low enough that costs derived from it would remain within the bounds of what the double type can represent. Aside from fixing the failing Assert, this also has the added benefit of making it so add_path() will still receive proper numerical values as costs which will allow it to make more sane choices when determining the cheaper path in extreme cases such as the one described above. Additionally, we also get rid of the isnan() checks in the join costing functions. The actual case which originally triggered those checks to be added in the first place never made it to the mailing lists. It seems likely that the new code being added to clamp_row_est() will result in those becoming checks redundant, so just remove them. The fairly harmless assert failure problem does also exist in the backbranches, however, a more minimalistic fix will be applied there. Reported-by: Onder Kalaci Reviewed-by: Tom Lane Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com
* Update the Winsock API version requested by libpq.Tom Lane2020-10-18
| | | | | | | | | | | | | | | According to Microsoft's documentation, 2.2 has been the current version since Windows 98 or so. Moreover, that's what the Postgres backend has been requesting since 2004 (cf commit 4cdf51e64). So there seems no reason for libpq to keep asking for 1.1. Bring thread_test along, too, so that we're uniformly asking for 2.2 in all our WSAStartup calls. It's not clear whether there's any point in back-patching this, so for now I didn't. Discussion: https://postgr.es/m/132799.1602960277@sss.pgh.pa.us
* In pg_restore's dump_lo_buf(), work a little harder on error handling.Tom Lane2020-10-18
| | | | | | | | | | | | | | | | | | Failure to write data to a large object during restore led to an ugly and uninformative error message. To add insult to injury, it then fatal'd out, where other SQL-level errors usually result in pressing on. Report the underlying error condition, rather than just giving not-very- useful byte counts, and use warn_or_exit_horribly() so as to adhere to pg_restore's general policy about whether to continue or not. Also recognize that lo_write() returns int not size_t. Per report from Justin Pryzby, though I didn't use his patch. Given the lack of comparable complaints, I'm not sure this is worth back-patching. Discussion: https://postgr.es/m/20201018010232.GF9241@telsasoft.com
* In libpq for Windows, call WSAStartup once and WSACleanup not at all.Tom Lane2020-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The Windows documentation insists that every WSAStartup call should have a matching WSACleanup call. However, if that ever had actual relevance, it wasn't in this century. Every remotely-modern Windows kernel is capable of cleaning up when a process exits without doing that, and must be so to avoid resource leaks in case of a process crash. Moreover, Postgres backends have done WSAStartup without WSACleanup since commit 4cdf51e64 in 2004, and we've never seen any indication of a problem with that. libpq's habit of doing WSAStartup during connection start and WSACleanup during shutdown is also rather inefficient, since a series of non-overlapping connection requests leads to repeated, quite expensive DLL unload/reload cycles. We document a workaround for that (having the application call WSAStartup for itself), but that's just a kluge. It's also worth noting that it's far from uncommon for applications to exit without doing PQfinish, and we've not heard reports of trouble from that either. However, the real reason for acting on this is that recent experiments by Alexander Lakhin suggest that calling WSACleanup during PQfinish might be triggering the symptom we occasionally see that a process using libpq fails to emit expected stdio output. Therefore, let's change libpq so that it calls WSAStartup only once per process, during the first connection attempt, and never calls WSACleanup at all. While at it, get rid of the only other WSACleanup call in our code tree, in pg_dump/parallel.c; that presumably is equally useless. If this proves to suppress the fairly-common ecpg test failures we see on Windows, I'll back-patch, but for now let's just do it in HEAD and see what happens. Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru
* Update time zone data files to tzdata release 2020c.Tom Lane2020-10-16
| | | | | | DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island, Casey Station (Antarctica). Historical corrections for France, Hungary, Monaco.
* Sync our copy of the timezone library with IANA release tzcode2020c.Tom Lane2020-10-16
| | | | | | | | | | | | | | | This changes zic's default output format from "-b fat" to "-b slim". We were already using "slim" in v13/HEAD, so those branches drop the explicit -b switch in the Makefiles. Instead, add an explicit "-b fat" in v12 and before, so that we don't change the output file format in those branches. (This is perhaps excessively conservative, but we decided not to do so in a12079109, and I'll stick with that.) Other non-cosmetic changes are to drop support for zic's long-obsolete "-y" switch, and to ensure that strftime() does not change errno unless it fails. As usual with tzcode changes, back-patch to all supported branches.
* llvmjit: Work around bug in LLVM 3.9 causing crashes after 72559438f92.Andres Freund2020-10-15
| | | | | | | | | | | | | | Unfortunately in LLVM 3.9 LLVMGetAttributeCountAtIndex(func, index) crashes when called with an index that has 0 attributes. Since there's no way to work around this in the C API, add a small C++ wrapper doing so. The only reason this didn't fail before 72559438f92 is that there always are function attributes... Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20201016001254.w2nfj7gd74jmb5in@alap3.anarazel.de Backpatch: 11-, like 72559438f92
* pg_upgrade: remove C99 compiler req. from commit 3c0471b5fdBruce Momjian2020-10-15
| | | | | | | | | | | | | | This commit required support for inline variable definition, which is not a requirement. RELEASE NOTE AUTHOR: the author of commit 3c0471b5fd (pg_upgrade/tablespaces) was Justin Pryzby, not me. Reported-by: Andres Freund Discussion: https://postgr.es/m/20201016001959.h24fkywfubkv2pc5@alap3.anarazel.de Backpatch-through: 9.5
* pg_upgrade: generate check error for left-over new tablespaceBruce Momjian2020-10-15
| | | | | | | | | | | | | Previously, if pg_upgrade failed, and the user recreated the cluster but did not remove the new cluster tablespace directory, a later pg_upgrade would fail since the new tablespace directory would already exists. This adds error reporting for this during check. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200925005531.GJ23631@telsasoft.com Backpatch-through: 9.5
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-30 17:56:11 +0000