aboutsummaryrefslogtreecommitdiff
path: root/src/backend/utils
Commit message (Collapse)AuthorAge
* Improve locking around InjectionPointRun()Michael Paquier2024-06-28
| | | | | | | | | | | | | | | | As coded, an injection point could be loaded into the local cache without the LWLock InjectionPointLock taken, hence a point detached and re-attached concurrently of a point running calling InjectionPointRun() may finish by loading a callback it did no set initially. Based on all the cases discussed until now on the lists, it is fine to delay the lock release until the callback is run, so let's do that. While on it, remove a useless LWLockRelease() called before an error in InjectionPointAttach(). Per discussion with Heikki Linnakangas and Noah Misch. Discussion: https://postgr.es/m/e1ffb822-054e-4006-ac06-50532767f75b@iki.fi
* Cope with inplace update making catcache stale during TOAST fetch.Noah Misch2024-06-27
| | | | | | | | | | | | | | | | This extends ad98fb14226ae6456fbaed7990ee7591cbe5efd2 to invals of inplace updates. Trouble requires an inplace update of a catalog having a TOAST table, so only pg_database was at risk. (The other catalog on which core code performs inplace updates, pg_class, has no TOAST table.) Trouble would require something like the inplace-inval.spec test. Consider GRANT ... ON DATABASE fetching a stale row from cache and discarding a datfrozenxid update that vac_truncate_clog() has already relied upon. Back-patch to v12 (all supported versions). Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240114201411.d0@rfd.leadboat.com Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Add an injection_points isolation test suite.Noah Misch2024-06-27
| | | | | | | | | Make the isolation harness recognize injection_points wait events as a type of blocked state. Test an extant inplace-update bug. Reviewed by Robert Haas and Michael Paquier. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Create waitfuncs.c for pg_isolation_test_session_is_blocked().Noah Misch2024-06-27
| | | | | | | | | The next commit makes the function inspect an additional non-lock contention source, so it no longer fits in lockfuncs.c. Reviewed by Robert Haas. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Add wait event type "InjectionPoint", a custom type like "Extension".Noah Misch2024-06-27
| | | | | | | | | Both injection points and customization of type "Extension" are new in v17, so this just changes a detail of an unreleased feature. Reported by Robert Haas. Reviewed by Michael Paquier. Discussion: https://postgr.es/m/CA+TgmobfMU5pdXP36D5iAwxV5WKE_vuDLtp_1QyH+H5jMMt21g@mail.gmail.com
* Avoid crashing when a JIT-inlined backend function throws an error.Tom Lane2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | errfinish() assumes that the __FUNC__ and __FILE__ arguments it's passed are compile-time constant strings that can just be pointed to rather than physically copied. However, it's possible for LLVM to generate code in which those pointers point into a dynamically loaded code segment. If that segment gets unloaded before we're done with the ErrorData struct, we have dangling pointers that will lead to SIGSEGV. In simple cases that won't happen, because we won't unload LLVM code before end of transaction. But it's possible to happen if the error is thrown within end-of-transaction code run by _SPI_commit or _SPI_rollback, because since commit 2e517818f those functions clean up by ending the transaction and starting a new one. Rather than fixing this by adding pstrdup() overhead to every elog/ereport sequence, let's fix it by copying the risky pointers in CopyErrorData(). That solves it for _SPI_commit/_SPI_rollback because they use that function to preserve the error data across the transaction end/restart sequence; and it seems likely that any other code doing something similar would need to do that too. I'm suspicious that this behavior amounts to an LLVM bug (or a bug in our use of it?), because it implies that string constant references that should be pointer-equal according to a naive understanding of C semantics will sometimes not be equal. However, even if it is a bug and someday gets fixed, we'll have to cope with the current behavior for a long time to come. Report and patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/1565654.1719425368@sss.pgh.pa.us
* Fix overflow with pgstats DSA reference countMichael Paquier2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | When pgstats is initialized for a backend, it uses dsa_attach_in_place() without a "segment" provided. Hence, no callback is registered to automatically release the DSA attached once a backend exits. Not doing any cleanup causes the reference count of the pgstats DSA to continuously increment, at some point overflowing it (the more the number of connections, the faster it is to reach this state). Once the reference count overflows and then gets back to 0, new backends are not able to attach to the pgstats DSA, failing startup. This issue is resolved by adding in the pgstats shutdown hook a call to dsa_release_in_place(), ensuring that the DSA attached at backend startup is correctly released, keeping the reference count at bay. The author of this patch has been able to see this issue on a server with a long uptime and a high connection turnover. Issue introduced by 5891c7a8ed8f, so backpatch down to 15. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_XqqJbJBL=M7Ym13TcB4Xnq58vRa2jcC+gwEPBgbAda6B1Q@mail.gmail.com Backpatch-through: 15
* Fix relcache invalidation when relfilelocator is updatedHeikki Linnakangas2024-06-21
| | | | | | | | | | In commit af0e7deb4a, I removed a call to RelationCloseSmgr(), because the dangling SMgrRelation was no longer an issue. However, we still need the call when the relation's relfilelocator changes, so that the new relfilelocator takes effect immediately. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://www.postgresql.org/message-id/987b1c8c-8c91-4847-ca0e-879f421680ff%40gmail.com
* jsonapi: Use const char *Peter Eisentraut2024-06-21
| | | | | | | | | | Apply const qualifiers to char * arguments and fields throughout the jsonapi. This allows the top-level APIs such as pg_parse_json_incremental() to declare their input argument as const. It also reduces the number of unconstify() calls. Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://www.postgresql.org/message-id/flat/f732b014-f614-4600-a437-dba5a2c3738b%40eisentraut.org
* SQL/JSON: Correct jsonpath variable name matchingAmit Langote2024-06-19
| | | | | | | | | | | | | | | Previously, GetJsonPathVar() allowed a jsonpath expression to reference any prefix of a PASSING variable's name. For example, the following query would incorrectly work: SELECT JSON_QUERY(context_item, jsonpath '$xy' PASSING val AS xyz); The fix ensures that the length of the variable name mentioned in a jsonpath expression matches exactly with the name of the PASSING variable before comparing the strings using strncmp(). Reported-by: Alvaro Herrera (off-list) Discussion: https://postgr.es/m/CA+HiwqFGkLWMvELBH6E4SQ45qUHthgcRH6gCJL20OsYDRtFx_w@mail.gmail.com
* Fix incorrect punctuation in error messagePeter Eisentraut2024-06-18
|
* Improve tracking of role dependencies of pg_init_privs entries.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 534287403 invented SHARED_DEPENDENCY_INITACL entries in pg_shdepend, but installed them only for non-owner roles mentioned in a pg_init_privs entry. This turns out to be the wrong thing, because there is nothing to cue REASSIGN OWNED to go and update pg_init_privs entries when the object's ownership is reassigned. That leads to leaving dangling entries in pg_init_privs, as reported by Hannu Krosing. Instead, install INITACL entries for all roles mentioned in pg_init_privs entries (except pinned roles), and change ALTER OWNER to not touch them, just as it doesn't touch pg_init_privs entries. REASSIGN OWNED will now substitute the new owner OID for the old in pg_init_privs entries. This feels like perhaps not quite the right thing, since pg_init_privs ought to be a historical record of the state of affairs just after CREATE EXTENSION. However, it's hard to see what else to do, if we don't want to disallow dropping the object's original owner. In any case this is better than the previous do-nothing behavior, and we're unlikely to come up with a superior solution in time for v17. While here, tighten up some coding rules about how ACLs in pg_init_privs should never be null or empty. There's not any obvious reason to allow that, and perhaps asserting that it's not so will catch some bugs. (We were previously inconsistent on the point, with some code paths taking care not to store empty ACLs and others not.) This leaves recordExtensionInitPrivWorker not doing anything with its ownerId argument, but we'll deal with that separately. catversion bump forced because of change of expected contents of pg_shdepend when pg_init_privs entries exist. Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
* Teach jsonpath string() to unwrap in lax modeAndrew Dunstan2024-06-17
| | | | | | | | | | | | This was an ommission in commit 66ea94e, and brings it into compliance with both other methods and the standard. Per complaint from David Wheeler. Author: David Wheeler, Jeevan Chalke Reviewed-by: Chapman Flack Discussion: https://postgr.es/m/A64AE04F-4410-42B7-A141-7A7349260F4D@justatheory.com
* Fix parsing of ignored operators in websearch_to_tsquery().Tom Lane2024-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | The manual says clearly that punctuation in the input of websearch_to_tsquery() is ignored, except for the special cases of dashes and quotes. However, this failed for cases like "(foo bar) or something", or in general an ISOPERATOR character in front of the "or". We'd switch back to WAITOPERAND state, then ignore the operator character while remaining in that state, and then reach the "or" in WAITOPERAND state which (intentionally) makes us treat it as data. The fix is simple enough: if we see an ISOPERATOR character while in WAITOPERATOR state, we have to skip it while staying in that state. (We don't need to worry about other punctuation characters: those will be consumed as though they were words, but then rejected by lexizing.) In v14 and up (since commit eb086056f) we can simplify the code a bit more too, because there is no longer a reason for the WAITOPERAND state to distinguish between quoted and unquoted operands. Per bug #18479 from Manos Emmanouilidis. Back-patch to all supported branches. Discussion: https://postgr.es/m/18479-d9b46e2fc242c33e@postgresql.org
* Harmonize function parameter names for Postgres 17.Peter Geoghegan2024-06-12
| | | | | | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions in a few places. These inconsistencies were all introduced during Postgres 17 development. pg_bsd_indent still has a couple of similar inconsistencies, which I (pgeoghegan) have left untouched for now. This commit was written with help from clang-tidy, by mechanically applying the same rules as similar clean-up commits (the earliest such commit was commit 035ce1fe).
* Add more debugging information when dropping twice pgstats entryMichael Paquier2024-06-07
| | | | | | | | | | | | | | | | | | | Floris Van Nee has reported a bug in the pgstats facility where a stats entry already dropped would get again dropped. This case should not happen, still the error generated did not offer any details about the stats entry getting dropped. This commit improves the error message generated to inform about the stats entry kind, database OID, object OID and refcount, which should help to debug more the problem reported. Bertrand Drouvot has been independently able to reach this error path while writing a new feature, and more details about the failure would have been helpful for debugging. Author: Andres Freund, Bertrand Drouvot Discussion: https://postgr.es/m/20240505160915.6boysum4f34siqct@awork3.anarazel.de Discussion: https://postgr.es/m/ZkM30paAD8Cr/Bix@ip-10-97-1-34.eu-west-3.compute.internal Backpatch-through: 15
* Make RelationFlushRelation() work without ResourceOwner during abortHeikki Linnakangas2024-06-06
| | | | | | | | | | | | | | | | | | | | | ReorderBufferImmediateInvalidation() executes invalidation messages in an aborted transaction. However, RelationFlushRelation sometimes required a valid resource owner, to temporarily increment the refcount of the relache entry. Commit b8bff07daa worked around that in the main subtransaction abort function, AbortSubTransaction(), but missed this similar case in ReorderBufferImmediateInvalidation(). To fix, introduce a separate function to invalidate a relcache entry. It does the same thing as RelationClearRelation(rebuild==true) does when outside a transaction, but can be called without incrementing the refcount. Add regression test. Before this fix, it failed with: ERROR: ResourceOwnerEnlarge called after release started Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://www.postgresql.org/message-id/e56be7d9-14b1-664d-0bfc-00ce9772721c@gmail.com
* Prevent inconsistent use of stats entry for replication slotsMichael Paquier2024-06-06
| | | | | | | | | | | | | | | | | | | | | | Concurrent activity around replication slot creation and drop could cause a replication slot to use a stats entry it should not have used when created, triggering an assertion failure when retrieving an inconsistent entry from the dshash table used by the stats facility. The issue is that pgstat_drop_replslot() calls pgstat_drop_entry() without checking the result. If pgstat_drop_entry() cannot free the entry related to the object dropped, pgstat_request_entry_refs_gc() should be called. AtEOXact_PgStat_DroppedStats() and surrounding routines dropping stats entries already do that. This is documented in pgstat_internal.h, but let's add a comment at the top of pgstat_drop_entry() as that can be easy to miss. Reported-by: Alexander Lakhin Author: Floris Van Nee Analyzed-by: Andres Freund Discussion: https://postgr.es/m/17947-b9554521ad963c9c@postgresql.org Backpatch-through: 15
* Move new SLRU buffers GUCs to a better place in postgresql.conf.samplePeter Eisentraut2024-06-05
| | | | | | They were under "File Locations", which doesn't make sense. Move them to Resource Usage / Memory, which matches their categorization in the source code and in the documentation.
* Fix another couple of outdated comments for MERGE RETURNING.Dean Rasheed2024-06-04
| | | | | | Oversights in c649fa24a4 which added RETURNING support to MERGE. Discussion: https://postgr.es/m/CAApHDvpqp6vtUzG-_josUEiBGyqnrnVxJ-VdF+hJLXjHdHzsyQ@mail.gmail.com
* Fix input of ISO "extended" time format for types time and timetz.Tom Lane2024-05-22
| | | | | | | | | | | | | | | | | | | | | | | Commit 3e1a373e2 missed teaching DecodeTimeOnly the same "ptype" manipulations it added to DecodeDateTime. While likely harmless at the time, it became a problem after 5b3c59535 added an error check that ptype must be zero once we exit the parsing loop (that is, there shouldn't be any unused prefixes). The consequence was that we'd reject time or timetz input like T12:34:56 (the "extended" format per ISO 8601-1:2019), even though that still worked in timestamp input. Since this is clearly under-tested code, add test cases covering all the ISO 8601 time formats. (Note: although 8601 allows just "Thh", we have never accepted that, and this patch doesn't change that. I'm content to leave that as-is because it seems too likely to be a mistake rather than intended input. If anyone wants to allow that, it should be a separate patch anyway, and not back-patched.) Per bug #18470 from David Perez. Back-patch to v16 where we broke it. Discussion: https://postgr.es/m/18470-34fad4c829106848@postgresql.org
* A few follow-up fixes for GUC name quotingPeter Eisentraut2024-05-17
| | | | | | Fixups for 17974ec259: Some messages were missed (and some were new since the patch was originally proposed), and there was a typo introduced.
* Revise GUC names quoting in messages againPeter Eisentraut2024-05-17
| | | | | | | | | | | | | | | After further review, we want to move in the direction of always quoting GUC names in error messages, rather than the previous (PG16) wildly mixed practice or the intermittent (mid-PG17) idea of doing this depending on how possibly confusing the GUC name is. This commit applies appropriate quotes to (almost?) all mentions of GUC names in error messages. It partially supersedes a243569bf65 and 8d9978a7176, which had moved things a bit in the opposite direction but which then were abandoned in a partial state. Author: Peter Smith <smithpb2250@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w%40mail.gmail.com
* Remove unused typedefsPeter Eisentraut2024-05-17
| | | | | | | | | | | | | | | | | | There were a few typedefs that were never used to define a variable or field. This had the effect that the buildfarm's typedefs.list would not include them, and so they would need to be re-added manually to keep the overall pgindent result perfectly clean. We can easily get rid of these typedefs to avoid the issue. In a few cases, we just let the enum or struct stand on its own without a typedef around it. In one case, an enum was abused to define flag bits; that's better done with macro definitions. This fixes all the remaining issues with missing entries in the buildfarm's typedefs.list. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/1919000.1715815925@sss.pgh.pa.us
* Remove enum WaitEventExtensionMichael Paquier2024-05-17
| | | | | | | | | | | | | | | | | | This enum was used to determine the first ID to use when assigning a custom wait event for extensions, which is always 1. It was kept so as it would be possible to add new in-core wait events in the category "Extension". There is no such thing currently, so let's remove this enum until a case justifying it pops up. This makes the code simpler and easier to understand. This has as effect to switch back autoprewarm.c to use PG_WAIT_EXTENSION rather than WAIT_EVENT_EXTENSION, on par with v16 and older stable branches. Thinko in c9af05465307. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/195c6c45-abce-4331-be6a-e87724e1d060@eisentraut.org
* Revert temporal primary keys and foreign keysPeter Eisentraut2024-05-16
| | | | | | | | | | | | | | | | | | | | This feature set did not handle empty ranges correctly, and it's now too late for PostgreSQL 17 to fix it. The following commits are reverted: 6db4598fcb8 Add stratnum GiST support function 46a0cd4cefb Add temporal PRIMARY KEY and UNIQUE constraints 86232a49a43 Fix comment on gist_stratnum_btree 030e10ff1a3 Rename pg_constraint.conwithoutoverlaps to conperiod a88c800deb6 Use daterange and YMD in without_overlaps tests instead of tsrange. 5577a71fb0c Use half-open interval notation in without_overlaps tests 34768ee3616 Add temporal FOREIGN KEY contraints 482e108cd38 Add test for REPLICA IDENTITY with a temporal key c3db1f30cba doc: clarify PERIOD and WITHOUT OVERLAPS in CREATE TABLE 144c2ce0cc7 Fix ON CONFLICT DO NOTHING/UPDATE for temporal indexes Discussion: https://www.postgresql.org/message-id/d0b64a7a-dfe4-4b84-a906-c7dedfa40a3e@eisentraut.org
* Revert structural changes to not-null constraintsAlvaro Herrera2024-05-13
| | | | | | | | | | | | | | | | | | | | | | | | | There are some problems with the new way to handle these constraints that were detected at the last minute, and require fixes that appear too invasive to be doing this late in the cycle. Revert this (again) for now, we'll try again with these problems fixed. The following commits are reverted: b0e96f311985 Catalog not-null constraints 9b581c534186 Disallow changing NO INHERIT status of a not-null constraint d0ec2ddbe088 Fix not-null constraint test ac22a9545ca9 Move privilege check to the right place b0f7dd915bca Check stack depth in new recursive functions 3af721794272 Update information_schema definition for not-null constraints c3709100be73 Fix propagating attnotnull in multiple inheritance d9f686a72ee9 Fix restore of not-null constraints with inheritance d72d32f52d26 Don't try to assign smart names to constraints 0cd711271d42 Better handle indirect constraint drops 13daa33fa5a6 Disallow NO INHERIT not-null constraints on partitioned tables d45597f72fe5 Disallow direct change of NO INHERIT of not-null constraints 21ac38f498b3 Fix inconsistencies in error messages Discussion: https://postgr.es/m/202405110940.joxlqcx4dogd@alvherre.pgsql
* Introduce private data area for injection pointsMichael Paquier2024-05-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit extends the backend-side infrastructure of injection points so as it becomes possible to register some input data when attaching a point. This private data can be registered with the function name and the library name of the callback when attaching a point, then it is given as input argument to the callback. This gives the possibility for modules to pass down custom data at runtime when attaching a point without managing that internally, in a manner consistent with the callback entry retrieved from the hash shmem table storing the injection point data. InjectionPointAttach() gains two arguments, to be able to define the private data contents and its size. A follow-up commit will rely on this infrastructure to close a race condition with the injection point detach in the module injection_points. While on it, this changes InjectionPointDetach() to return a boolean, returning false if a point cannot be detached. This has been mentioned by Noah as useful when it comes to implement more complex tests with concurrent point detach, solid with the automatic detach done for local points in the test module. Documentation is adjusted in consequence. Per discussion with Noah Misch. Reviewed-by: Noah Misch Discussion: https://postgr.es/m/20240509031553.47@rfd.leadboat.com
* Fix typo in src/backend/utils/resowner/README.Etsuro Fujita2024-05-08
|
* Remove obsolete comment.Jeff Davis2024-05-07
| | | | | | | | Per suggestion from Peter, the comment was not helpful, so remove it rather than fixing it. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/d9421b21-e759-4b74-a039-c487b469c1f3@eisentraut.org
* postgresql.conf: align variable comments, mostly new onesBruce Momjian2024-05-06
| | | | Backpatch-through: master
* Revert: Remove useless self-joinsAlexander Korotkov2024-05-06
| | | | | | | | | | | | | | | | | This commit reverts d3d55ce5713 and subsequent fixes 2b26a694554, 93c85db3b5b, b44a1708abe, b7f315c9d7d, 8a8ed916f73, b5fb6736ed3, 0a93f803f45, e0477837ce4, a7928a57b9f, 5ef34a8fc38, 30b4955a466, 8c441c08279, 028b15405b4, fe093994db4, 489072ab7a9, and 466979ef031. We are quite late in the release cycle and new bugs continue to appear. Even though we have fixes for all known bugs, there is a risk of throwing many bugs to end users. The plan for self-join elimination would be to do more review and testing, then re-commit in the early v18 cycle. Reported-by: Tom Lane Discussion: https://postgr.es/m/2422119.1714691974%40sss.pgh.pa.us
* Fix an assortment of typosDavid Rowley2024-05-04
| | | | | Author: Alexander Lakhin Discussion: https://postgr.es/m/ae9f2fcb-4b24-5bb0-4240-efbbbd944ca1@gmail.com
* Revert "Add GUC backtrace_on_internal_error"Peter Eisentraut2024-04-29
| | | | | | | | | | | | This reverts commit a740b213d4b4d3360ad0cac696e47e5ec0eb8864. Subsequent discussion showed that there was interest in a more general facility to configure when server log events would produce backtraces, and this existing limited way couldn't be extended in a compatible way. So the consensus was to revert this for PostgreSQL 17 and reconsider this topic for PostgreSQL 18. Discussion: https://www.postgresql.org/message-id/flat/CAGECzQTChkvn5Xj772LB3%3Dxo2x_LcaO5O0HQvXqobm1xVp6%2B4w%40mail.gmail.com#764bcdbb73e162787e1ad984935e51e3
* Detect more overflows in timestamp[tz]_pl_interval.Tom Lane2024-04-28
| | | | | | | | | | | | | | | | | In commit 25cd2d640 I (tgl) opined that "The additions of the months and microseconds fields could also overflow, of course. However, I believe we need no additional checks there; the existing range checks should catch such cases". This is demonstrably wrong however for the microseconds field, and given that discovery it seems prudent to be paranoid about the months addition as well. Report and patch by Joseph Koshakow. As before, back-patch to all supported branches. (However, the test case doesn't work before v15 because we didn't allow wider-than-int32 numbers in interval literals. A variant test could probably be built that fits within that restriction, but it didn't seem worth the trouble.) Discussion: https://postgr.es/m/CAAvxfHf77sRHKoEzUw9_cMYSpbpNS2C+J_+8Dq4+0oi8iKopeA@mail.gmail.com
* Improve "out of range" error messages for GUCs.Tom Lane2024-04-23
| | | | | | | If the GUC has a unit, label the minimum and maximum values with the unit explicitly. Per suggestion from Jian He. Discussion: https://postgr.es/m/CACJufxFJo6FyVg9W8yvNAxbjP+EJ9wieE9d9vw5LpPzyLnLLOQ@mail.gmail.com
* Fix typos and duplicate wordsDaniel Gustafsson2024-04-18
| | | | | | | | | | | | This fixes various typos, duplicated words, and tiny bits of whitespace mainly in code comments but also in docs. Author: Daniel Gustafsson <daniel@yesql.se> Author: Heikki Linnakangas <hlinnaka@iki.fi> Author: Alexander Lakhin <exclusion@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
* SQL/JSON: Miscellaneous fixes and improvementsAmit Langote2024-04-18
| | | | | | | | | | | | | | | | | | | | This addresses some post-commit review comments for commits 6185c973, de3600452, and 9425c596a0, with the following changes: * Fix JSON_TABLE() syntax documentation to use the term "path_expression" for JSON path expressions instead of "json_path_specification" to be consistent with the other SQL/JSON functions. * Fix a typo in the example code in JSON_TABLE() documentation. * Rewrite some newly added comments in jsonpath.h. * In JsonPathQuery(), add missing cast to int before printing an enum value. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com
* SQL/JSON: Improve some error messagesAmit Langote2024-04-18
| | | | | | | | | | | | | | | This improves some error messages emitted by SQL/JSON query functions by mentioning column name when available, such as when they are invoked as part of evaluating JSON_TABLE() columns. To do so, a new field column_name is added to both JsonFuncExpr and JsonExpr that is only populated when creating those nodes for transformed JSON_TABLE() columns. While at it, relevant error messages are reworded for clarity. Reported-by: Jian He <jian.universality@gmail.com> Suggested-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com
* Update mmgr's README to mention BumpContextDavid Rowley2024-04-17
| | | | | | | | | | | | Oversight in 29f6a959c. In passing, since we now have 4 memory context types to choose from, provide a brief overview of the specialities of each memory context type. Reported-by: Amul Sul Author: Amul Sul, David Rowley Discussion: https://postgr.es/m/CAAJ_b94U2s9nHh--DEK=sPEZUQ+x7vQJ7529fF8UAH97QJ9NXg@mail.gmail.com
* Push dedicated BumpBlocks to the tail of the blocks listDavid Rowley2024-04-17
| | | | | | | | | | | | | | | | | | | | | | | BumpContext relies on using the head block from its 'blocks' field to use as the current block to allocate new chunks to. When we receive an allocation request larger than allocChunkLimit, we place these chunks on a new dedicated block and, until now, we pushed the block onto the *head* of the 'blocks' list. This behavior caused the previous bump block to no longer be available for new normal-sized (non-large) allocations and would result in blocks only being partially filled if a large allocation request arrived before the block became full. Here adjust the code to push these dedicated blocks onto the *tail* of the blocks list so that the head block remains intact and available to be used by normal allocation request sizes until it becomes full. In passing, make the elog(ERROR) calls for the unsupported callbacks consistent. Likewise for the header comments for those functions. Discussion: https://postgr.es/m/CAApHDvp9___r-ayJj0nZ6GD3MeCGwGZ0_6ZptWpwj+zqHtmwCw@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvqerXpzUnuDQfUEi3DZA+9=Ud9WSt3ruxN5b6PcOosx2g@mail.gmail.com
* Fix type-checking of RECORD-returning functions in FROM, redux.Tom Lane2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 2ed8f9a01 intended to institute a policy that if a RangeTblFunction has a coldeflist, then the function return type is certainly RECORD, and we should use the coldeflist as the source of truth about what the columns of the record type are. When the original function has been folded to a constant, inspection of the constant might give a different answer. This situation will lead to a tuple-type-mismatch error at execution, but up until that point we need to consistently believe the coldeflist, or we'll have problems from different bits of code reaching different conclusions. expandRTE didn't get that memo though, and would try to produce a tupdesc based on the constant in this situation, leading to an assertion failure. (Desultory testing suggests that non-assert builds often manage to give the expected error, although I also saw a "cache lookup failed for type 0" error, and it seems at least possible that a crash could happen.) Some other callers of get_expr_result_type and get_expr_result_tupdesc were also being incautious about this. While none of them seem to have actual bugs, they're working harder than necessary in this case, besides which it seems safest to have an explicit policy of not using those functions on an RTE with a coldeflist. Adjust the code accordingly, and add commentary to funcapi.c about this policy. Also fix an obsolete comment that claimed "get_expr_result_type() doesn't know how to extract type info from a RECORD constant". That hasn't been true since commit d57534740. Per bug #18422 from Alexander Lakhin. As with the previous commit, back-patch to all supported branches. Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
* ATTACH PARTITION: Don't match a PK with a UNIQUE constraintAlvaro Herrera2024-04-15
| | | | | | | | | | When matching constraints in AttachPartitionEnsureIndexes() we weren't testing the constraint type, which could make a UNIQUE key lacking a not-null constraint incorrectly satisfy a primary key requirement. Fix this by testing that the constraint types match. (Other possible mismatches are verified by comparing index properties.) Discussion: https://postgr.es/m/202402051447.wimb4xmtiiyb@alvherre.pgsql
* Fix ALTER DOMAIN NOT NULL syntaxPeter Eisentraut2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This addresses a few problems with commit e5da0fe3c22 ("Catalog domain not-null constraints"). In CREATE DOMAIN, a NOT NULL constraint looks like CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL (Before e5da0fe3c22, the constraint name was accepted but ignored.) But in ALTER DOMAIN, a NOT NULL constraint looks like ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE where VALUE is where for a table constraint the column name would be. (This works as of e5da0fe3c22. Before e5da0fe3c22, this syntax resulted in an internal error.) But for domains, this latter syntax is confusing and needlessly inconsistent between CREATE and ALTER. So this changes it to just ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL (None of these syntaxes are per SQL standard; we are just living with the bits of inconsistency that have built up over time.) In passing, this also changes the psql \dD output to not show not-null constraints in the column "Check", since it's already shown in the column "Nullable". This has also been off since e5da0fe3c22. Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
* Revert: Implement pg_wal_replay_wait() stored procedureAlexander Korotkov2024-04-11
| | | | | | | This commit reverts 06c418e163, e37662f221, bf1e650806, 25f42429e2, ee79928441, and 74eaf66f98 per review by Heikki Linnakangas. Discussion: https://postgr.es/m/b155606b-e744-4218-bda5-29379779da1a%40iki.fi
* Revert: Allow table AM to store complex data structures in rd_amcacheAlexander Korotkov2024-04-11
| | | | | | This commit reverts 02eb07ea89 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Revert: Custom reloptions for table AMAlexander Korotkov2024-04-11
| | | | | | This commit reverts 9bd99f4c26 and 422041542f per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Fixup various StringInfo function usagesDavid Rowley2024-04-10
| | | | | | | | | | | | | | | This adjusts various appendStringInfo* function calls to use a more appropriate and efficient function with the same behavior. For example, use appendStringInfoChar() when appending a single character rather than appendStringInfo() and appendStringInfoString() when no formatting is required rather than using appendStringInfo(). All adjustments made here are in code that's new to v17, so it makes sense to fix these now rather than wait a few years and make backpatching harder. Discussion: https://postgr.es/m/CAApHDvojY2UvMiO+9_55ArTj10P1LBNJyyoGB+C65BLDNT0GsQ@mail.gmail.com Reviewed-by: Nathan Bossart, Tom Lane
* revert: Transform OR clauses to ANY expressionAlexander Korotkov2024-04-10
| | | | | | | This commit reverts 72bd38cc99 due to implementation and design issues. Reported-by: Tom Lane Discussion: https://postgr.es/m/3604469.1712628736%40sss.pgh.pa.us
* Remove unused BumpBlockIsValid macroDavid Rowley2024-04-10
| | | | | | | | | | | The bump allocator was recently added in 29f6a959c. Our other allocators have a similar macro to this, but seemingly the version of the macro for those allocators is only used in places where the chunk header is decoded. Since the bump allocator has no chunk header, none of those functions exist for bump therefore macro is unused. Remove it. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/5f724fb2-96e1-4f36-b65b-47b337ad432e@eisentraut.org