Sync Fork from Upstream Repo #143
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Do some minor cleanup for commit c8434d6: 1) remove a useless assignment (in normal builds) and 2) improve comments a little. Back-patch to v13 where the aforementioned commit went in. Author: Etsuro Fujita Reviewed-by: Alvaro Herrera Discussion: https://postgr.es/m/CAPmGK16yCd2R4=bQ4g8N2dT9TtA5ZU+qNmJ3LPc_nypbNy4_2A@mail.gmail.com
Reported-by: Robert Kahlert Author: Daniel Gustafsson
EXTRACT of date type is implemented as a wrapper around EXTRACT of timestamp, so the code is already tested there. But the externally visible behavior of EXTRACT on date is not recorded anywhere. Since there is some discussion about reimplementing or refactoring some of this, add some more explicit tests of EXTRACT on date, similar in structure to existing EXTRACT tests on other data types. Discussion: https://www.postgresql.org/message-id/flat/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
We have had multiple reports that point to the '@colReorder=latn-digit' collation customization being buggy. We have reported this to ICU and are waiting for a fix. In the meantime, remove references to this from the documentation and replace it by another reordering example. Apparently, many users have been picking up this example specifically from the documentation. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://www.postgresql.org/message-id/flat/153201618542.1404.3611626898935613264%40wrigleys.postgresql.org
Add libssl and libcrypto to libpq.pc's Requires.private. This allows static linking to work if those libssl or libcrypto themselves have dependencies in their *.private fields, such as -lz in some cases. Reported-by: Sandro Mani <manisandro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/837d1dcf-2fca-ee6e-0d7e-6bce1a1bac75@gmail.com
Apparently, this was never used when introduced (3dad73e). Discussion: https://www.postgresql.org/message-id/flat/511bb100-f829-ba21-2f10-9f952ec06ead%402ndquadrant.com
Sometimes it happens that the visibility information for a tuple becomes corrupted, either due to bugs in the database software or external factors. Provide a function heap_force_kill() that can be used to truncate such dead tuples to dead line pointers, and a function heap_force_freeze() that can be used to overwrite the visibility information in such a way that the tuple becomes all-visible. These functions are unsafe, in that you can easily use them to corrupt a database that was not previously corrupted, and you can use them to further corrupt an already-corrupted database or to destroy data. The documentation accordingly cautions against casual use. However, in some cases they permit recovery of data that would otherwise be very difficult to recover, or to allow a system to continue to function when it would otherwise be difficult to do so. Because we may want to add other functions for performing other kinds of surgery in the future, the new contrib module is called pg_surgery rather than something specific to these functions. I proposed back-patching this so that it could be more easily used by people running existing releases who are facing these kinds of problems, but that proposal did not attract enough support, so no back-patch for now. Ashutosh Sharma, reviewed and tested by Andrey M. Borodin, M. Beena Emerson, Masahiko Sawada, Rajkumar Raghuwanshi, Asim Praveen, and Mark Dilger, and somewhat revised by me. Discussion: http://postgr.es/m/CA+TgmoZW1fsU-QUNCRUQMGUygBDPVeOTLCqRdQZch=EYZnctSA@mail.gmail.com
Bring the signal handling for startup-packet collection into line with the policy established in commits bedadc7 and 8e19a82, namely don't risk running atexit callbacks when handling SIGQUIT. Ideally, we'd not do so for SIGTERM or timeout interrupts either, but that change seems a bit too risky for the back branches. For now, just improve the comments in this area to describe the risk. Also relocate where BackendInitialize re-disables these interrupts, to minimize the code span where they're active. This doesn't buy a whole lot of safety, but it can't hurt. In passing, rename startup_die() to remove confusion about whether it is for the startup process. Like the previous commits, back-patch to all supported branches. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
This helps debuggability when looking at WAL streams containing logical messages. Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CAExHW5sWx49rKmXbg5H1Xc1t+nRv9PaYKQmgw82HPt6vWDVmDg@mail.gmail.com
We were decoding empty transactions via streaming APIs added in commit 45fdc97 even when the user used the option 'skip-empty-xacts'. The APIs makes no effort to skip empty xacts under the assumption that we will never try to stream such transactions. However, that is not true because we can pick to stream a transaction that has change messages for REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT and we don't send such messages to downstream rather they are just to update the internal state. So, we need to skip such xacts when plugin uses the option 'skip-empty-xacts'. Diagnosed-By: Amit Kapila Author: Dilip Kumar Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAA4eK1+OqgFNZkf7=ETe_y5ntjgDk3T0wcdkd4Sot_u1hySGfw@mail.gmail.com
Thinko in 40b3e2c. Reported-by: "Wang, Shenhao" <wangsh.fnst@cn.fujitsu.com> Discussion: https://postgr.es/m/ed98706b82694b57a8c0d339a10732aa@G08CNEXMBPEKD06.g08.fujitsu.local
…ket. Although 58c6fec fixed the case for SIGQUIT, we were still calling proc_exit() from signal handlers for SIGTERM and timeout failures in ProcessStartupPacket. Fortunately, at the point where that code runs, we haven't yet connected to shared memory in any meaningful way, so there is nothing we need to undo in shared memory. This means it should be safe to use _exit(1) here, ie, not run any atexit handlers but also inform the postmaster that it's not a crash exit. To make sure nobody breaks the "nothing to undo" expectation, add a cross-check that no on-shmem-exit or before-shmem-exit handlers have been registered yet when we finish using these signal handlers. This change is simple enough that maybe it could be back-patched, but I won't risk that right now. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
Currently, no useful trace is left in the logs when the postmaster is forced to use SIGKILL to shut down children that failed to respond to SIGQUIT. Some questions were raised about how often that scenario happens in the buildfarm, so let's add a LOG-level message showing that it happened. Discussion: https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
The stats target can be set since commit d06215d, but wasn't shown by psql. Author: Justin Pryzby <justin@telsasoft.com> Discussion: https://postgr.es/m/20200831050047.GG5450@telsasoft.com Reviewed-by: Georgios Kokolatos <gkokolatos@protonmail.com> Reviewed-by: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
The bgwriter, checkpointer, walwriter, and walreceiver processes claimed to allow SIGQUIT "at all times". In reality SIGQUIT would get re-blocked during error recovery, because we didn't update the actual signal mask immediately, so sigsetjmp() would save and reinstate a mask that includes SIGQUIT. This appears to be simply a coding oversight. There's never a good reason to hold off SIGQUIT in these processes, because it's going to just call _exit(2) which should be safe enough, especially since the postmaster is going to tear down shared memory afterwards. Hence, stick in PG_SETMASK() calls to install the modified BlockSig mask immediately. Also try to improve the comments around sigsetjmp blocks. Most of them were just referencing postgres.c, which is misleading because actually postgres.c manages the signals differently. No back-patch, since there's no evidence that this is causing any problems in the field. Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
The preallocation logic is only useful for HashAgg, so disable it when sorting. Also, adjust an out-of-date comment. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wzn_o7tE2+hRVvwSFghRb75AJ5g-nqGzDUqLYMexjOAe=g@mail.gmail.com Backpatch-through: 13
In the passing, fix a typo in pgoutput.c. Reported-by: Tomas Vondra Author: Tomas Vondra Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/20200909084353.pncuclpbwlr7vylh@development
We use the timestamp of the global statfile if we are not able to determine it for a particular database in case the entry for that database doesn't exist. However, we were using it even when the statfile is corrupt. As there is no user reported issue and it is not clear if there is any impact of this on actual application so decided not to backpatch. Reported-by: Amit Kapila Author: Amit Kapila Reviewed-by: Sawada Masahiko, Magnus Hagander and Alvaro Herrera Discussion: https://postgr.es/m/CAA4eK1J3oTJKyVq6v7K4d3jD+vtnruG9fHRib6UuWWsrwAR6Aw@mail.gmail.com
transformCreateStmt() adjusts the transformed statement's RangeVar to specify the target schema explicitly, for the express reason of making sure that auxiliary statements derived by parse transformation operate on the right table. But the refactoring I did in commit 5028981 got this wrong and passed the untransformed RangeVar to expandTableLikeClause(). This could lead to assertion failures or weird misbehavior if the wrong table was accessed. Per report from Alexander Lakhin. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/05051f9d-b32b-cb35-6735-0e9f2ab86b5f@gmail.com
…ump/ If there are no objects of a certain type, there is no need to do an allocation for a set of DumpableObject items. The previous coding did an allocation of 1 byte instead as per the fallback of pg_malloc() in the event of an allocation size of zero. This assigns NULL instead for a set of dumpable objects. A similar rule already applied to findObjectByOid(), so this makes the code more defensive as we would just fail with a pointer dereference instead of attempting to use some incorrect data if a non-existing, positive, OID is given by a caller of this function. Author: Daniel Gustafsson Reviewed-by: Julien Rouhaud, Ranier Vilela Discussion: https://postgr.es/m/26C43E58-BDD0-4F1A-97CC-4A07B52E32C5@yesql.se
3c84046 is the original commit that introduced index_set_state_flags(), where the presence of SnapshotNow made necessary the use of an in-place update. SnapshotNow has been removed in 813fb03, so there is no actual reasons to not make this operation transactional. Note that while making the operation more robust, using a transactional operation in this routine was not strictly necessary as there was no use case for it yet. However, some future features are going to need a transactional behavior, like support for CREATE/DROP INDEX CONCURRENTLY with partitioned tables, where indexes in a partition tree need to have all their pg_index.indis* flags updated in the same transaction to make the operation stable to the end-user by keeping partition trees consistent, even with a failure mid-flight. REINDEX CONCURRENTLY uses already transactional updates when swapping the old and new indexes, making this change more consistent with the index-swapping logic. Author: Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/20200903080440.GA8559@paquier.xyz
Author: Naoki Nakamichi Discussion: https://postgr.es/m/b6919d145af00295a8e86ce4d034b7cd@oss.nttdata.com
A pre-commit review had reported the problem, but the fix reached only v10 and earlier. Back-patch to v11. Discussion: https://postgr.es/m/20200423.140546.1055476118690602079.horikyota.ntt@gmail.com
A walsender process that has executed a SQL command left the text of that command in pg_stat_activity.query indefinitely, which is quite confusing if it's in RUNNING state but not doing that query. An easy and useful fix is to treat replication commands as if they were SQL queries, and show them in pg_stat_activity according to the same rules as for regular queries. While we're at it, it seems also sensible to set debug_query_string, allowing error logging and debugging to see the replication command. While here, clean up assorted silliness in exec_replication_command: * The SQLCmd path failed to restore CurrentMemoryContext to the caller's value, and failed to delete the temp context created in this routine. It's only through great good fortune that these oversights did not result in long-term memory leaks or other problems. It seems cleaner to code SQLCmd as a separate early-exit path, so do it like that. * Remove useless duplicate call of SnapBuildClearExportedSnapshot(). * replication_scanner_finish() was never called. None of those things are significant enough to merit a backpatch, so this is for HEAD only. Discussion: https://postgr.es/m/880181.1600026471@sss.pgh.pa.us
Introduced in 0aa8f76. MSVC warned about performing 32-bit bit shifting when it appeared like we might like a 64-bit result. We did, but it just so happened that none of the calls to this function could have caused the 32-bit shift to overflow. Here we just cast the constant to int64 to make the compiler happy. Discussion: https://postgr.es/m/CAApHDvofA_vsrpC13mq_hZyuye5B-ssKEaer04OouXYCO5-uXQ@mail.gmail.com
This expands on the work done in d2d8a22 and allows incremental sort to be considered during create_window_paths(). Author: David Rowley Reviewed-by: Daniel Gustafsson, Tomas Vondra Discussion: https://postgr.es/m/CAApHDvoOHobiA2x13NtWnWLcTXYj9ddpCkv9PnAJQBMegYf_xw%40mail.gmail.com
Reporting this has been rather useful in some recent recovery speedup work. It also seems like something that will be useful to the average DBA too. Author: David Rowley Reviewed-by: Thomas Munro Discussion: https://postgr.es/m/CAApHDvqYVORiZxq2xPvP6_ndmmsTkvr6jSYv4UTNaFa5i1kd%3DQ%40mail.gmail.com
ALTER TABLE commands in an extension script are added to an event trigger command list; but starting with commit b5810de they do so in a memory context that's too short-lived, so when execution ends and time comes to use the entries, they've already been freed. (This would also be a problem with ALTER TABLE commands in a multi-command query string, but these serendipitously end in PortalContext -- which probably explains why it took so long for this to be reported.) Fix by using the memory context specifically set for that, instead. Backpatch to 13, where the aforementioned commit appeared. Reported-by: Philippe Beaudoin Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20200902193715.6e0269d4@firost
This function could often be seen in profiles of vacuum and could often be a significant bottleneck during recovery. The problem was that a qsort was performed in order to sort an array of item pointers in reverse offset order so that we could use that to safely move tuples up to the end of the page without overwriting the memory of yet-to-be-moved tuples. i.e. we used to compact the page starting at the back of the page and move towards the front. The qsort that this required could be expensive for pages with a large number of tuples. In this commit, we take another approach to tuple compactification. Now, instead of sorting the remaining item pointers array we first check if the array is presorted and only memmove() the tuples that need to be moved. This presorted check can be done very cheaply in the calling functions when the array is being populated. This presorted case is very fast. When the item pointer array is not presorted we must copy tuples that need to be moved into a temp buffer before copying them back into the page again. This differs from what we used to do here as we're now copying the tuples back into the page in reverse line pointer order. Previously we left the existing order alone. Reordering the tuples results in an increased likelihood of hitting the pre-sorted case the next time around. Any newly added tuple which consumes a new line pointer will also maintain the correct sort order of tuples in the page which will also result in the presorted case being hit the next time. Only consuming an unused line pointer can cause the order of tuples to go out again, but that will be corrected next time the function is called for the page. Benchmarks have shown that the non-presorted case is at least equally as fast as the original qsort method even when the page just has a few tuples. As the number of tuples becomes larger the new method maintains its performance whereas the original qsort method became much slower when the number of tuples on the page became large. Author: David Rowley Reviewed-by: Thomas Munro Tested-by: Jakub Wartak Discussion: https://postgr.es/m/CA+hUKGKMQFVpjr106gRhwk6R-nXv0qOcTreZuQzxgpHESAL6dw@mail.gmail.com
Since commit 913bbd8, 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
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 a45bc8a 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
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
More precisely, correctly handle the ONLY flag indicating not to recurse. This was implemented in 86f5759 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
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
After de8feb1, 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.
Commit 8dace66 added #ifdefs for a number of errno symbols because they were not present on Windows. Later, commit 125ad53 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
Theoretically one could go into src/test/thread and build/run this program there. In practice, that hasn't worked since 96bf88d, 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
Should cause tests to be a bit faster
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 a45bc8a and 8e5793a, 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
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
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
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
Mark Dilger, with a couple of stray semicolons removed by me. Discussion: http://postgr.es/m/2A7DA1A8-C4AA-43DF-A985-3CA52F4DC775@enterprisedb.com
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
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
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
DST law changes in Palestine, with a whopping 120 hours' notice. Also some historical corrections for Palestine.
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
Thinko in commit 1375422. 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
The behavioural change in the -t/--table option happened around 15 years ago and there seems little point in keeping it around. Author: Ian Barwick Discussion: https://www.postgresql.org/message-id/CAB8KJ%3Dh-XALik4M7gv-pX48%3D%2BSPWexfaYwa%2ByTnPwD3DxceXrg%40mail.gmail.com
The order of AuthenticationGSSContinue and AuthenticationSSPI was swapped, based on the other Authentication* protocol messages being listed in subcode order.
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
The tests added by commit 866e24d failed on big-endian machines due to lack of attention to endianness considerations. Fix that. While here, improve a few small cosmetic things, such as running it through perltidy. Mark Dilger Discussion: https://postgr.es/m/30B8E99A-2D9C-48D4-A55C-741C9D5F1563@enterprisedb.com
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
verify_heapam() wasn't being careful to sanity-check tuple line pointers before using them, resulting in SIGBUS on alignment-picky architectures. Fix that, add some more test coverage. Mark Dilger, some tweaking by me Discussion: https://postgr.es/m/30B8E99A-2D9C-48D4-A55C-741C9D5F1563@enterprisedb.com
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
…on code 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.