fb32748e32e2b6b2fcb32220980b93d5436f855e^..b6074846cebc33d752f1d9a66e5a9932f21ad177 #84
Conversation
This file contains 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
BABELFISH-CONFLICT: see Postgres community repo for original commit This commit changes six SQL keywords to use COERCE_SQL_SYNTAX rather than relying on SQLValueFunction: - CURRENT_ROLE - CURRENT_USER - USER - SESSION_USER - CURRENT_CATALOG - CURRENT_SCHEMA Among the six, "user", "current_role" and "current_catalog" require specific SQL functions to allow ruleutils.c to map them to the SQL keywords these require when using COERCE_SQL_SYNTAX. Having pg_proc.proname match with the keyword ensures that the compatibility remains the same when projecting any of these keywords in a FROM clause to an attribute name when an alias is not specified. This is covered by the tests added in 2e0d80c, making sure that a correct mapping happens with each SQL keyword. The three others (current_schema, session_user and current_user) already have pg_proc entries for this job, so this brings more consistency between the way such keywords are treated in the parser, the executor and ruleutils.c. SQLValueFunction is reduced to half its contents after this change, simplifying its logic a bit as there is no need to enforce a C collation anymore for the entries returning a name as a result. I have made a few performance tests, with a million-ish calls to these keywords without seeing a difference in run-time or in perf profiles (ExecEvalSQLValueFunction() is removed from the profiles). The remaining SQLValueFunctions are now related to timestamps and dates. Bump catalog version. Reviewed-by: Corey Huinker Discussion: https://postgr.es/m/[email protected] (cherry picked from commit fb32748e32e2b6b2fcb32220980b93d5436f855e)
Currently there is a race condition where if concurrent TAP tests both test that they can open a port they will assume that it is free and use it, causing one of them to fail. To prevent this we record a reservation using an exclusive lock, and any TAP test that discovers a reservation checks to see if the reserving process is still alive, and looks for another free port if it is. Ports are reserved in a directory set by the environment setting PG_TEST_PORT_DIR, or if that doesn't exist a subdirectory of the top build directory as set by meson or Makefile.global, or its own tmp_check directory. The prove_check recipe in Makefile.global.in is extended to export top_builddir to the TAP tests. This was already exported by the prove_installcheck recipes. Per complaint from Andres Freund This will be backpatched in due course after some testing. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 9b4eafcaf41d1192a34b574c21262b755aa455ee)
Commit c94959d fixed DROP OPERATOR to reset oprcom/oprnegate links to the dropped operator; but it missed updating this old comment that claimed we allow such links to dangle. (cherry picked from commit 75b8d3de989eb88a5960c93cddd88caf5d245024)
Commit 3d14e171e dropped regress_roleoption_donor twice and regress_roleoption_protagonist not at all. Leaving roles behind after "make installcheck" is unfriendly in its own right, plus it causes repeated runs of "make installcheck" to fail. (cherry picked from commit b62303794efd97f2afb55f1e1b82fffae2cf8a2d)
As the list of shared relations is fixed, we can just dispatch based IsSharedRelation(), instead of first trying to look up stats for a non-shared rel and falling back to shared stats. Author: "Drouvot, Bertrand" <[email protected]> Reviewed-by: Bharath Rupireddy <[email protected]> Reviewed-by: Andres Freund <[email protected] Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 061bf98fb8f468b9a8c9fb617bb30136db1cc812)
Until now LWLockDequeueSelf() sequentially searched the list of waiters to see if the current proc is still is on the list of waiters, or has already been removed. In extreme workloads, where the wait lists are very long, this leads to a quadratic behavior. #backends iterating over a list #backends long. Additionally, the likelihood of needing to call LWLockDequeueSelf() in the first place also increases with the increased length of the wait queue, as it becomes more likely that a lock is released while waiting for the wait list lock, which is held for longer during lock release. Due to the exponential back-off in perform_spin_delay() this is surprisingly hard to detect. We should make that easier, e.g. by adding a wait event around the pg_usleep() - but that's a separate patch. The fix is simple - track whether a proc is currently waiting in the wait list or already removed but waiting to be woken up in PGPROC->lwWaiting. In some workloads with a lot of clients contending for a small number of lwlocks (e.g. WALWriteLock), the fix can substantially increase throughput. As the quadratic behavior arguably is a bug, we might want to decide to backpatch this fix in the future. Author: Andres Freund <[email protected]> Reviewed-by: Bharath Rupireddy <[email protected]> Discussion: https://postgr.es/m/[email protected] Discussion: https://postgr.es/m/CALj2ACXktNbG=K8Xi7PSqbofTZozavhaxjatVc14iYaLu4Maag@mail.gmail.com (cherry picked from commit a4adc31f6902f6fc29d74868e8969412fc590da9)
As per one of the CI reports, there is an assertion failure which indicates that we were trying to use an unenforced xmin horizon for decoding snapshots. Though, we couldn't figure out the reason for assertion failure these checks would help us in finding the reason if the problem happens again in the future. Author: Amit Kapila based on suggestions by Andres Freund Reviewd by: Andres Freund Discussion: https://postgr.es/m/CAA4eK1L8wYcyTPxNzPGkhuO52WBGoOZbT0A73Le=ZUWYAYmdfw@mail.gmail.com (cherry picked from commit 240e0dbacd390a8465552e27c5af11f67d747adb)
BABELFISH-CONFLICT: see Postgres community repo for original commit This switch impacts 9 patterns related to a SQL-mandated special syntax for function calls: - LOCALTIME [ ( typmod ) ] - LOCALTIMESTAMP [ ( typmod ) ] - CURRENT_TIME [ ( typmod ) ] - CURRENT_TIMESTAMP [ ( typmod ) ] - CURRENT_DATE Five new entries are added to pg_proc to compensate the removal of SQLValueFunction to provide backward-compatibility and making this change transparent for the end-user (for example for the attribute generated when a keyword is specified in a SELECT or in a FROM clause without an alias, or when specifying something else than an Iconst to the parser). The parser included a set of checks coming from the files in charge of holding the C functions used for the SQLValueFunction calls (as of transformSQLValueFunction()), which are now moved within each function's execution path, so this reduces the dependencies between the execution and the parsing steps. As of this change, all the SQL keywords use the same paths for their work, relying only on COERCE_SQL_SYNTAX. Like fb32748, no performance difference has been noticed, while the perf profiles get reduced with ExecEvalSQLValueFunction() gone. Bump catalog version. Reviewed-by: Corey Huinker, Ted Yu Discussion: https://postgr.es/m/[email protected] (cherry picked from commit f193883fc9cebe8fa20359b0797832837a788112)
pageinspect has occasionally failed on slow buildfarm members, with symptoms indicating that the expected effects of VACUUM FREEZE didn't happen. This is presumably because a background transaction such as auto-analyze was holding back global xmin. We can work around that by using a temp table in the test. Since commit a7212be, that will use an up-to-date cutoff xmin regardless of other processes. And pageinspect itself shouldn't really care whether the table is temp. Back-patch to v14. There would be no point in older branches without back-patching a7212be, which seems like more trouble than the problem is worth. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit e2933a6e11791191050cd925d52d34e785eece77)
BABELFISH-CONFLICT: see Postgres community repo for original commit The postmaster normally sends SIGQUIT to force-terminate its child processes after a child crash or immediate-stop request. If that doesn't result in child exit within a few seconds, we follow it up with SIGKILL. This patch provides GUC flags that allow either of these signals to be replaced with SIGABRT. On typically-configured Unix systems, that will result in a core dump being produced for each such child. This can be useful for debugging problems, although it's not something you'd want to have on in production due to the risk of disk space bloat from lots of core files. The old postmaster -T switch, which sent SIGSTOP in place of SIGQUIT, is changed to be the same as send_abort_for_crash. As far as I can tell from the code comments, the intent of that switch was just to block things for long enough to force core dumps manually, which seems like an unnecessary extra step. (Maybe at the time, there was no way to get most kernels to produce core files with per-PID names, requiring manual core file renaming after each one. But now it's surely the hard way.) I also took the opportunity to remove the old postmaster -n (skip shmem reinit) switch, which hasn't actually done anything in decades, though the documentation still claimed it did. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 51b5834cd53f0bd068729043b55f7da3ca6bb15f)
These functions have been marked parallel safe, but the buildfarm's response to commit e2933a6e1 exposed the flaw in that thinking: if you try to use them on a temporary table, and they run inside a parallel worker, they'll fail with "cannot access temporary tables during a parallel operation". Fix that by marking them parallel restricted instead. Maybe someday we'll have a better answer and can reverse this decision. Back-patch to v15. To go back further, we'd have to devise variant versions of pre-1.10 pageinspect versions. Given the lack of field complaints, it doesn't seem worth the trouble. We'll just deem this case unsupported pre-v15. (If anyone does complain, it might be good enough to update the markings manually in their DBs.) Discussion: https://postgr.es/m/[email protected] (cherry picked from commit aeaaf520f409cf314f97c811d2713c99858f035d)
At least on linux, set_ps_display() breaks /proc/$pid/environ. The sanitizer's
helper library uses /proc/$pid/environ to implement getenv(), as it wants to
work independent of libc. When just using undefined and alignment sanitizers,
the sanitizer library is only initialized when the first error occurs, by
which time we've often already called set_ps_display(), preventing the
sanitizer libraries from seeing the options.
We can work around that by defining __ubsan_default_options, a weak symbol
libsanitizer uses to get defaults from the application, and return
getenv("UBSAN_OPTIONS"). But only if main already was reached, so that we
don't end up relying on a not-yet-working getenv().
As it's just a function that won't get called when not running a sanitizer, it
doesn't seem necessary to make compilation of the function conditional.
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/[email protected]
(cherry picked from commit f686ae82f2d7e29facde8300209deef7abe13605)
I just spent an annoying amount of time reverse-engineering the 100%-undocumented API between ts_headline and the text search parser's prsheadline function. Add some commentary about that while it's fresh in mind. Also remove some unused macros in wparser_def.c. While at it, I noticed that when commit 78e73e8 added a CHECK_FOR_INTERRUPTS call in TS_execute_recurse, it missed doing so in the parallel function TS_phrase_execute, which surely needs one just as much. Back-patch because of the missing CHECK_FOR_INTERRUPTS. Might as well back-patch the rest of this too. (cherry picked from commit 5644d6f909b1bc92155741064275cda82d9b9b70)
The Hunspell project moved from Sourceforge to Github sometime in 2016, so update our links to match the new URL. Backpatch the doc changes to all supported versions. Discussion: https://postgr.es/m/[email protected] Backpatch-through: v11 (cherry picked from commit f1d042b21dfdac069601fe97b7b7ba572dd0abe2)
We don't need CIRRUS_ESCAPING_PROCESSES anymore as the whole tests now run within a single script: block. We don't need NO_TEMP_INSTALL anymore it was addressing an issue specific to vcregress.pl. Author: Justin Pryzby <[email protected]> Discussion: https://postgr.es/m/[email protected] (cherry picked from commit ec267fd5a5e85223979a90c814e1a98053ca7f3b)
To avoid unnecessarily spinning up a lot of VMs / containers for entirely broken commits, have a minimal task that all others depend on. The concrete motivation for the change is to use sanitizers in the linux tasks. As that makes the tests slower, the start of the CompilerWarnings would be delayed even more. With this change the CompilerWarnings only depends on the SanityCheck task. This has the added advantage that now the CompilerWarnings task is not prevented from running by (most) test failures (particularly annoying when caused by a test that is flappy in HEAD). Reviewed-by: Justin Pryzby <[email protected]> Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 94a3e026cc4da4c5a3f82d02ae2c62c9f060788a)
We have coverage of the various sanitizers in the buildfarm. The sanitizers however particularly interesting during the development of patches, where the likelihood of bugs is even higher. There also have been complaints about only seeing such failures on the buildfarm, rather than before commit. This commit enables a reasonable set of sanitizers in CI. Use the linux task for that, as it currently is one of the fastests tasks. Also several of the sanitizers work best on linux. The overhead of alignment sanitizer is low, undefined behaviour has moderate overhead. Test alignment sanitizer in the meson task, as it does both 32 and 64 bit builds and is thus more likely to expose alignment bugs. Address sanitizer in contrast somewhat expensive. Enable it in the autoconf task, as the meson task tests both 32 and 64bit which would exacerbate the cost. Reviewed-by: Justin Pryzby <[email protected]> Discussion: https://postgr.es/m/[email protected] Discussion: https://postgr.es/m/[email protected] (cherry picked from commit bd82928625e7ebe40eaaa57f3b4d03bae19491de)
The lwlock wait queue scalability issue fixed in a4adc31f690 was quite hard to find because of the exponential backoff and because we adjust spins_per_delay over time within a backend. To make it easier to find similar issues in the future, add a wait event for the pg_usleep() in perform_spin_delay(). Showing a wait event while spinning without sleeping would increase the overhead of spinlocks, which we do not want. We may at some later point want to have more granular wait events, but that'd be a substantial amount of work. This provides at least some insights into something currently hard to observe. Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Robert Haas <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> https://postgr.es/m/[email protected] (cherry picked from commit 92daeca45df6551dd85f92f7369eaa57a35fb8a9)
Once a logical slot has acquired a catalog_xmin, it doesn't let go of it, even when invalidated by exceeding the max_slot_wal_keep_size, which means that dead catalog tuples are not removed by vacuum anymore since the point is invalidated, until the slot is dropped. This could be catastrophic if catalog churn is high. Change the computation of Xmin to ignore invalidated slots, to prevent dead rows from accumulating. Backpatch to 13, where slot invalidation appeared. Author: Sirisha Chamarthi <[email protected]> Reviewed-by: Ashutosh Bapat <[email protected]> Discussion: https://postgr.es/m/CAKrAKeUEDeqquN9vwzNeG-CN8wuVsfRYbeOUV9qKO_RHok=j+g@mail.gmail.com (cherry picked from commit 0557e1770230fe5ca855fdf45bb297bd38a9ec1b)
Update comments atop pg_get_replication_slots to make it clear that it shows all replication slots that currently exist on the database cluster. Author: sirisha chamarthi Discussion: https://postgr.es/m/CAKrAKeXRuFpeiWS+STGFm-RFfW19sUDxju66JkyRi13kdQf94Q@mail.gmail.com (cherry picked from commit a1efcda7c33d10658dac214514ca8359a1da4e42)
This was trying to exercise an ERROR we don't actually have. Backpatch to 15. Reported by Teja Mupparti <[email protected]> Discussion: https://postgr.es/m/SN6PR2101MB1040BDAF740EA4389484E92BF0079@SN6PR2101MB1040.namprd21.prod.outlook.com (cherry picked from commit 0538d4c0c33551029f408fdc29ee51b817632e11)
We've made multiple attempts at preventing get_actual_variable_range from taking an unreasonable amount of time (3ca930f, fccebe4). But there's still an issue for the very first planning attempt after deletion of a large number of extremal-valued tuples. While that planning attempt will set "killed" bits on the tuples it visits and thereby reduce effort for next time, there's still a lot of work it has to do to visit the heap and then set those bits. It's (usually?) not worth it to do that much work at plan time to have a slightly better estimate, especially in a context like this where the table contents are known to be mutating rapidly. Therefore, let's bound the amount of work to be done by giving up after we've visited 100 heap pages. Giving up just means we'll fall back on the extremal value recorded in pg_statistic, so it shouldn't mean that planner estimates suddenly become worthless. Note that this means we'll still gradually whittle down the problem by setting a few more index "killed" bits in each planning attempt; so eventually we'll reach a good state (barring further deletions), even in the absence of VACUUM. Simon Riggs, per a complaint from Jakub Wartak (with cosmetic adjustments by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKZiRmznOwi0oaV=4PHOCM4ygcH4MgSvt8=5cu_vNCfc8FSUug@mail.gmail.com (cherry picked from commit 9c6ad5eaa957bdc2132b900a96e0d2ec9264d39c)
Examine ParseNamespaceItem flags to detect whether a column name is unreferenceable for lack of LATERAL, or could be referenced if a qualified name were used, and give better hints for such cases. Also, don't phrase the message to imply that there's only one matching column when there is really more than one. Many of the regression test output changes are not very interesting, but just reflect reclassifying the "There is a column ... but it cannot be referenced from this part of the query" messages as DETAIL rather than HINT. They are details per our style guide, in the sense of being factual rather than offering advice; and this change provides room to offer actual HINTs about what to do. While here, adjust the fuzzy-name-matching code to be a shade less impenetrable. It was overloading the meanings of FuzzyAttrMatchState fields way too much IMO, so splitting them into multiple fields seems to make it clearer. It's not like we need to shave bytes in that struct. Per discussion of bug #17233 from Alexander Korolev. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 56d0ed3b756b2e3799a7bbc0ac89bc7657ca2c33)
per gripe from Andres Freund and Tom Lane Backpatch to all live branches. (cherry picked from commit b425bf0081386a544e1faf872a75da69a971e173)
We shouldn't ever need to rely on whether HEAP_XMAX_INVALID is set in t_infomask when considering whether or not an xmax should be deemed already frozen, since that status flag is just a hint. The only acceptable representation for an "xmax_already_frozen" raw xmax field is the transaction ID value zero (also known as InvalidTransactionId). Adjust code that superficially appeared to rely on HEAP_XMAX_INVALID to make the rule about xmax_already_frozen clear. Also avoid needlessly rereading the tuple's raw xmax. Oversight in bugfix commit d2599ec. There is no evidence that this ever led to incorrect behavior, so no backpatch. The worst consequence of this bug was that VACUUM could hypothetically fail to notice and report on certain kinds of corruption, which seems fairly benign. Author: Peter Geoghegan <[email protected]> Discussion: https://postgr.es/m/CAH2-Wzkh3DMCDRPfhZxj9xCq9v3WmzvmbiCpf1dNKUBPadhCbQ@mail.gmail.com (cherry picked from commit 02d647bbf0576ebb87f9dc24e1db4dd034f04048)
Pass VACUUM parameters (VacuumParams state) to vacuum_set_xid_limits() directly, rather than passing most individual VacuumParams fields as separate arguments. Also make vacuum_set_xid_limits() output parameter symbol names match those used by its vacuumlazy.c caller. Author: Peter Geoghegan <[email protected]> Discussion: https://postgr.es/m/CAH2-Wz=TE7gW5DgSahDkf0UEZigFGAoHNNN6EvSrdzC=Kn+hrA@mail.gmail.com (cherry picked from commit b6074846cebc33d752f1d9a66e5a9932f21ad177)
We're running out of bits for new permissions. This change doubles the number of permissions we can accomodate from 16 to 32, so the forthcoming new ones for vacuum/analyze don't exhaust the pool. Nathan Bossart Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael Paquier. Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13 (cherry picked from commit 7b378237aa805711353075de142021b1d40ff3b0)
This will more easily accomodate adding new permissions for vacuum and analyze. Nathan Bossart following a suggestion from Kyotaro Horiguchi Discussion: https://postgr.es/m/[email protected] (cherry picked from commit b7a5ef17cf75c712b0fe5c5a20133a88da897aab)
Some custom table access method may have their tuple format and use custom executor nodes for their custom scan types. The ability to set a custom slot would save them from tuple format conversion. Other users of custom executor nodes may also benefit. Discussion: https://postgr.es/m/CAPpHfduJUU6ToecvTyRE_yjxTS80FyPpct4OHaLFk3OEheMTNA@mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov (cherry picked from commit cee120951427fe39a54ab800abfa2834d85b8771)
The list of TokenizedAuthLines generated at parsing for the HBA and ident files is now stored in a static context called tokenize_context, where only all the parsed tokens are stored. This context is created when opening the first authentication file of a HBA/ident set (hba_file or ident_file), and is cleaned up once we are done all the work around it through a new routine called free_auth_file(). One call of open_auth_file() should have one matching call of free_auth_file(), the creation and deletion of the tokenization context is controlled by the recursion depth of the tokenization. Rather than having tokenize_auth_file() return a memory context that includes all the records, the tokenization logic now creates and deletes one memory context each time this function is called. This will simplify recursive calls to this routine for the upcoming inclusion record logic. While on it, rename tokenize_inc_file() to tokenize_expand_file() as this would conflict with the upcoming patch that will add inclusion records for HBA/ident files. An '@' file has its tokens added to an existing list. Reloading HBA/indent configuration in a tight loop shows no leaks, as of one type of test done (with and without -DEXEC_BACKEND). Author: Michael Paquier Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/[email protected] (cherry picked from commit efc981627a723d91e86865fb363d793282e473d1)
Up to now we have allowed manual creation of an ON SELECT rule on a table to convert it into a view. That was never anything but a horrid, error-prone hack though. pg_dump used to rely on that behavior to deal with cases involving circular dependencies, where a dependency loop could be broken by separating the creation of a view from installation of its ON SELECT rule. However, we changed pg_dump to use CREATE OR REPLACE VIEW for that in commit d8c05af (which was later back-patched as far as 9.4), so there's not a good argument anymore for continuing to support the behavior. The proximate reason for axing it now is that we found that the new statistics code has failure modes associated with the relkind change caused by this behavior. We'll patch around that in v15, but going forward it seems like a better idea to get rid of the need to support relkind changes. Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com (cherry picked from commit b23cd185fd5410e5204683933f848d4583e34b35)
Some options of these commands need to be able to identify the start of the function body within the output of pg_get_functiondef(). It used to be that that always began with "AS", but since the introduction of new-style SQL functions, it might also start with "BEGIN" or "RETURN". Fix that on the psql side, and add some regression tests. Noted by me awhile ago, but I didn't do anything about it. Thanks to David Johnston for a nag. Discussion: https://postgr.es/m/AM9PR01MB8268D5CDABDF044EE9F42173FE8C9@AM9PR01MB8268.eurprd01.prod.exchangelabs.com (cherry picked from commit cabfb8241dea84206b90578a769e9cbd813ba477)
Check that if we generate a call to copy, compare, write, or read a specific node type, that node type does have the appropriate support function. (This doesn't protect against trying to invoke nonexistent code when considering generic field types such as "Node *", but it seems like a useful check anyway.) Check that array_size() refers to a field appearing earlier in the struct. Aside from catching obvious errors like a misspelled field name, this protects against a more subtle mistake: if the size field appears later in the struct than the array field, then compare and read functions would misbehave. There is actually exactly that situation in PlannerInfo, but it's okay since we do not need compare or read functionality for that (today anyway). Discussion: https://postgr.es/m/[email protected] (cherry picked from commit b6bd5def3a6382995634d33f46d20e191a475914)
It seems better to deal with this by explicit annotations on the fields in question, instead of magic knowledge embedded in the script. While that creates a risk-of-omission from failing to annotate fields, the preceding commit should catch any such oversights. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 4c689a69eef639caa881539ee546ff1a5b11f98f)
Ignore trailing spaces for non-deterministic collations when hashing. The previous behavior could lead to tuples falling into the wrong partitions when hash partitioning is combined with the BPCHAR type and a non-deterministic collation. Fortunately, it did not affect hash indexes, because hash indexes do not use extended hash functions. Decline to backpatch, per discussion. Discussion: https://postgr.es/m/[email protected] Reviewed-by: Richard Guo, Tom Lane (cherry picked from commit 7ac0f8d384a4633c6652ae1f6bba40d42d21ec18)
When the relkind of a relache entry changes, because a table is converted into a view, pgstats can get confused in 15+, leading to crashes or assertion failures. For HEAD, Tom fixed this in b23cd185fd5, by removing support for converting a table to a view, removing the source of the inconsistency. This commit just adds an assertion that a relcache entry's relkind does not change, just in case we end up with another case of that in the future. As there's no cases of changing relkind anymore, we can't add a test that that's handled correctly. For 15, fix the problem by not maintaining the association with the old pgstat entry when the relkind changes during a relcache invalidation processing. In that case the pgstat entry needs to be unlinked first, to avoid PgStat_TableStatus->relation getting out of sync. Also add a test reproducing the issues. No known problem exists in 11-14, so just add the test there. Reported-by: vignesh C <[email protected]> Author: Andres Freund <[email protected]> Reviewed-by: Tom Lane <[email protected]> Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com Discussion: https://postgr.es/m/CALDaNm3oZA-8Wbps2Jd1g5_Gjrr-x3YWrJPek-mF5Asrrvz2Dg@mail.gmail.com Backpatch: 15- (cherry picked from commit cb2e7ddfe571e2a158725200a33f728232059c2e)
This is present in the declaration for ReadDataFromArchive, so we'd better have it in the definition too in order to avoid compilers from complaining about the mismatch of function signatures. (cherry picked from commit f73bd5fd081187a6515d0f0764730958ad6ba41a)
I wrote this to provide a home for a planned discussion of error return conventions for non-error-throwing functions. But it seems useful as documentation of existing code no matter what becomes of that proposal, so commit separately. (cherry picked from commit 29452de7341b0effbc275d7e139ade83cebda69f)
Experiments have shown that modern versions of both gcc and clang are unable to fully optimize the multiplication by 10 that we're doing in the pg_strtointNN functions. Both compilers seem to be making use of "imul", which is not the most efficient way to multiply by 10. This seems to be due to the overflow checking that we're doing. Without the overflow checks, both those compilers switch to a more efficient method of multiplying by 10. In absence of overflow concern, integer multiplication by 10 can be done by bit-shifting left 3 places to multiply by 8 and then adding the original value twice. To allow compilers this flexibility, here we adjust the code so that we accumulate the number as an unsigned version of the type and remove the use of pg_mul_sNN_overflow() and pg_sub_sNN_overflow(). The overflow checking can be done simply by checking if the accumulated value has gone beyond a 10th of the maximum *signed* value for the given type. If it has then the accumulation of the next digit will cause an overflow. After this is done, we do a final overflow check before converting the unsigned version of the number back to its signed counterpart. Testing has shown about an 8% speedup of a COPY into a table containing 2 INT columns. Author: David Rowley, Dean Rasheed Discussion: https://postgr.es/m/CAApHDvrL6_+wKgPqRHr7gH_6xy3hXM6a3QCsZ5ForurjDFfenA@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvrdYByjfj-=WbmVNFgmVZg88-dE7heukw8p55aJ+W=qxQ@mail.gmail.com (cherry picked from commit 6b423ec677d67c120d13a865fe3962985f0104c8)
We might fail to generate a partitionwise join, because reparameterize_path_by_child() does not support all path types. This should not be a hard failure condition: we should just fall back to a non-partitioned join. However, generate_partitionwise_join_paths did not consider this possibility and would emit the (misleading) error "could not devise a query plan for the given query" if we'd failed to make any paths for a child join. Fix it to give up on partitionwise joining if so. (The accepted technique for giving up appears to be to set rel->nparts = 0, which I find pretty bizarre, but there you have it.) I have not added a test case because there'd be little point: any omissions of this sort that we identify would soon get fixed by extending reparameterize_path_by_child(), so the test would stop proving anything. However, right now there is a known test case based on failure to cover MaterialPath, and with that I've found that this is broken in all supported versions. Hence, patch all the way back. Original report and patch by me; thanks to Richard Guo for identifying a test case that works against committed versions. Discussion: https://postgr.es/m/[email protected] (cherry picked from commit fe12f2f8fa608156c2d3c027cdd6aa9af0788e3a)
These two functions failed to cover MaterialPath. That's not a fatal problem, but we can generate better plans in some cases if we support it. Tom Lane and Richard Guo Discussion: https://postgr.es/m/[email protected] (cherry picked from commit 6eb2f0ed4cd5c8668c3127024a8a58b10fa2e8dc)
It neglected to recurse to the subpath, meaning you'd get back a path identical to the input. This could produce wrong query results if the omission meant that the subpath fails to enforce some join clause it should be enforcing. We don't have a test case for this at the moment, but the code is obviously broken and the fix is equally obvious. Back-patch to v14 where Memoize was introduced. Richard Guo Discussion: https://postgr.es/m/CAMbWs4_R=ORpz=Lkn2q3ebPC5EuWyfZF+tmfCPVLBVK5W39mHA@mail.gmail.com (cherry picked from commit e76913802c5347831aadc1513b59196bde6ec116)
Just because I'm a neatnik, and I'm currently working on code in this area. It annoys me to not be able to pgindent my patches without working around unrelated changes. (cherry picked from commit 92c4dafe1eed511c5af92bcea5311cf627673377)
Add a comment explaining our policy that we don't put excludes for nonstandard tools into committed .gitignore files. Also, remove the entries about libraries with .sl extensions, since that convention was only used on now-desupported HP-UX. Discussion: https://postgr.es/m/CAHxW8BAiyPwfXbN813GhorQozwMBs4f3DTxLkKNxiGQuJuw4Vw@mail.gmail.com (cherry picked from commit d94f32d49f620fb08c1fd0e8c9345844ccd9b7c0)
As pointed out by Dean Rasheed, we really should be using tmp > -(PG_INTNN_MIN / 10) rather than tmp > (PG_INTNN_MAX / 10) for checking for overflows in the accumulation in the pg_strtointNN functions. This does happen to be the same number when dividing by 10, but there is a pending patch which adds other bases and this is not the same number if we were to divide by 2 rather than 10, for example. If the base 2 parsing was to follow this example then we could accidentally think a string containing the value of PG_INT32_MIN was an overflow in pg_strtoint32. Clearly that shouldn't overflow. This does not fix any actual live bugs, only some bad examples of overflow checks for future bases. Reported-by: Dean Rasheed Discussion: https://postgr.es/m/CAEZATCVEtwfhdm-K-etZYFB0=qsR0nT6qXta_W+GQx4RYph1dg@mail.gmail.com (cherry picked from commit 8692f6644e71f51db9a853a652f37909c9cb534d)
The error messages reported during any failures while reading or validating the header of a WAL currently includes only the offset of the page but not the compiled LSN referring to the page, requiring an extra step to compile it if looking at the surroundings with pg_waldump or similar. Adding this information costs a bit in translation, but also eases debugging. Author: Bharath Rupireddy Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, Maxim Orlov, Michael Paquier Discussion: https://postgr.es/m/CALj2ACWV=FCddsxcGbVOA=cvPyMr75YCFbSQT6g4KDj=gcJK4g@mail.gmail.com (cherry picked from commit 71cb84ec69a38444c48bd8d3b5451b2da157848b)
Missing such markups makes it impossible to create links back to these GUCs, and all the other parameters have one already. Author: Ian Lawrence Barwick Discussion: https://postgr.es/m/CAB8KJ=jx=6dFB_EN3j0UkuvG3cPu5OmQiM-ZKRAz+fKvS+u8Ng@mail.gmail.com Backpatch-through: 11 (cherry picked from commit 8a476fda5e45f16d8c608c73d9e871bd4023d79a)
For historical reasons, pg_dump refers to large objects as "BLOBs". This term is not used anywhere else in PostgreSQL, and it also means something different in the SQL standard and other SQL systems. This patch renames internal functions, code comments, documentation, etc. to use the "large object" or "LO" terminology instead. There is no functionality change, so the archive format still uses the name "BLOB" for the archive entry. Additional long command-line options are added with the new naming. Reviewed-by: Daniel Gustafsson <[email protected]> Discussion: https://www.postgresql.org/message-id/flat/868a381f-4650-9460-1726-1ffd39a270b4%40enterprisedb.com (cherry picked from commit 35ce24c333cf6dee3c92bc5f67553c7720bd9988)
A couple of places weren't up to speed for this. By sheer good luck, we didn't fail but just selected a non-memoized join plan, at least in the test case we have. Nonetheless, it's a bug, and I'm not quite sure that it couldn't have worse consequences in other examples. So back-patch to v14 where Memoize came in. Richard Guo Discussion: https://postgr.es/m/CAMbWs48GkNom272sfp0-WeD6_0HSR19BJ4H1c9ZKSfbVnJsvRg@mail.gmail.com (cherry picked from commit d69d01ba9d8d774487032459ebb83d2086715f01)
Keeping the SQL commands that initdb runs in string arrays before feeding them to PG_CMD_PUTS() seems unnecessarily verbose and inflexible. In some cases, the array only has one member. In other cases, one might want to use PG_CMD_PRINTF() instead, to parametrize a command, but that would require breaking up the loop or using workarounds like replace_token(). Unwind all that; it's much simpler that way. Reviewed-by: John Naylor <[email protected]> Reviewed-by: Andrew Dunstan <[email protected]> Discussion: https://www.postgresql.org/message-id/flat/2c50823b-f453-bb97-e38b-34751c51dcdf%40enterprisedb.com (cherry picked from commit 1bd47d0dca9f63dc26abc00d4912857440cd101c)
By default, the contents generated by the custom and directory dump formats are compressed. However, with the existing test facility, the restore program will succeed regardless of whether the dumped output was compressed or not without checking if anything has been compressed. This commit implements a portable way to check the contents of the custom and directory dump formats: - glob_patterns, that can be defined for each test as an array of glob()-compilable strings, tracking the contents that should or should not be compressed. While this is useful to make sure that the table data is compressed, this also checks that blobs.toc and toc.dat are never compressed. - command_like, to execute a command on a dump and check its generated output. This is used here in correlation with pg_restore -l to check if the dumps have been compressed or not, depending on if the build supports gzip, or not. This hole in the tests has come up when working on 5e73a60, where compression has to be applied by default, if available, for both dump formats. The idea of glob_patterns comes from me, and Georgios has come up with the design for command_like. Author: Georgios Kokolatos, Michael Paquier Discussion: https://postgr.es/m/DQn4czCWR1rcbGPLL7p3LfEr5-kGmlySm-H05VgroINdikvhtS5r9EdI6b8D8sjnbKdJ09k-cxs2AqijBeHAWk9Q8gvEAxPRHuLRhwONcGc=@pm.me (cherry picked from commit a7885c9bb22de3efcc0ba4e79bbde8a77e8b7036)
Passing a NULL snapshot (InvalidSnapshot) is going to work but only as long as the index can't find any matching rows. This can be confusing for the extension authors, so add an explicit check for this argument. The check is implemented with Assert() in order to avoid overhead in release builds. Reported-by: Sven Klemm Discussion: https://postgr.es/m/CAJ7c6TPxitD4vbKyP-mpmC1XwyHdPPqvjLzm%2BVpB88h8LGgneQ%40mail.gmail.com Author: Aleksander Alekseev Reviewed-by: Pavel Borisov (cherry picked from commit 941aa6a6268a6a66f6895401aad6b5329111d412)
The same code pattern is repeated 17 times for int64 counters (0 for missing entry) and 5 times for timestamps (NULL for missing entry) on table entries. This code is switched to use a macro for the basic code instead, shaving a few hundred lines of originally-duplicated code. The function names remain the same, but some fields of PgStat_StatTabEntry have to be renamed to cope with the new style. Author: Bertrand Drouvot Reviewed-by: Nathan Bossart Discussion: https:/postgr.es/m/20221204173207.GA2669116@nathanxps13 (cherry picked from commit 83a1a1b56645b7a55ec00e44f8018116ee87c720)
9d9c02c added window "run conditions", which allows the evaluation of monotonic window functions to be skipped when the run condition is no longer true. Prior to this commit, once the run condition was no longer true and we stopped evaluating the window functions, we simply just left the ecxt_aggvalues[] and ecxt_aggnulls[] arrays alone to store whatever value was stored there the last time the window function was evaluated. Leaving a stale value in there isn't really a problem on 64-bit builds as all of the window functions which we recognize as monotonic all return int8, which is passed by value on 64-bit builds. However, on 32-bit builds, this was a problem as the value stored in the ecxt_values[] element would be a by-ref value and it would be pointing to some memory which would get reset once the tuple context is destroyed. Since the WindowAgg node will output these values in the resulting tupleslot, this could be problematic for the top-level WindowAgg node which must look at these values to filter out the rows that don't meet its filter condition. Here we fix this by just zeroing the ecxt_aggvalues[] and setting the ecxt_aggnulls[] array to true when the run condition first becomes false. This results in the WindowAgg's output having NULLs for the WindowFunc's columns rather than the stale or pointer pointing to possibly freed memory. These tuples with the NULLs can only make it as far as the top-level WindowAgg node before they're filtered out. To ensure that these tuples *are* always filtered out, we now insist that OpExprs making up the run condition are strict OpExprs. Currently, all the window functions which the planner recognizes as monotonic return INT8 and the operator which is used for the run condition must be a member of a btree opclass. In reality, these restrictions exclude nothing that's built-in to Postgres and are unlikely to exclude anyone's custom operators due to the requirement that the operator is part of a btree opclass. It would be unusual if those were not strict. Reported-by: Sergey Shinderuk, using valgrind Reviewed-by: Richard Guo, Sergey Shinderuk Discussion: https://postgr.es/m/[email protected] Backpatch-through: 15, where 9d9c02c was added (cherry picked from commit a8583272218ab6e97c7bdeb8a960d8d4de329b7e)
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
Sairakan
pushed a commit
to amazon-aurora/babelfish_extensions
that referenced
this pull request
Dec 7, 2023
Signed-off-by: Jason Teng <[email protected]>
2jungkook
approved these changes
Dec 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
117 commits, trivial merge conflicts
Extension PR for validation: amazon-aurora/babelfish_extensions#54
fb32748e32e2b6b2fcb32220980b93d5436f855e Switch SQLValueFunction on "name" to use COERCE_SQL_SYNTAX
Merge conflict in src/backend/parser/gram.y - resolved by taking both changes and then deleting the makeSQLValueFunction() call from our changes
f193883fc9cebe8fa20359b0797832837a788112 Replace SQLValueFunction by COERCE_SQL_SYNTAX
Merge conflict in src/backend/parser/gram.y - resolved by taking both changes and then replacing the makeSQLValueFunction() call from our changes with the community changes
51b5834cd53f0bd068729043b55f7da3ca6bb15f Provide options for postmaster to kill child processes with SIGABRT.
Merge conflict in src/backend/postmaster/postmaster.c - resolved by keeping our changes and then deleting everything below
};to get rid of the non-babelfish code per the community changesffbb7e65a873e8f1584d2e593416fbe3adc130b6 Fix handling of pending inserts in nodeModifyTable.c.
Merge conflict in src/backend/executor/nodeModifyTable.c - resolved by keeping current changes and then deleting the section above `/* for INSERT ... EXECUTE */ as that is Babelfish-specific code
822e8836d5c895e454d8a22c674456cb9a9a2f44 Add portlock directory to .gitignore
Merge conflict in .gitignore - resolved by just taking their changes