aboutsummaryrefslogtreecommitdiff
path: root/src/backend/commands/tablecmds.c
Commit message (Collapse)AuthorAge
...
* Improve handling of inherited GENERATED expressions.Tom Lane2023-01-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In both partitioning and traditional inheritance, require child columns to be GENERATED if and only if their parent(s) are. Formerly we allowed the case of an inherited column being GENERATED when its parent isn't, but that results in inconsistent behavior: the column can be directly updated through an UPDATE on the parent table, leading to it containing a user-supplied value that might not match the generation expression. This also fixes an oversight that we enforced partition-key-columns-can't- be-GENERATED against parent tables, but not against child tables that were dynamically attached to them. Also, remove the restriction that the child's generation expression be equivalent to the parent's. In the wake of commit 3f7836ff6, there doesn't seem to be any reason that we need that restriction, since generation expressions are always computed per-table anyway. By removing this, we can also allow a child to merge multiple inheritance parents with inconsistent generation expressions, by overriding them with its own expression, much as we've long allowed for DEFAULT expressions. Since we're rejecting a case that we used to accept, this doesn't seem like a back-patchable change. Given the lack of field complaints about the inconsistent behavior, it's likely that no one is doing this anyway, but we won't change it in minor releases. Amit Langote and Tom Lane Discussion: https://postgr.es/m/2793383.1672944799@sss.pgh.pa.us
* Update copyright for 2023Bruce Momjian2023-01-02
| | | | Backpatch-through: 11
* Fix operator typo in tablecmds.cMichael Paquier2022-12-22
| | | | | | | | | | | | | | A bitwise operator was getting used on two bools in ATAddCheckConstraint() to track if constraints should be merged or not with the existing ones of a relation, though obviously this should use a boolean OR operator. This led to the same result, but let's be clean. Oversight in 074c5cf. Author: Ranier Vilela Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/CAEudQAp2R2fbbi0OHHhv_n4=Ch0t1VtjObR9YMqtGKHJ+faUFQ@mail.gmail.com
* Add grantable MAINTAIN privilege and pg_maintain role.Jeff Davis2022-12-13
| | | | | | | | | | | | | Allows VACUUM, ANALYZE, REINDEX, REFRESH MATERIALIZED VIEW, CLUSTER, and LOCK TABLE. Effectively reverts 4441fc704d. Instead of creating separate privileges for VACUUM, ANALYZE, and other maintenance commands, group them together under a single MAINTAIN privilege. Author: Nathan Bossart Discussion: https://postgr.es/m/20221212210136.GA449764@nathanxps13 Discussion: https://postgr.es/m/45224.1670476523@sss.pgh.pa.us
* Get rid of recursion-marker values in enum AlterTableTypeAlvaro Herrera2022-12-12
| | | | | | | | | | | | | | | During ALTER TABLE execution, when prep-time handling of subcommands of certain types determine that execution-time handling requires recursion, they signal this by changing the subcommand type to a special value. This can be done in a simpler way by using a separate flag introduced by commit ec0925c22a3d, so do that. Catversion bumped. It's not clear to me that ALTER TABLE subcommands are stored anywhere in catalogs (CREATE FUNCTION rejects it in BEGIN ATOMIC function bodies), but we do have both write and read support for them, so be safe. Discussion: https://postgr.es/m/20220929090033.zxuaezcdwh2fgfjb@alvherre.pgsql
* Fix FK comment think-oPeter Eisentraut2022-12-07
| | | | | | | | from commit d6f96ed94e7 Author: Paul Jungwirth <pj@illuminatedcomputing.com> Reviewed-by: Ian Lawrence Barwick <barwick@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/6a7c7338-1aa2-4689-d171-0b0b294fdd84%40illuminatedcomputing.com
* Add 'missing_ok' argument to build_attrmap_by_nameAlvaro Herrera2022-11-29
| | | | | | | | | | | | When it's given as true, return a 0 in the position of the missing column rather than raising an error. This is currently unused, but it allows us to reimplement column permission checking in a subsequent commit. It seems worth breaking into a separate commit because it affects unrelated code. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqFfiai=qBxPDTjaio_ZcaqUKh+FC=prESrB8ogZgFNNNQ@mail.gmail.com
* Add a SET option to the GRANT command.Robert Haas2022-11-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Similar to how the INHERIT option controls whether or not the permissions of the granted role are automatically available to the grantee, the new SET permission controls whether or not the grantee may use the SET ROLE command to assume the privileges of the granted role. In addition, the new SET permission controls whether or not it is possible to transfer ownership of objects to the target role or to create new objects owned by the target role using commands such as CREATE DATABASE .. OWNER. We could alternatively have made this controlled by the INHERIT option, or allow it when either option is given. An advantage of this approach is that if you are granted a predefined role with INHERIT TRUE, SET FALSE, you can't go and create objects owned by that role. The underlying theory here is that the ability to create objects as a target role is not a privilege per se, and thus does not depend on whether you inherit the target role's privileges. However, it's surely something you could do anyway if you could SET ROLE to the target role, and thus making it contingent on whether you have that ability is reasonable. Design review by Nathan Bossat, Wolfgang Walther, Jeff Davis, Peter Eisentraut, and Stephen Frost. Discussion: http://postgr.es/m/CA+Tgmob+zDSRS6JXYrgq0NWdzCXuTNzT5eK54Dn2hhgt17nm8A@mail.gmail.com
* Refactor aclcheck functionsPeter Eisentraut2022-11-13
| | | | | | | | | | | | | | | | | | Instead of dozens of mostly-duplicate pg_foo_aclcheck() functions, write one common function object_aclcheck() that can handle almost all of them. We already have all the information we need, such as which system catalog corresponds to which catalog table and which column is the ACL column. There are a few pg_foo_aclcheck() that don't work via the generic function and have special APIs, so those stay as is. I also changed most pg_foo_aclmask() functions to static functions, since they are not used outside of aclchk.c. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Reviewed-by: Antonin Houska <ah@cybertec.at> Discussion: https://www.postgresql.org/message-id/flat/95c30f96-4060-2f48-98b5-a4392d3b6066@enterprisedb.com
* Refactor ownercheck functionsPeter Eisentraut2022-11-13
| | | | | | | | | | | | Instead of dozens of mostly-duplicate pg_foo_ownercheck() functions, write one common function object_ownercheck() that can handle almost all of them. We already have all the information we need, such as which system catalog corresponds to which catalog table and which column is the owner column. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Reviewed-by: Antonin Houska <ah@cybertec.at> Discussion: https://www.postgresql.org/message-id/flat/95c30f96-4060-2f48-98b5-a4392d3b6066@enterprisedb.com
* Support writing "CREATE/ALTER TABLE ... SET STORAGE DEFAULT".Tom Lane2022-11-10
| | | | | | | | | | We already allow explicitly writing DEFAULT for SET COMPRESSION, so it seems a bit inflexible and non-orthogonal to not have it for STORAGE. Aleksander Alekseev Discussion: https://postgr.es/m/CAJ7c6TMX9ui+6y3TQFaXJYVpZyBukvqhQbVDJ8OUokeLRhtnpA@mail.gmail.com
* Create FKs properly when attaching table as partitionAlvaro Herrera2022-11-03
| | | | | | | | | | | | | | | | | | | | | | Commit f56f8f8da6af added some code in CloneFkReferencing that's way too lax about a Constraint node it manufactures, not initializing enough struct members -- initially_valid in particular was forgotten. This causes some FKs in partitions added by ALTER TABLE ATTACH PARTITION to be marked as not validated. Set initially_valid true, which fixes the bug. While at it, make the struct initialization more complete. Very similar code was added in two other places by the same commit; make them all follow the same pattern for consistency, though no bugs are apparent there. This bug has never been reported: I only happened to notice while working on commit 614a406b4ff1. The test case that was added there with the improper result is repaired. Backpatch to 12. Discussion: https://postgr.es/m/20221005105523.bhuhkdx4olajboof@alvherre.pgsql
* Resolve partition strategy during early parsingAlvaro Herrera2022-11-03
| | | | | | | | This has little practical value, but there's no reason to let the partition strategy names travel through DDL as strings. Reviewed-by: Japin Li <japinli@hotmail.com> Discussion: https://postgr.es/m/20221021093216.ffupd7epy2mytkux@alvherre.pgsql
* Remove AssertArg and AssertStatePeter Eisentraut2022-10-28
| | | | | | | | | These don't offer anything over plain Assert, and their usage had already been declared obsolescent. Author: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/20221009210148.GA900071@nathanxps13
* Fix self-referencing foreign keys with partitioned tablesAlvaro Herrera2022-10-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | There are a number of bugs in this area. Two of them are fixed here, namely: 1. get_relation_idx_constraint_oid does not restrict the type of constraint that's returned, so with sufficient bad luck it can return the OID of a foreign key constraint. This has the effect that a primary key in a partition can end up as a child of a foreign key, which makes no sense (it needs to be the child of the equivalent primary key.) Change the API contract so that only index-backed constraints are returned, mimicking get_constraint_index(). 2. Both CloneFkReferenced and CloneFkReferencing clone a self-referencing foreign key, so the partition ends up with a duplicate foreign key. Change the former function to ignore such constraints. Add some tests to verify that things are better now. (However, these new tests show some additional misbehavior that will be fixed later -- namely that there's a constraint marked NOT VALID.) Backpatch to 12, where these constraints are possible at all. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220603154232.1715b14c@karst
* Rename shadowed local variablesDavid Rowley2022-10-05
| | | | | | | | | | | | In a similar effort to f01592f91, here we mostly rename shadowed local variables to remove the warnings produced when compiling with -Wshadow=compatible-local. This fixes 63 warnings and leaves just 5. Author: Justin Pryzby, David Rowley Reviewed-by: Justin Pryzby Discussion https://postgr.es/m/20220817145434.GC26426%40telsasoft.com
* Revert 56-bit relfilenode change and follow-up commits.Robert Haas2022-09-28
| | | | | | | | There are still some alignment-related failures in the buildfarm, which might or might not be able to be fixed quickly, but I've also just realized that it increased the size of many WAL records by 4 bytes because a block reference contains a RelFileLocator. The effect of that hasn't been studied or discussed, so revert for now.
* Increase width of RelFileNumbers from 32 bits to 56 bits.Robert Haas2022-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | RelFileNumbers are now assigned using a separate counter, instead of being assigned from the OID counter. This counter never wraps around: if all 2^56 possible RelFileNumbers are used, an internal error occurs. As the cluster is limited to 2^64 total bytes of WAL, this limitation should not cause a problem in practice. If the counter were 64 bits wide rather than 56 bits wide, we would need to increase the width of the BufferTag, which might adversely impact buffer lookup performance. Also, this lets us use bigint for pg_class.relfilenode and other places where these values are exposed at the SQL level without worrying about overflow. This should remove the need to keep "tombstone" files around until the next checkpoint when relations are removed. We do that to keep RelFileNumbers from being recycled, but now that won't happen anyway. However, this patch doesn't actually change anything in this area; it just makes it possible for a future patch to do so. Dilip Kumar, based on an idea from Andres Freund, who also reviewed some earlier versions of the patch. Further review and some wordsmithing by me. Also reviewed at various points by Ashutosh Sharma, Vignesh C, Amul Sul, Álvaro Herrera, and Tom Lane. Discussion: http://postgr.es/m/CA+Tgmobp7+7kmi4gkq7Y+4AM9fTvL+O1oQ4-5gFTT+6Ng-dQ=g@mail.gmail.com
* Allow publications with schema and table of the same schema.Amit Kapila2022-09-23
| | | | | | | | | | | | | | | | | | | | | We previously thought that allowing such cases can confuse users when they specify DROP TABLES IN SCHEMA but that doesn't seem to be the case based on discussion. This helps to uplift the restriction during ALTER TABLE ... SET SCHEMA which used to ensure that we couldn't end up with a publication having both a schema and the same schema's table. To allow this, we need to forbid having any schema on a publication if column lists on a table are specified (and vice versa). This is because otherwise we still need a restriction during ALTER TABLE ... SET SCHEMA to forbid cases where it could lead to a publication having both a schema and the same schema's table with column list. Based on suggestions by Peter Eisentraut. Author: Hou Zhijie and Vignesh C Reviewed-By: Peter Smith, Amit Kapila Backpatch-through: 15, where it was introduced Discussion: https://postgr.es/m/2729c9e2-9aac-8cda-f2f4-34f2bcc18f4e@enterprisedb.com
* Harmonize more parameter names in bulk.Peter Geoghegan2022-09-20
| | | | | | | | | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions in optimizer, parser, utility, libpq, and "commands" code, as well as in remaining library code. Do the same for all code related to frontend programs (with the exception of pg_dump/pg_dumpall related code). Like other recent commits that cleaned up function parameter names, this commit was written with help from clang-tidy. Later commits will handle ecpg and pg_dump/pg_dumpall. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
* Fix GetForeignKey*Triggers for self-referential FKsAlvaro Herrera2022-09-09
| | | | | | | | | | | | | | | | | | | | Because of inadequate filtering, the check triggers were confusing the search for action triggers in GetForeignKeyActionTriggers and vice-versa in GetForeignKeyCheckTriggers; this confusion results in seemingly random assertion failures, and can have real impact in non-asserting builds depending on catalog order. Change these functions so that they correctly ignore triggers that are not relevant to each side. To reduce the odds of further problems, do not break out of the searching loop in assertion builds. This break is likely to hide bugs; without it, we would have detected this bug immediately. This problem was introduced by f4566345cf40, so backpatch to 15 where that commit first appeared. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/20220908172029.sejft2ppckbo6oh5@awork3.anarazel.de Discussion: https://postgr.es/m/4104619.1662663056@sss.pgh.pa.us
* Choose FK name correctly during partition attachmentAlvaro Herrera2022-09-08
| | | | | | | | | | | | | | | During ALTER TABLE ATTACH PARTITION, if the name of a parent's foreign key constraint is already used on the partition, the code tries to choose another one before the FK attributes list has been populated, so the resulting constraint name was "<relname>__fkey" instead of "<relname>_<attrs>_fkey". Repair, and add a test case. Backpatch to 12. In 11, the code to attach a partition was not smart enough to cope with conflicting constraint names, so the problem doesn't exist there. Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com> Discussion: https://postgr.es/m/20220901184156.738ebee5@karst
* More -Wshadow=compatible-local warning fixesDavid Rowley2022-08-26
| | | | | | | | | | | | In a similar effort to f01592f91, here we're targetting fixing the warnings where we've deemed the shadowing variable to serve a close enough purpose to the shadowed variable just to reuse the shadowed version and not declare the shadowing variable at all. By my count, this takes the warning count from 106 down to 71. Author: Justin Pryzby Discussion: https://postgr.es/m/20220825020839.GT2342@telsasoft.com
* Ensure that pg_auth_members.grantor is always valid.Robert Haas2022-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, "GRANT foo TO bar" or "GRANT foo TO bar GRANTED BY baz" would record the OID of the grantor in pg_auth_members.grantor, but that role could later be dropped without modifying or removing the pg_auth_members record. That's not great, because we typically try to avoid dangling references in catalog data. Now, a role grant depends on the grantor, and the grantor can't be dropped without removing the grant or changing the grantor. "DROP OWNED BY" will remove the grant, just as it does for other kinds of privileges. "REASSIGN OWNED BY" will not, again just like what we do in other cases involving privileges. pg_auth_members now has an OID column, because that is needed in order for dependencies to work. It also now has an index on the grantor column, because otherwise dropping a role would require a sequential scan of the entire table to see whether the role's OID is in use as a grantor. That probably wouldn't be too large a problem in practice, but it seems better to have an index just in case. A follow-on patch is planned with the goal of more thoroughly rationalizing the behavior of role grants. This patch is just trying to do enough to make sure that the data we store in the catalogs is at some basic level valid. Patch by me, reviewed by Stephen Frost Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com
* Avoid using list_length() to test for empty list.Tom Lane2022-08-17
| | | | | | | | | | | | | | | | | | | | | | | | The standard way to check for list emptiness is to compare the List pointer to NIL; our list code goes out of its way to ensure that that is the only representation of an empty list. (An acceptable alternative is a plain boolean test for non-null pointer, but explicit mention of NIL is usually preferable.) Various places didn't get that memo and expressed the condition with list_length(), which might not be so bad except that there were such a variety of ways to check it exactly: equal to zero, less than or equal to zero, less than one, yadda yadda. In the name of code readability, let's standardize all those spellings as "list == NIL" or "list != NIL". (There's probably some microscopic efficiency gain too, though few of these look to be at all performance-critical.) A very small number of cases were left as-is because they seemed more consistent with other adjacent list_length tests that way. Peter Smith, with bikeshedding from a number of us Discussion: https://postgr.es/m/CAHut+PtQYe+ENX5KrONMfugf0q6NHg4hR5dAhqEXEc2eefFeig@mail.gmail.com
* Fix ENABLE/DISABLE TRIGGER to handle recursion correctlyAlvaro Herrera2022-08-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Using ATSimpleRecursion() in ATPrepCmd() to do so as bbb927b4db9b did is not correct, because ATPrepCmd() can't distinguish between triggers that may be cloned and those that may not, so would wrongly try to recurse for the latter category of triggers. So this commit restores the code in EnableDisableTrigger() that 86f575948c77 had added to do the recursion, which would do it only for triggers that may be cloned, that is, row-level triggers. This also changes tablecmds.c such that ATExecCmd() is able to pass the value of ONLY flag down to EnableDisableTrigger() using its new 'recurse' parameter. This also fixes what seems like an oversight of 86f575948c77 that the recursion to partition triggers would only occur if EnableDisableTrigger() had actually changed the trigger. It is more apt to recurse to inspect partition triggers even if the parent's trigger didn't need to be changed: only then can we be certain that all descendants share the same state afterwards. Backpatch all the way back to 11, like bbb927b4db9b. Care is taken not to break ABI compatibility (and that no catversion bump is needed.) Co-authored-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Dmitry Koval <d.koval@postgrespro.ru> Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
* Feed ObjectAddress to event triggers for ALTER TABLE ATTACH/DETACHMichael Paquier2022-07-31
| | | | | | | | | | | | These flavors of ALTER TABLE were already shaped to report the ObjectAddress of the partition attached or detached, but this data was not added to what is collected for event triggers. The tests of test_ddl_deparse are updated to show the modification in the data reported. Author: Hou Zhijie Reviewed-by: Álvaro Herrera, Amit Kapila, Hayato Kuroda, Michael Paquier Discussion: https://postgr.es/m/OS0PR01MB571626984BD099DADF53F38394899@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Use TRUNCATE to preserve relfilenode for pg_largeobject + index.Robert Haas2022-07-28
| | | | | | | | | | | | | | | | | | | | | | Commit 9a974cbcba005256a19991203583a94b4f9a21a9 arranged to preserve the relfilenode of user tables across pg_upgrade, but failed to notice that pg_upgrade treats pg_largeobject as a user table and thus it needs the same treatment. Otherwise, large objects will appear to vanish after a pg_upgrade. Commit d498e052b4b84ae21b3b68d5b3fda6ead65d1d4d fixed this problem by teaching pg_dump to UPDATE pg_class.relfilenode for pg_largeobject and its index. However, because an UPDATE on the catalog rows doesn't change anything on disk, this can leave stray files behind in the new cluster. They will normally be empty, but it's a little bit untidy. Hence, this commit arranges to do the same thing using DDL. Specifically, it makes TRUNCATE work for the pg_largeobject catalog when in binary-upgrade mode, and it then uses that command in binary-upgrade dumps as a way of setting pg_class.relfilenode for pg_largeobject and its index. That way, the old files are removed from the new cluster. Discussion: http://postgr.es/m/CA+TgmoYYMXGUJO5GZk1-MByJGu_bB8CbOL6GJQC8=Bzt6x6vDg@mail.gmail.com
* Tweak detail and hint messages to be consistent with project policyMichael Paquier2022-07-20
| | | | | | | | | | | Detail and hint messages should be full sentences and should end with a period, but some of the messages newly-introduced in v15 did not follow that. Author: Justin Pryzby Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/20220719120948.GF12702@telsasoft.com Backpatch-through: 15
* Replace many MemSet calls with struct initializationPeter Eisentraut2022-07-16
| | | | | | | | | | | | | | This replaces all MemSet() calls with struct initialization where that is easily and obviously possible. (For example, some cases have to worry about padding bits, so I left those.) (The same could be done with appropriate memset() calls, but this patch is part of an effort to phase out MemSet(), so it doesn't touch memset() calls.) Reviewed-by: Ranier Vilela <ranier.vf@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/9847b13c-b785-f4e2-75c3-12ec77a3b05c@enterprisedb.com
* Allow specifying STORAGE attribute for a new tablePeter Eisentraut2022-07-13
| | | | | | | | | | | | | | | | Previously, the STORAGE specification was only available in ALTER TABLE. This makes it available in CREATE TABLE as well. Also make the code and the documentation for STORAGE and COMPRESSION attributes consistent. Author: Teodor Sigaev <teodor@sigaev.ru> Author: Aleksander Alekseev <aleksander@timescale.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: wenjing zeng <wjzeng2012@gmail.com> Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/de83407a-ae3d-a8e1-a788-920eb334f25b@sigaev.ru
* Remove useless assertionsPeter Eisentraut2022-07-13
| | | | | | We don't need Assert(IsA(foo, String)) right before running strVal(foo), since strVal() already does the assertion internally (via castNode()).
* Change internal RelFileNode references to RelFileNumber or RelFileLocator.Robert Haas2022-07-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have been using the term RelFileNode to refer to either (1) the integer that is used to name the sequence of files for a certain relation within the directory set aside for that tablespace/database combination; or (2) that value plus the OIDs of the tablespace and database; or occasionally (3) the whole series of files created for a relation based on those values. Using the same name for more than one thing is confusing. Replace RelFileNode with RelFileNumber when we're talking about just the single number, i.e. (1) from above, and with RelFileLocator when we're talking about all the things that are needed to locate a relation's files on disk, i.e. (2) from above. In the places where we refer to (3) as a relfilenode, instead refer to "relation storage". Since there is a ton of SQL code in the world that knows about pg_class.relfilenode, don't change the name of that column, or of other SQL-facing things that derive their name from it. On the other hand, do adjust closely-related internal terminology. For example, the structure member names dbNode and spcNode appear to be derived from the fact that the structure itself was called RelFileNode, so change those to dbOid and spcOid. Likewise, various variables with names like rnode and relnode get renamed appropriately, according to how they're being used in context. Hopefully, this is clearer than before. It is also preparation for future patches that intend to widen the relfilenumber fields from its current width of 32 bits. Variables that store a relfilenumber are now declared as type RelFileNumber rather than type Oid; right now, these are the same, but that can now more easily be changed. Dilip Kumar, per an idea from me. Reviewed also by Andres Freund. I fixed some whitespace issues, changed a couple of words in a comment, and made one other minor correction. Discussion: http://postgr.es/m/CA+TgmoamOtXbVAQf9hWFzonUo6bhhjS6toZQd7HZ-pmojtAmag@mail.gmail.com Discussion: http://postgr.es/m/CA+Tgmobp7+7kmi4gkq7Y+4AM9fTvL+O1oQ4-5gFTT+6Ng-dQ=g@mail.gmail.com Discussion: http://postgr.es/m/CAFiTN-vTe79M8uDH1yprOU64MNFE+R3ODRuA+JWf27JbhY4hJw@mail.gmail.com
* Pre-beta mechanical code beautification.Tom Lane2022-05-12
| | | | | Run pgindent, pgperltidy, and reformat-dat-files. I manually fixed a couple of comments that pgindent uglified.
* Remove extraneous blank lines before block-closing bracesAlvaro Herrera2022-04-13
| | | | | | | | | These are useless and distracting. We wouldn't have written the code with them to begin with, so there's no reason to keep them. Author: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com Discussion: https://postgr.es/m/attachment/133167/0016-Extraneous-blank-lines.patch
* Remove "recheck" argument from check_index_is_clusterable()Michael Paquier2022-04-13
| | | | | | | | | | | The last usage of this argument in this routine can be tracked down to 7e2f9062, aka 11 years ago. Getting rid of this argument can also be an advantage for extensions calling check_index_is_clusterable(), as it removes any need to worry about the meaning of what a recheck would be at this level. Author: Justin Pryzby Discussion: https://postgr.es/m/20220411140609.GF26620@telsasoft.com
* Revert "Logical decoding of sequences"Tomas Vondra2022-04-07
| | | | | | | | | | | | | | | | | | | | | This reverts a sequence of commits, implementing features related to logical decoding and replication of sequences: - 0da92dc530c9251735fc70b20cd004d9630a1266 - 80901b32913ffa59bf157a4d88284b2b3a7511d9 - b779d7d8fdae088d70da5ed9fcd8205035676df3 - d5ed9da41d96988d905b49bebb273a9b2d6e2915 - a180c2b34de0989269fdb819bff241a249bf5380 - 75b1521dae1ff1fde17fda2e30e591f2e5d64b6a - 2d2232933b02d9396113662e44dca5f120d6830e - 002c9dd97a0c874fd1693a570383e2dd38cd40d5 - 05843b1aa49df2ecc9b97c693b755bd1b6f856a9 The implementation has issues, mostly due to combining transactional and non-transactional behavior of sequences. It's not clear how this could be fixed, but it'll require reworking significant part of the patch. Discussion: https://postgr.es/m/95345a19-d508-63d1-860a-f5c2f41e8d40@enterprisedb.com
* Unlogged sequencesPeter Eisentraut2022-04-07
| | | | | | | | | | | | | | | | | | Add support for unlogged sequences. Unlike for unlogged tables, this is not a performance feature. It allows sequences associated with unlogged tables to be excluded from replication. A new subcommand ALTER SEQUENCE ... SET LOGGED/UNLOGGED is added. An identity/serial sequence now automatically gets and follows the persistence level (logged/unlogged) of its owning table. (The sequences owned by temporary tables were already temporary through the separate mechanism in RangeVarAdjustRelationPersistence().) But you can still change the persistence of an owned sequence separately. Also, pg_dump and pg_upgrade preserve the persistence of existing sequences. Discussion: https://www.postgresql.org/message-id/flat/04e12818-2f98-257c-b926-2845d74ed04f%402ndquadrant.com
* Allow granting SET and ALTER SYSTEM privileges on GUC parameters.Tom Lane2022-04-06
| | | | | | | | | | | | | | | | | | | | This patch allows "PGC_SUSET" parameters to be set by non-superusers if they have been explicitly granted the privilege to do so. The privilege to perform ALTER SYSTEM SET/RESET on a specific parameter can also be granted. Such privileges are cluster-wide, not per database. They are tracked in a new shared catalog, pg_parameter_acl. Granting and revoking these new privileges works as one would expect. One caveat is that PGC_USERSET GUCs are unaffected by the SET privilege --- one could wish that those were handled by a revocable grant to PUBLIC, but they are not, because we couldn't make it robust enough for GUCs defined by extensions. Mark Dilger, reviewed at various times by Andrew Dunstan, Robert Haas, Joshua Brindle, and myself Discussion: https://postgr.es/m/3D691E20-C1D5-4B80-8BA5-6BEB63AF3029@enterprisedb.com
* Allow CLUSTER on partitioned tablesAlvaro Herrera2022-04-02
| | | | | | | | | | | | | | | | | | | | This is essentially the same as applying VACUUM FULL to a partitioned table, which has been supported since commit 3c3bb99330aa (March 2017). While there's no great use case in applying CLUSTER to partitioned tables, we don't have any strong reason not to allow it either. For now, partitioned indexes cannot be marked clustered, so an index must always be specified. While at it, rename some variables that were RangeVars during the development that led to 8bc717cb8878 but never made it that way to the source tree; there's no need to perpetuate names that have always been more confusing than helpful. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/20201028003312.GU9241@telsasoft.com Discussion: https://postgr.es/m/20200611153502.GT14879@telsasoft.com
* Add new block-by-block strategy for CREATE DATABASE.Robert Haas2022-03-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because this strategy logs changes on a block-by-block basis, it avoids the need to checkpoint before and after the operation. However, because it logs each changed block individually, it might generate a lot of extra write-ahead logging if the template database is large. Therefore, the older strategy remains available via a new STRATEGY parameter to CREATE DATABASE, and a corresponding --strategy option to createdb. Somewhat controversially, this patch assembles the list of relations to be copied to the new database by reading the pg_class relation of the template database. Cross-database access like this isn't normally possible, but it can be made to work here because there can't be any connections to the database being copied, nor can it contain any in-doubt transactions. Even so, we have to use lower-level interfaces than normal, since the table scan and relcache interfaces will not work for a database to which we're not connected. The advantage of this approach is that we do not need to rely on the filesystem to determine what ought to be copied, but instead on PostgreSQL's own knowledge of the database structure. This avoids, for example, copying stray files that happen to be located in the source database directory. Dilip Kumar, with a fairly large number of cosmetic changes by me. Reviewed and tested by Ashutosh Sharma, Andres Freund, John Naylor, Greg Nancarrow, Neha Sharma. Additional feedback from Bruce Momjian, Heikki Linnakangas, Julien Rouhaud, Adam Brusselback, Kyotaro Horiguchi, Tomas Vondra, Andrew Dunstan, Álvaro Herrera, and others. Discussion: http://postgr.es/m/CA+TgmoYtcdxBjLh31DLxUXHxFVMPGzrU5_T=CYCvRyFHywSBUQ@mail.gmail.com
* Add decoding of sequences to built-in replicationTomas Vondra2022-03-24
| | | | | | | | | | | | | | | | | | | | | | | | | This commit adds support for decoding of sequences to the built-in replication (the infrastructure was added by commit 0da92dc530). The syntax and behavior mostly mimics handling of tables, i.e. a publication may be defined as FOR ALL SEQUENCES (replicating all sequences in a database), FOR ALL SEQUENCES IN SCHEMA (replicating all sequences in a particular schema) or individual sequences. To publish sequence modifications, the publication has to include 'sequence' action. The protocol is extended with a new message, describing sequence increments. A new system view pg_publication_sequences lists all the sequences added to a publication, both directly and indirectly. Various psql commands (\d and \dRp) are improved to also display publications including a given sequence, or sequences included in a publication. Author: Tomas Vondra, Cary Huang Reviewed-by: Peter Eisentraut, Amit Kapila, Hannu Krosing, Andres Freund, Petr Jelinek Discussion: https://postgr.es/m/d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com Discussion: https://postgr.es/m/1710ed7e13b.cd7177461430746.3372264562543607781@highgo.ca
* Fix bogus dependency handling for GENERATED expressions.Tom Lane2022-03-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For GENERATED columns, we record all dependencies of the generation expression as AUTO dependencies of the column itself. This means that the generated column is silently dropped if any dependency is removed, even if CASCADE wasn't specified. This is at least a POLA violation, but I think it's actually based on a misreading of the standard. The standard does say that you can't drop a dependent GENERATED column in RESTRICT mode; but that's buried down in a subparagraph, on a different page from some pseudocode that makes it look like an AUTO drop is being suggested. Change this to be more like the way that we handle regular default expressions, ie record the dependencies as NORMAL dependencies of the pg_attrdef entry. Also, make the pg_attrdef entry's dependency on the column itself be INTERNAL not AUTO. That has two effects: * the column will go away, not just lose its default, if any dependency of the expression is dropped with CASCADE. So we don't need any special mechanism to make that happen. * it provides an additional cross-check preventing someone from dropping the default expression without dropping the column. catversion bump because of change in the contents of pg_depend (which also requires a change in one information_schema view). Per bug #17439 from Kevin Humphreys. Although this is a longstanding bug, it seems impractical to back-patch because of the need for catalog contents changes. Discussion: https://postgr.es/m/17439-7df4421197e928f0@postgresql.org
* Move pg_attrdef manipulation code into new file catalog/pg_attrdef.c.Tom Lane2022-03-21
| | | | | | | | | | | | | | | | | | | | | This is a pure refactoring commit: there isn't (I hope) any functional change. StoreAttrDefault and RemoveAttrDefault[ById] are moved from heap.c, reducing the size of that overly-large file by about 300 lines. I took the opportunity to trim unused #includes from heap.c, too. Two new functions for translating between a pg_attrdef OID and the relid/attnum of the owning column are created by extracting ad-hoc code from objectaddress.c. This already removes one copy of said code, and a follow-on bug fix will create more callers. The only other function directly manipulating pg_attrdef is AttrDefaultFetch. I judged it was better to leave that in relcache.c, since it shares special concerns about recursion and error handling with the rest of that module. Discussion: https://postgr.es/m/651168.1647451676@sss.pgh.pa.us
* Fix risk of deadlock failure while dropping a partitioned index.Tom Lane2022-03-21
| | | | | | | | | | | | | | | | | | | | | | | DROP INDEX needs to lock the index's table before the index itself, else it will deadlock against ordinary queries that acquire the relation locks in that order. This is correctly mechanized for plain indexes by RangeVarCallbackForDropRelation; but in the case of a partitioned index, we neglected to lock the child tables in advance of locking the child indexes. We can fix that by traversing the inheritance tree and acquiring the needed locks in RemoveRelations, after we have acquired our locks on the parent partitioned table and index. While at it, do some refactoring to eliminate confusion between the actual and expected relkind in RangeVarCallbackForDropRelation. We can save a couple of syscache lookups too, by having that function pass back info that RemoveRelations will need. Back-patch to v11 where partitioned indexes were added. Jimmy Yih, Gaurab Dey, Tom Lane Discussion: https://postgr.es/m/BYAPR05MB645402330042E17D91A70C12BD5F9@BYAPR05MB6454.namprd05.prod.outlook.com
* Parse/analyze function renamingPeter Eisentraut2022-03-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are three parallel ways to call parse/analyze: with fixed parameters, with variable parameters, and by supplying your own parser callback. Some of the involved functions were confusingly named and made this API structure more confusing. This patch renames some functions to make this clearer: parse_analyze() -> parse_analyze_fixedparams() pg_analyze_and_rewrite() -> pg_analyze_and_rewrite_fixedparams() (Otherwise one might think this variant doesn't accept parameters, but in fact all three ways accept parameters.) pg_analyze_and_rewrite_params() -> pg_analyze_and_rewrite_withcb() (Before, and also when considering pg_analyze_and_rewrite(), one might think this is the only way to pass parameters. Moreover, the parser callback doesn't necessarily need to parse only parameters, it's just one of the things it could do.) parse_fixed_parameters() -> setup_parse_fixed_parameters() parse_variable_parameters() -> setup_parse_variable_parameters() (These functions don't actually do any parsing, they just set up callbacks to use during parsing later.) This patch also adds some const decorations to the fixed-parameters API, so the distinction from the variable-parameters API is more clear. Reviewed-by: Nathan Bossart <bossartn@amazon.com> Discussion: https://www.postgresql.org/message-id/flat/c67ce276-52b4-0239-dc0e-39875bf81840@enterprisedb.com
* Remove xloginsert.h from xlog.hAlvaro Herrera2022-01-30
| | | | | | | | | xlog.h is directly and indirectly #included in a lot of places. With this change, xloginsert.h is no longer unnecessarily included in the large number of them that don't need it. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/CALj2ACVe-W+WM5P44N7eG9C2_FmaeM8Dq5aCnD3fHt0Ba=WR6w@mail.gmail.com
* Update copyright for 2022Bruce Momjian2022-01-07
| | | | Backpatch-through: 10
* Prevent altering partitioned table's rowtype, if it's used elsewhere.Tom Lane2022-01-06
| | | | | | | | | | | | | | We disallow altering a column datatype within a regular table, if the table's rowtype is used as a column type elsewhere, because we lack code to go around and rewrite the other tables. This restriction should apply to partitioned tables as well, but it was not checked because ATRewriteTables and ATPrepAlterColumnType were not on the same page about who should do it for which relkinds. Per bug #17351 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17351-6db1870f3f4f612a@postgresql.org
* Create foreign key triggers in partitioned tables tooAlvaro Herrera2022-01-05
| | | | | | | | | | | | | | | | | | | | | | | | | | While user-defined triggers defined on a partitioned table have a catalog definition for both it and its partitions, internal triggers used by foreign keys defined on partitioned tables only have a catalog definition for its partitions. This commit fixes that so that partitioned tables get the foreign key triggers too, just like user-defined triggers. Moreover, like user-defined triggers, partitions' internal triggers will now also have their tgparentid set appropriately. This is to allow subsequent commit(s) to make the foreign key related events to be fired in some cases using the parent table triggers instead of those of partitions'. This also changes what tgisinternal means in some cases. Currently, it means either that the trigger is an internal implementation object of a foreign key constraint, or a "child" trigger on a partition cloned from the trigger on the parent. This commit changes it to only mean the former to avoid confusion. As for the latter, it can be told by tgparentid being nonzero, which is now true both for user- defined and foreign key's internal triggers. Author: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Arne Roland <A.Roland@index.de> Discussion: https://postgr.es/m/CA+HiwqG7LQSK+n8Bki8tWv7piHD=PnZro2y6ysU2-28JS6cfgQ@mail.gmail.com