Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Il 30/07/12 19:16, Gabriele Bartolini ha scritto: And it can be also interchanged with "Array element Foreign Key". As promised, we have sent a patch for the "Array ELEMENT foreign key" support. We are discontinuing this thread here and continue discussing the former "Foreign Keys with Arrays"/"EACH Foreign Key" feature support from here: http://archives.postgresql.org/pgsql-hackers/2012-08/msg00011.php Thank you, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Il 30/07/12 19:11, Alvaro Herrera ha scritto: I was going to say the same, except I had ELEMENT as a capitalized word in my mind (and in the docs it'd be within ). So: "Array ELEMENT Foreign Key" +1 for me And it can be also interchanged with "Array element Foreign Key". -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Excerpts from Simon Riggs's message of lun jul 30 11:21:46 -0400 2012: > On 30 July 2012 16:12, Gabriele Bartolini > wrote: > > > * Array Foreign Key > > * Foreign Key Arrays > > * ELEMENT Foreign Keys > > * ... > > > >Which one is your favourite? > > Array Element Foreign Key I was going to say the same, except I had ELEMENT as a capitalized word in my mind (and in the docs it'd be within ). -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Mon, Jul 30, 2012 at 11:12 AM, Gabriele Bartolini wrote: > Hi guys, > >it is time to give another go to this patch. I would like to thank > everyone for suggestions and ideas expressed through this list. > >We are happy that "Part 0" of the patch has been committed > (array_remove() and array_replace() functions), which will be useful in > "Part 2" (too early now to talk about it). Let's not rush though and focus > on "Part 1" of the patch. :) > >First, I would like to find a unique and general term for this feature. > We started with "Foreign keys with arrays" and ended up with "EACH foreign > keys". Following Peter's suggestion, we will use the "ELEMENT" keyword (so > that maybe in the future we can extend the usage). Our proposals are: > > * Array Foreign Key > * Foreign Key Arrays > * ELEMENT Foreign Keys > * ... > >Which one is your favourite? I think having the word "element" in there makes it a lot clearer. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On 30 July 2012 16:12, Gabriele Bartolini wrote: > * Array Foreign Key > * Foreign Key Arrays > * ELEMENT Foreign Keys > * ... > >Which one is your favourite? Array Element Foreign Key -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi guys, it is time to give another go to this patch. I would like to thank everyone for suggestions and ideas expressed through this list. We are happy that "Part 0" of the patch has been committed (array_remove() and array_replace() functions), which will be useful in "Part 2" (too early now to talk about it). Let's not rush though and focus on "Part 1" of the patch. :) First, I would like to find a unique and general term for this feature. We started with "Foreign keys with arrays" and ended up with "EACH foreign keys". Following Peter's suggestion, we will use the "ELEMENT" keyword (so that maybe in the future we can extend the usage). Our proposals are: * Array Foreign Key * Foreign Key Arrays * ELEMENT Foreign Keys * ... Which one is your favourite? Secondly, we have decided to split the patch we proposed back in March in two smaller patches. The most important goal of "Part 1" is to find a generally accepted syntax. By removing ACTION handling from "Part 1" (see limitations below), we believe that the community will be able to contribute more to driving future directions and requirements. Based on Peter's comments, we would like to propose the use of the "ELEMENT" keyword, rather than the "EACH" keyword proposed in March. You can find three examples at the bottom of this email. Finally, "Part 1" of this patch will have these limitations: * Only one |ELEMENT| column allowed in a multi-column key (same as the proposed patch in March) * Supported actions|: * NO ACTION|| * RESTRICT| Cheers, Gabriele Example 1: inline usage CREATE TABLE drivers ( driver_id integer PRIMARY KEY, first_name text, last_name text, ... ); CREATE TABLE races ( race_id integer PRIMARY KEY, title text, race_day DATE, ... final_positions integer[] ELEMENT REFERENCES drivers ); Example 2: with FOREIGN KEY CREATE TABLE races ( race_id integer PRIMARY KEY, title text, race_day DATE, ... final_positions integer[], FOREIGN KEY (ELEMENT final_positions) REFERENCES drivers ); Example 3: with ALTER TABLE CREATE TABLE races ( race_id integer PRIMARY KEY, title text, race_day DATE, ... final_positions integer[] ); ALTER TABLE races ADD FOREIGN KEY (ELEMENT final_positions) REFERENCES drivers; -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
2012/6/18 Kevin Grittner > The many-to-one case seems like it is better handled in the other > direction -- with the referenced table holding the set of valid keys > and the referencing table holding the single key. (I believe the > general case of this is what Jeff called an "inclusion constraint" > -- a feature he wants to add at some point.) I can't think of a use > case where that would not be better for this type of relationship > than putting the array on the referencing side. > > Hi Kevin, Well, from my point of view "many-to-one" or "one-to-many" is more related from which point we are looking to the thing... But let me better explain what I thought... Example 1) If we have one table with master data TableA(ID, other properties...) and TableB(tableAIDFK, propA, propB, propC) -PK of TableB is irrelavant in this point... and let say a lot of TableA tuples could have the same TableB properties... So we can have how many common TableA tuples, that many tuples in TableB with the same values in PropA, PropB, and PropC with FK the same type as in Table A, or to have 1 tuple in TableB with Array type as FK field So it (1 tuple in TableB) can point many tuples in TableC... And in the same time simple element can exist in TableA, but could or doesn't have to exist in TableB... What test would show is there any gain in this approach - I don't know... but think it should - especially if propA,PropB, and C should be updated for all of them... Example 2) >From other side, what Jeff propose, and what is also usefull, but different thing is... to have the main data in TableA, but key field is an range datatype... what later each element what belong to the range, could have related tuple in TableB (Also, as the same range datatype - but smaller... contained by Master one... or simple datatype subtype of the range) - which is other way around... Opposite from exmaple 1 - but differnet from functional point of view... Depending what is the Master... Also, for example 1 - data in FK do not need to be in range.. so basicaly ID [1, 2 ,4 ,7] could have 1 tuple with its properties, and [3,5,6] in second tuple with different properties... I am not sure Example 2) Jeff called "Inclusion Constraint" - Jeff can explain it better :) Based on Simon Riggs wrote: >> Do we need something like Exclusion FKs? i.e. the FK partner of >> Exclusion Constraints? >Yes, "Inclusion Constraints". I've known we need something like that >since I did Exclusion Constraints, but I haven't gotten further than >that. >Regards, > Jeff Davis I have understood it as: TableA(ID, properties...) TableB(ID, properties...) Now if we define FK on TableB to TableA... It means that row inserted in TableB, must have already row with the same ID value in TableA... But what would be usefull, to define Exclude FK to table A, to we prevent insert new row in Table B with ID value what already exist in TableA... btw, if anyone is happy to point me in right direction, and there is common feeling it is usefull feature, I am happy to code it... Actually that is something what I will code anyway for "in-house" solution - but would be good to do it under Postgres standards... Kind Regards, Misa
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Misa Simic wrote: > 2012/6/17 Kevin Grittner >> Can someone provide a practical example of a "foreign key with >> array" use case? The only situations I'm able to think of right >> now are the same cases where you would now use a table with >> primary keys of two tables to provide a many-to-many linkage. >> Does this proposed feature handle other cases or handle this type >> of case better? > > I can't imagine either other usablity... Just many-to-one > linkage... or to have many-to-many link with less rows in middle > table... The many-to-one case seems like it is better handled in the other direction -- with the referenced table holding the set of valid keys and the referencing table holding the single key. (I believe the general case of this is what Jeff called an "inclusion constraint" -- a feature he wants to add at some point.) I can't think of a use case where that would not be better for this type of relationship than putting the array on the referencing side. The many-to-many relationship does seem like a potentially useful feature, at least in some cases. For example, a message board where a limited number of tags can be attached to each message -- the message could contain an array of tag IDs. Clearly this could be done as a table holding message_id and tag_id, but it seems potentially more convenient from a programming perspective to have an array of tag_id values in the message table. Logically, you are associating each of these with the primary key of the row it is in. Are there other obvious use-cases? If nobody can put one forward, this seems like a good "reality test" for proposed behaviors in any corner cases where "correct" behavior isn't obvious -- what would you want to do to the data if it were in the separate table with just the primary keys to link the two tables? > What is better - I think should be measured... It would make an interesting test to compare performance of a suitably sized data set and reasonable workload for both techniques. Of course, that's a non-trivial amount of work. It seems like people may be able to justify this just on ease-of-use, versus claiming that it's an optimization. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sun, 2012-06-17 at 21:10 +0800, Simon Riggs wrote: > Do we need something like Exclusion FKs? i.e. the FK partner of > Exclusion Constraints? Yes, "Inclusion Constraints". I've known we need something like that since I did Exclusion Constraints, but I haven't gotten further than that. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sun, Jun 17, 2012 at 09:58:17AM -0500, Kevin Grittner wrote: > Simon Riggs wrote: > Misa Simic wrote: > > >> IMO, both approaches make sense... > > > > Agreed. > > Can someone provide a practical example of a "foreign key with array" > use case? The only situations I'm able to think of right now are the > same cases where you would now use a table with primary keys of two > tables to provide a many-to-many linkage. Does this proposed feature > handle other cases or handle this type of case better? The way I think about "array foreign keys" is that they represent the "aggregated" form of a classical foreign key. In the aggregated form, each row in the referencing side represents a group of rows in the non-aggregated form. One advantage is that constraints on each group of rows as a whole are now possible, because they become constraints on a single row in the aggregated form. Example. If you have a table of points, then you can have a table of polygons where each polygon contains an array of points. The non-aggregated model would instead require an additional point_polygon table which references both the point and the polygon table, because the point <-> polygon relationship is many-to-many. In the aggregated model, you can easily specify a CHECK constraint that requires each polygon to have at least three points, while the corresponding condition cannot be specified in the non-aggregated model. Cheers, Dr. Gianni Ciolli - 2ndQuadrant Italia PostgreSQL Training, Services and Support gianni.cio...@2ndquadrant.it | www.2ndquadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
2012/6/17 Kevin Grittner > > > Can someone provide a practical example of a "foreign key with array" > use case? The only situations I'm able to think of right now are the > same cases where you would now use a table with primary keys of two > tables to provide a many-to-many linkage. Does this proposed feature > handle other cases or handle this type of case better? > I can't imagine either other usablity... Just many-to-one linkage... or to have many-to-many link with less rows in middle table... What is better - I think should be measured...
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
2012/6/17 Simon Riggs > > Do we need something like Exclusion FKs? i.e. the FK partner of > Exclusion Constraints? > +1 Definatelly it would be something usefull... Today's workaround to achieve that with additional table, and additional column in Key is a bit awkward... > > -- > Simon Riggs http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services >
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Simon Riggs wrote: Misa Simic wrote: >> IMO, both approaches make sense... > > Agreed. Can someone provide a practical example of a "foreign key with array" use case? The only situations I'm able to think of right now are the same cases where you would now use a table with primary keys of two tables to provide a many-to-many linkage. Does this proposed feature handle other cases or handle this type of case better? The "referencing value is contained by the referenced value" has many obvious uses. For example, in our courts data we have a statute table which effectively has a statute cite and effective date range for the primary key, and we have a charge table with a statute cite and an offense date used to match it to a statute row. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On 17 June 2012 19:16, Misa Simic wrote: > IMO, both approaches make sense... Agreed. It's also a good reason to do as Peter suggests and come up with a better description than just EACH. Do we need something like Exclusion FKs? i.e. the FK partner of Exclusion Constraints? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
IMO, both approaches make sense... >From temporal point no doubt, referencing should be contained by referenced table >From other side could be useful if in master table are elements with simple data type, but for some set of elements there could be common properties in another table.. What today is doable on the way to in another table have the same data type and repeat the same properties for each element...That would be possible with Range data type, though it does not mean always data are in range so array is probably better option... However I am not sure from maintaining point of view, i,e when an element should be removed from that common properties set - but it is different topic :) Kind Regards, Misa Sent from my Windows Phone -Original Message- From: Jeff Davis Sent: 17/06/2012 08:55 To: Gabriele Bartolini Cc: PostgreSQL-development; Marco Nenciarini Subject: Re: [HACKERS] [PATCH] Support for foreign keys with arrays -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Fri, 2011-11-04 at 13:48 +0100, Gabriele Bartolini wrote: > This patch adds basic support of arrays in foreign keys, by allowing to > define a referencing column as an array of elements having the same type > as the referenced column in the referenced table. > Every NOT NULL element in the referencing array is matched against the > referenced table. I'm trying to find commonalities between this feature and my future RANGE FOREIGN KEY feature (not past the hand-waving stage yet). The first thing I observe is that my idea for range foreign keys is almost the opposite of your idea for array FKs. I was imagining a range FK to mean that the referencing side is contained by the referenced side. This is the common definition in the temporal world, because the valid period for the referencing row must be within the valid period for the row it references (same for transaction time). The referenced side must be a range, and the referencing side must be either a range of the same type or the subtype of the range. Other similar definitions exist by replacing "contained by" with some other operator, though the use cases for those aren't as clear to me. This definition works for arrays and possibly many other types (geometry?) as well. It looks like this is orthogonal from your work, but it does seem like it has potential for confusion in the future. Thoughts? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On 6 April 2012 07:21, Peter Eisentraut wrote: > On lör, 2012-03-24 at 10:01 +, Gianni Ciolli wrote: >> ON (DELETE | UPDATE) actions for EACH foreign keys >> == >> >> -- --- --- >> | ON | ON | >> Action | DELETE | UPDATE | >> -- --- --- >> CASCADE | Row | Forbidden | >> SET NULL | Row | Row | >> SET DEFAULT | Row | Row | >> EACH CASCADE | Element | Element | >> EACH SET NULL | Element | Element | >> EACH SET DEFAULT | Forbidden | Forbidden | >> NO ACTION | - | - | >> RESTRICT | - | - | >> -- - - >> > I took another fresh look at this feature after not having looked for a > month or two. I think the functionality is probably OK, but I find the > interfaces somewhat poorly named. Consider, "PostgreSQL adds EACH > foreign keys" -- huh? I think they key word ELEMENT would be more > descriptive and precise, and it also leaves the door open to other kind > of non-atomic foreign key relationships outside of arrays. EACH has no > relationship with arrays. It might as well refer to each row. > > On the matter of the above chart, there has been a long back and forth > about whether the row or the element case should be the default. Both > cases are probably useful, but unfortunately you have now settled on > making maximum destruction the default. Additionally, we would now have > the case that sometimes, depending on some configuration elsewhere, an > ON DELETE CASCADE deletes more than what was actually involved in the > foreign key. What I'd suggest is to make both cases explicit. That is, > forbid ON DELETE CASCADE altogether and make people write ON DELETE > CASCADE ROW or ON DELETE CASCADE ELEMENT. In addition to making things > more explicit and safer, it would again leave the door open to other > kinds of relationships later. +1 -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Il giorno mer, 13/06/2012 alle 18.07 -0400, Noah Misch ha scritto: > On Wed, Jun 13, 2012 at 10:12:18PM +0200, Gabriele Bartolini wrote: > > Our goal is to work on this patch from the next commit fest. > > > > What we are about to do for this commit fest is to split the previous > > patch and send a small one just for the array_remove() and > > array_replace() functions. > > > > Then we will sit down and organise the development of the feature > > according to Peter's feedback. It is important indeed that we find a > > commonly accepted terminology and syntax for this feature. > > > > I hope this sounds like a reasonable plan. Thank you. > > Sounds fine. The 2012-01 CF entry for this patch has been moved to the > 2012-06 CF. Please mark that entry Returned with Feedback and create a new > entry for the subset you're repackaging for this CommitFest. > > Thanks, > nm > Dear Noah, I have added a new patch to the commitfest (miscellaneous category) entitled "Support for array_remove and array_replace functions". I have also marked the "Foreign Key Array" patch as " Returned with Feedback" as per your request. We will start working again on this patch in the next commitfest as anticipated by Gabriele, hoping that the array functions will be committed by then. Thank you. Cheers, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Wed, Jun 13, 2012 at 10:12:18PM +0200, Gabriele Bartolini wrote: > Our goal is to work on this patch from the next commit fest. > > What we are about to do for this commit fest is to split the previous > patch and send a small one just for the array_remove() and > array_replace() functions. > > Then we will sit down and organise the development of the feature > according to Peter's feedback. It is important indeed that we find a > commonly accepted terminology and syntax for this feature. > > I hope this sounds like a reasonable plan. Thank you. Sounds fine. The 2012-01 CF entry for this patch has been moved to the 2012-06 CF. Please mark that entry Returned with Feedback and create a new entry for the subset you're repackaging for this CommitFest. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Noah, Il 10/06/12 22:53, Noah Misch ha scritto: This has bitrotted; please refresh. Also, please evaluate Peter's feedback: http://archives.postgresql.org/message-id/1333693277.32606.9.ca...@vanquo.pezone.net Our goal is to work on this patch from the next commit fest. What we are about to do for this commit fest is to split the previous patch and send a small one just for the array_remove() and array_replace() functions. Then we will sit down and organise the development of the feature according to Peter's feedback. It is important indeed that we find a commonly accepted terminology and syntax for this feature. I hope this sounds like a reasonable plan. Thank you. Cheers, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Wed, Mar 28, 2012 at 04:25:06PM +0200, Marco Nenciarini wrote: > Il giorno lun, 19/03/2012 alle 18.41 +0100, Marco Nenciarini ha scritto: > > > > Attached is v5, which should address all the remaining issues. > > Please find attached v6 of the EACH Foreign Key patch. From v5 only > cosmetic changes to the documentation were made. This has bitrotted; please refresh. Also, please evaluate Peter's feedback: http://archives.postgresql.org/message-id/1333693277.32606.9.ca...@vanquo.pezone.net Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Fri, Apr 06, 2012 at 09:21:17AM +0300, Peter Eisentraut wrote: > On l??r, 2012-03-24 at 10:01 +, Gianni Ciolli wrote: > > ON (DELETE | UPDATE) actions for EACH foreign keys > > == > > > > -- --- --- > > |ON |ON | > > Action| DELETE | UPDATE | > > -- --- --- > > CASCADE |Row| Forbidden | > > SET NULL |Row|Row| > > SET DEFAULT |Row|Row| > > EACH CASCADE | Element | Element | > > EACH SET NULL | Element | Element | > > EACH SET DEFAULT | Forbidden | Forbidden | > > NO ACTION | - | - | > > RESTRICT | - | - | > > -- - - > > > I took another fresh look at this feature after not having looked for a > month or two. I think the functionality is probably OK, but I find the > interfaces somewhat poorly named. Consider, "PostgreSQL adds EACH > foreign keys" -- huh? I think they key word ELEMENT would be more > descriptive and precise, and it also leaves the door open to other kind > of non-atomic foreign key relationships outside of arrays. EACH has no > relationship with arrays. It might as well refer to each row. Good points. Your proposed naming works for me. > On the matter of the above chart, there has been a long back and forth > about whether the row or the element case should be the default. Both > cases are probably useful, but unfortunately you have now settled on > making maximum destruction the default. Additionally, we would now have > the case that sometimes, depending on some configuration elsewhere, an > ON DELETE CASCADE deletes more than what was actually involved in the > foreign key. What I'd suggest is to make both cases explicit. That is, > forbid ON DELETE CASCADE altogether and make people write ON DELETE > CASCADE ROW or ON DELETE CASCADE ELEMENT. In addition to making things > more explicit and safer, it would again leave the door open to other > kinds of relationships later. I'm ambivalent on this one. ON DELETE CASCADE truly does the same thing regardless of whether the FK incorporates an EACH column. The current syntax arises from that symmetry rather than a decision to dub its behavior as a default. That said, when a user wants CASCADE ELEMENT, your proposal would more-rapidly divert him from wrongly using CASCADE ROW. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On lör, 2012-03-24 at 10:01 +, Gianni Ciolli wrote: > ON (DELETE | UPDATE) actions for EACH foreign keys > == > > -- --- --- > |ON |ON | > Action| DELETE | UPDATE | > -- --- --- > CASCADE |Row| Forbidden | > SET NULL |Row|Row| > SET DEFAULT |Row|Row| > EACH CASCADE | Element | Element | > EACH SET NULL | Element | Element | > EACH SET DEFAULT | Forbidden | Forbidden | > NO ACTION | - | - | > RESTRICT | - | - | > -- - - > I took another fresh look at this feature after not having looked for a month or two. I think the functionality is probably OK, but I find the interfaces somewhat poorly named. Consider, "PostgreSQL adds EACH foreign keys" -- huh? I think they key word ELEMENT would be more descriptive and precise, and it also leaves the door open to other kind of non-atomic foreign key relationships outside of arrays. EACH has no relationship with arrays. It might as well refer to each row. On the matter of the above chart, there has been a long back and forth about whether the row or the element case should be the default. Both cases are probably useful, but unfortunately you have now settled on making maximum destruction the default. Additionally, we would now have the case that sometimes, depending on some configuration elsewhere, an ON DELETE CASCADE deletes more than what was actually involved in the foreign key. What I'd suggest is to make both cases explicit. That is, forbid ON DELETE CASCADE altogether and make people write ON DELETE CASCADE ROW or ON DELETE CASCADE ELEMENT. In addition to making things more explicit and safer, it would again leave the door open to other kinds of relationships later. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Il giorno lun, 19/03/2012 alle 18.41 +0100, Marco Nenciarini ha scritto: > > Attached is v5, which should address all the remaining issues. Please find attached v6 of the EACH Foreign Key patch. From v5 only cosmetic changes to the documentation were made. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it EACH-foreign-keys.v6.patch.bz2 Description: application/bzip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hello Tom, I started to look at this patch a bit. I'm quite confused by the fact that some, but not all, of the possible FK action types now come in an EACH variant. This makes no sense at all to me. ISTM that EACH is a property of the FK constraint as a whole, that is that it says the constraint is from array elements on the referencing side to column values on the referenced side, rather than the normal case of column values to column values. The specification that Gianni posted applies only to v5 of the patch. The original idea was indeed to have the whole foreign key to be defined with an EACH property (initially we were actually thinking of the ARRAY keyword following your advice, then for grammar reasons we opted for EACH). However, during the actual development we faced some difficulties with multi-column foreign keys. Through discussions on this list and with the reviewer we opted to allow the EACH keyword at column level. We started with the case where at most one column is EACH, which is easier to understand. The case of two or more EACH columns in the same foreign key has been left open for future development. Why would the possible actions be affected, and why only these? We had to add the EACH variant to two actions (EACH CASCADE and EACH SET NULL), in order to leave users the flexibility to choose the operation to be performed in case of delete or update of one or more elements from the referenced table. Some users indeed might prefer that, in case a referenced row is deleted, the whole row is deleted (therefore they'd use the standard CASCADE action). Others mights simply require that references to that row is removed from the referencing array (therefore they'd use the variant EACH CASCADE action). The same concept applies for SET NULL (the whole array is set to NULL) and EACH SET NULL (referencing elements are set to NULL). Thank you. Cheers, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Thu, Mar 22, 2012 at 03:02:45PM -0400, Tom Lane wrote: > It's even less clear about what the semantics are in multi-key > cases. Right offhand I would say that multi-key cases are > nonsensical and should be forbidden outright, because there is no > way to figure out which collections of elements of different arrays > should be considered to be a referencing item. Currently multi-column keys with more than one EACH column are unsupported, mainly because it's unclear how they should work (and I agree that they might not work at all). > Could we see a specification of what the referencing semantics are > intended to be, please? You are right, the discussion has never been put together in a single place, as it should have. Please find below an updated version of the specification, which Marco and I put together from the discussion in this list, and taking into account the changes happened in the review phase. Some comments have also been added to explain why some choices have been forbidden. Best regards, Dr. Gianni Ciolli - 2ndQuadrant Italia PostgreSQL Training, Services and Support gianni.cio...@2ndquadrant.it | www.2ndquadrant.it ---8<--8<--8<--8<--8<--8<--8<--8<--8<--- ON (DELETE | UPDATE) actions for EACH foreign keys == -- --- --- |ON |ON | Action| DELETE | UPDATE | -- --- --- CASCADE |Row| Forbidden | SET NULL |Row|Row| SET DEFAULT |Row|Row| EACH CASCADE | Element | Element | EACH SET NULL | Element | Element | EACH SET DEFAULT | Forbidden | Forbidden | NO ACTION | - | - | RESTRICT | - | - | -- - - Example 1. Table C references table B via a (non-array) foreign key. Example 2. The referencing table A is constructed as GROUP BY of table C in Example 1. There is an EACH foreign key on A which references B, representing the same relationship as the foreign key in Example 1. Remark 3. Examples 1 and 2 are related, because they represent the same model; in making choices about a certain action on Example 2 we will considering its relationship with Example 1. Example 4. Assume that the FK in Example 1 has a ON DELETE CASCADE action. Deleting one row on table B will delete all the referencing rows in table A. The state that we get after the DELETE is the same obtained by Example 2 with the ON DELETE EACH CASCADE action after removing the same row. Example 4 suggests to associate the "Element" behaviour to the ON DELETE EACH CASCADE action. The user can choose between two different options for a CASCADE-style action when a referenced row is deleted; both of them have use cases, as the following Example 5 shows. Example 5. If you remove a vertex from a polygon (represented as an array of vertices), you can either destroy the polygon (ON DELETE CASCADE) or transform it into a polygon with less vertices (ON DELETE EACH CASCADE). ON UPDATE SET NULL has its own purpose as a different behaviour than ON UPDATE EACH SET NULL; again, both options are provided to the user, essentially like with ON DELETE CASCADE and ON DELETE EACH CASCADE. ON (UPDATE | DELETE) EACH SET DEFAULT is forbidden, because table A does not carry a default value for an array element. In theory the default value could be retrieved from the referenced table B, but that would be unusual and in any case different from the corresponding case of Example 1 with ON (UPDATE | DELETE) SET DEFAULT. ON UPDATE CASCADE is forbidden because, as far as we can see, the only meaningful action to propagate updated values is ON UPDATE EACH CASCADE. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Marco Nenciarini writes: > Attached is v5, which should address all the remaining issues. I started to look at this patch a bit. I'm quite confused by the fact that some, but not all, of the possible FK action types now come in an EACH variant. This makes no sense at all to me. ISTM that EACH is a property of the FK constraint as a whole, that is that it says the constraint is from array elements on the referencing side to column values on the referenced side, rather than the normal case of column values to column values. Why would the possible actions be affected, and why only these? The patch documentation is extremely unclear about this. It's even less clear about what the semantics are in multi-key cases. Right offhand I would say that multi-key cases are nonsensical and should be forbidden outright, because there is no way to figure out which collections of elements of different arrays should be considered to be a referencing item. Could we see a specification of what the referencing semantics are intended to be, please? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Mon, Mar 19, 2012 at 06:41:39PM +0100, Marco Nenciarini wrote: > Attached is v5, which should address all the remaining issues. Looks clean to me. > On Fri, Mar 16, 2012 at 11:33:12PM -0400, Noah Misch wrote: > > If the cost doesn't exceed O(F log P), where F is the size of the FK table > > and > > P is the size of the PK table, I'm not worried. If it can be O(F^2), we > > would > > have a problem to be documented, if not fixed. > > We have rewritten the old query in a simpler way; now its cost is O(F log P). > Here F must represent the size of the "flattened" table, that is, the total > number of values that must be checked, which seems a reasonable assumption > in any case. Great; I find that approach easier to reason about. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Noah,
thank you again for your thorough review, which is much appreciated.
> I think the patch has reached the stage where a committer can review
> it without wasting much time on things that might change radically.
> So, I'm marking it Ready for Committer. Please still submit an update
> correcting the above; I'm sure your committer will appreciate it.
Attached is v5, which should address all the remaining issues.
Regards,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
On Fri, Mar 16, 2012 at 11:33:12PM -0400, Noah Misch wrote:
> I recommend removing the new block of code in RI_FKey_eachcascade_del() and
> letting array_remove() throw the error. If you find a way to throw a nicer
> error without an extra scan, by all means submit that to a future CF as an
> improvement. I don't think it's important enough to justify cycles at this
> late hour of the current CF.
It makes sense; we have removed the block of code and updated the error
message following your suggestion. Now the error is thrown by array_remove()
itself.
We'll keep an eye on this for further improvements in the future.
> > > pg_constraint.confeach needs documentation.
> >
> > Most of pg_constraint columns, including all the boolean ones, are
> > documented only in the "description" column of
> >
> > http://www.postgresql.org/docs/9.1/static/catalog-pg-constraint.html#AEN85760
> >
> > it seems that confiseach should not be an exception, since it just
> > indicates whether the constraint is of a certain kind or not.
>
> Your patch adds two columns to pg_constraint, confiseach and confeach, but it
> only mentions confiseach in documentation. Just add a similar minimal mention
> of confeach.
Sorry, our mistake; a mention for confeach has now been added, and both
entries have been reordered to reflect the column position in
pg_constraint).
> That is to say, they start with a capital letter and end with a period. Your
> old text was fine apart from the lack of a period. (Your new text also lacks
> a period.)
Fixed, it should be fine now (another misunderstanding on our side, apologies).
> If the cost doesn't exceed O(F log P), where F is the size of the FK table and
> P is the size of the PK table, I'm not worried. If it can be O(F^2), we would
> have a problem to be documented, if not fixed.
We have rewritten the old query in a simpler way; now its cost is O(F log P).
Here F must represent the size of the "flattened" table, that is, the total
number of values that must be checked, which seems a reasonable assumption
in any case.
> Your change to array_replace() made me look at array_remove() again and
> realize that it needs the same treatment. This command yields a segfault:
> SELECT array_remove('{a,null}'::text[], null);
Fixed.
>
> This latest version introduces two calls to get_element_type(), both of which
> should be get_base_element_type().
Fixed.
> Patch "Avoid FK validations for no-rewrite ALTER TABLE ALTER TYPE", committed
> between v3b and v4 of this patch, added code to ATAddForeignKeyConstraint()
> requiring an update in this patch. Run this in the regression DB:
> [local] regression=# alter table fktableforeachfk alter ftest1 type int[];
> ERROR: could not find cast from 1007 to 23
Thanks for pointing it out. We have added a regression test and then fixed the
issue.
>
> RI_PLAN_EACHCASCADE_DEL_DOUPDATE should be RI_PLAN_EACHCASCADE_DEL_DODELETE.
Fixed.
EACH-foreign-key.v5.patch.bz2
Description: application/bzip
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
[used followup EACH-foreign-key.v4b.patch.bz2 for review]
On Wed, Mar 14, 2012 at 07:03:08PM +0100, Marco Nenciarini wrote:
> please find attached v4 of the EACH Foreign Key patch (formerly known
> also as Foreign Key Array).
> On Fri, Feb 24, 2012 at 09:01:35PM -0500, Noah Misch wrote:
> > > We can use multi-dimensional arrays as well as referencing columns. In
> > > that case though, ON DELETE EACH CASCADE will behave like ON DELETE EACH
> > > SET NULL. This is a safe way of implementing the action.
> > > We have some ideas on how to implement this, but we feel it is better to
> > > limit the behaviour for now.
> >
> > This still feels awfully unprincipled to me. How about just throwing an
> > error
> > when we need to remove an element from a multidimensional array?
> > Essentially,
> > have ON DELETE EACH CASCADE downgrade to ON DELETE RESTRICT when it
> > encounters
> > a multidimensional array. That's strictly less code than what you have now,
> > and it keeps our options open. We can always change from error to set-null
> > later, but we can't so readily change from set-null to anything else.
>
> That seems reasonable to me. Implemented and documented.
I like the semantics now. You implemented this by doing two scans per PK
delete, the first to detect multidimensional dependants of the PK row and the
second to fix 1-dimensional dependants. We don't require an index to support
the scan, so this can mean two seq scans. Currently, the only benefit to
doing it this way is a change of error message. Here is the current error
message when we would need to remove a multidimensional array element:
ERROR: update or delete on table "pktableforarray" violates foreign key
constraint "fktableforarray_ftest1_fkey" on table "fktableforarray"
DETAIL: Key (ptest1)=(2) is still referenced from table "fktableforarray".
If I just rip out the first scan, we get the same outcome with a different
error message:
ERROR: removing elements from multidimensional arrays is not supported
CONTEXT: SQL statement "UPDATE ONLY "public"."fktableforarray" SET "ftest1"
= array_remove("ftest1", $1) WHERE $1 OPERATOR(pg_catalog.=) ANY ("ftest1")"
That has less polish, but I think it's actually more useful. The first
message gives no indication that a multidimensional array foiled your ON
DELETE EACH CASCADE. The second message hints at that cause.
I recommend removing the new block of code in RI_FKey_eachcascade_del() and
letting array_remove() throw the error. If you find a way to throw a nicer
error without an extra scan, by all means submit that to a future CF as an
improvement. I don't think it's important enough to justify cycles at this
late hour of the current CF.
> > As I pondered this patch again, I came upon formal hazards around
> > non-default
> > operator classes. Today, ATAddForeignKeyConstraint() always chooses pfeqop,
> > ffeqop and ppeqop in the same operator family. If it neglected this, we
> > would
> > get wrong behavior when one of the operators is sensitive to a change to
> > which
> > another is insensitive. For EACH FK constraints, this patch sets ffeqop =
> > ARRAY_EQ_OP unconditionally. array_eq() uses the TYPECACHE_EQ_OPR (usually
> > from the default B-tree operator class). That operator may not exist at
> > all,
> > let alone share an operator family with pfeqop. Shall we deal with this by
> > retrieving TYPECACHE_EQ_OPR in ATAddForeignKeyConstraint() and rejecting the
> > constraint creation if it does not share an operator family with pfeqop?
> > The
> > same hazard affects ri_triggers.c use of array_remove() and array_replace(),
> > and the same change mitigates it.
>
> Check added. As a consequence of this stricter policy, certain
> previously allowed cases are now unacceptable (e.g pk FLOAT and fk
> INT[]).
> Regression tests have been added.
Ah, good. Not so formal after all.
> > pg_constraint.confeach needs documentation.
>
> Most of pg_constraint columns, including all the boolean ones, are
> documented only in the "description" column of
>
> http://www.postgresql.org/docs/9.1/static/catalog-pg-constraint.html#AEN85760
>
> it seems that confiseach should not be an exception, since it just
> indicates whether the constraint is of a certain kind or not.
Your patch adds two columns to pg_constraint, confiseach and confeach, but it
only mentions confiseach in documentation. Just add a similar minimal mention
of confeach.
> > > ***
> > > *** 5736,5741 ATAddForeignKeyConstraint(AlteredTableInfo *tab,
> > > Relation rel,
> > > --- 5759,5807
> > > (errcode(ERRCODE_INVALID_FOREIGN_KEY),
> > >errmsg("number of referencing and
> > > referenced columns for foreign key disagree")));
> > >
> > > + /* Enforce each foreign key restrictions */
> > > + if (fkconstraint->fk_is_each)
> > > + {
> > > + /*
> > > +
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hello, Il 15/03/12 05:03, Marco Nenciarini ha scritto: please find attached v4 of the EACH Foreign Key patch (formerly known also as Foreign Key Array). Please find attached version v4b which replaces v4 and fixes a bug in array_replace() and adds further regression tests on array_replace() and fixes a few typos in the documentation. Thank you, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it EACH-foreign-key.v4b.patch.bz2 Description: BZip2 compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Il giorno gio, 08/03/2012 alle 08.11 -0500, Robert Haas ha scritto: > On Fri, Feb 24, 2012 at 9:01 PM, Noah Misch wrote: > > I consider these the core changes needed to reach Ready for Committer: > > > > - Fix crash in array_replace(arr, null, null). > > - Don't look through the domain before calling can_coerce_type(). > > - Compare operator family of pfeqop and TYPECACHE_EQ_OPR at creation. > > - Move post-processing from gram.y and remove "t"/"f" magic values. > > So, is someone working on making these changes? > We are working on it and I hope we can send the v4 version during the upcoming weekend. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Fri, Feb 24, 2012 at 9:01 PM, Noah Misch wrote: > I consider these the core changes needed to reach Ready for Committer: > > - Fix crash in array_replace(arr, null, null). > - Don't look through the domain before calling can_coerce_type(). > - Compare operator family of pfeqop and TYPECACHE_EQ_OPR at creation. > - Move post-processing from gram.y and remove "t"/"f" magic values. So, is someone working on making these changes? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Marco, This version fixes everything I noted in my last review. Apart from corner cases I note, the patch is technically sound. I pose a few policy-type questions; comments from a broader audience would help those. On Mon, Feb 06, 2012 at 07:04:42PM +0100, Marco Nenciarini wrote: > Please find attached version 3 of our patch. We thoroughly followed your > suggestions and were able to implement "EACH foreign key constraints" > with multi-column syntax as well. [actual review based on v3b] > * support for EACH foreign keys in multi-column foreign key table > constraints > - e.g. FOREIGN KEY (c1, EACH c2) REFERENCES t1 (u1, u2) You support, and have test cases for, constraints with multiple EACH columns. The documentation and comments do not explain their semantics. On reviewing the behavior and code, you have implemented it in terms of a Cartesian product. That's logical, but when is such a constraint actually useful? If someone can think of an application, great; let's document his example. Otherwise, let's reject such constraints at DDL time. > As previously said, we preferred to keep this patch simple for 9.2 and > forbid EACH CASCADE and EACH SET NULL on multi-column foreign keys. > After all, majority of use cases is represented by EACH foreign key > column constraints (single-column foreign key arrays), and more > complicated use cases can be discussed for 9.3 - should this patch make > it. :) Good call. > We can use multi-dimensional arrays as well as referencing columns. In > that case though, ON DELETE EACH CASCADE will behave like ON DELETE EACH > SET NULL. This is a safe way of implementing the action. > We have some ideas on how to implement this, but we feel it is better to > limit the behaviour for now. This still feels awfully unprincipled to me. How about just throwing an error when we need to remove an element from a multidimensional array? Essentially, have ON DELETE EACH CASCADE downgrade to ON DELETE RESTRICT when it encounters a multidimensional array. That's strictly less code than what you have now, and it keeps our options open. We can always change from error to set-null later, but we can't so readily change from set-null to anything else. As I pondered this patch again, I came upon formal hazards around non-default operator classes. Today, ATAddForeignKeyConstraint() always chooses pfeqop, ffeqop and ppeqop in the same operator family. If it neglected this, we would get wrong behavior when one of the operators is sensitive to a change to which another is insensitive. For EACH FK constraints, this patch sets ffeqop = ARRAY_EQ_OP unconditionally. array_eq() uses the TYPECACHE_EQ_OPR (usually from the default B-tree operator class). That operator may not exist at all, let alone share an operator family with pfeqop. Shall we deal with this by retrieving TYPECACHE_EQ_OPR in ATAddForeignKeyConstraint() and rejecting the constraint creation if it does not share an operator family with pfeqop? The same hazard affects ri_triggers.c use of array_remove() and array_replace(), and the same change mitigates it. Let me say again how much I like the test coverage from this patch. It's great to think of something worth verifying and find existing coverage for it in the submitted test cases. > *** a/doc/src/sgml/catalogs.sgml > --- b/doc/src/sgml/catalogs.sgml > *** > *** 2102,2107 > --- 2106,2118 > > If a check constraint, a human-readable representation of the > expression > > + > + > + confisarray Name is now confiseach. pg_constraint.confeach needs documentation. > + bool > + > + If a foreign key, true if a EACH REFERENCE foreign key > + > > > > *** a/doc/src/sgml/ddl.sgml > --- b/doc/src/sgml/ddl.sgml > + > + Another option you have with foreign keys is to use a > + referencing column which is an array of elements with > + the same type (or a compatible one) as the referenced > + column in the related table. This feature, commonly known > + as foreign key arrays, is implemented Is it indeed "commonly known" by that name? My web searching did not turn up any independent use of the term. In any event, this should be the only place where we mention multiple names for the feature. Pick one preferred term and use it for all other mentions. The documentation, comments and messages currently have a mix of "each foreign key", "EACH foreign key" and "foreign key array". > + > + Even though the most common use case for foreign key arrays > + is on a single column key, you can define an each foreign > + key constraint on a group of columns. As the following > + contrived example shows, it needs to be written in table constraint > form: > + > + CREATE TABLE t1 ( > + a integer PRIMARY KEY, > + b integer, > + c integer[], > + FOREIGN KEY (b, EACH c) REFERENCES other_table (c1, > c2) > + ); > + Rath
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hello Erik, Il 21/02/12 16:22, Erik Rijkers ha scritto: (I reply to an older message but I did use the newest patch, version 3) I wanted to have a look at v3 of this patch today, but it seems it won't apply and compile anymore. As Peter pointed out, it is due to a new Boolean field added in the pg_proc catalog. I have updated our patch to set this value to false by default. I'd like to try this out a bit; could you see if you can fix it? Thank you so much for dedicating your time on reviewing this patch. I have attached version 3b (noting that is just a very small modification/change to major 3 version of the patch), which passes all tests. Cheers, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it EACH-foreign-key-constraints-aka-foreign-key-arrays.v3b.patch.bz2 Description: BZip2 compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On 21 February 2012 15:22, Erik Rijkers wrote: > (I reply to an older message but I did use the newest patch, version 3) > > I wanted to have a look at v3 of this patch today, but it seems it won't > apply and compile anymore. That's just because a new column has been added to pg_proc - proleakproof. Generalise from the example of commit cd30728fb2ed7c367d545fc14ab850b5fa2a4850 - it modified ever pg_proc entry, and this patch needs the same treatment: http://git.postgresql.org/gitweb/?p=postgresql.git;a=blobdiff;f=src/include/catalog/pg_proc.h;h=fb2923f94db06cd528d588fa52ca3294e025f9f6;hp=d926a88ff8468a43e7d2982273709fbc34058ade;hb=cd30728fb2ed7c367d545fc14ab850b5fa2a4850;hpb=2bbd88f8f841b01efb073972b60d4dc1ff1f6fd0 -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
(I reply to an older message but I did use the newest patch, version 3)
I wanted to have a look at v3 of this patch today, but it seems it won't apply
and compile anymore.
Here are the protestations of patch:
patching file src/include/catalog/pg_proc.h
Hunk #1 FAILED at 868.
Hunk #2 FAILED at 1985.
2 out of 2 hunks FAILED -- saving rejects to file
src/include/catalog/pg_proc.h.rej
and in case it's any use, a cat of src/include/catalog/pg_proc.h.rej:
***
*** 868,873
DATA(insert OID = 2335 ( array_agg PGNSP PGUID 12 1 0 0 0 t f f
f f i 1 0 2277 "2283"
_null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into an array");
DATA(insert OID = 760 ( smgrin PGNSP PGUID 12 1 0 0
0 f f f t f s 1 0 210 "2275" _null_
_null_ _null_ _null_smgrin _null_ _null_ _null_ ));
DESCR("I/O");
DATA(insert OID = 761 ( smgrout PGNSP PGUID 12 1 0 0 0 f f f
t f s 1 0 2275 "210" _null_
_null_ _null_ _null_smgrout _null_ _null_ _null_ ));
--- 868,878
DATA(insert OID = 2335 ( array_agg PGNSP PGUID 12 1 0 0 0 t f f
f f i 1 0 2277 "2283"
_null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into an array");
+ DATA(insert OID = 3157 ( array_remove PGNSP PGUID 12 1 0 0 0 f f f
f f i 2 0 2277 "2277
2283" _null_ _null_ _null_ _null_ array_remove _null_ _null_ _null_ ));
+ DESCR("remove any occurrence of an element from an array");
+ DATA(insert OID = 3158 ( array_replace PGNSP PGUID 12 1 0 0 0 f f f
f f i 3 0 2277 "2277
2283 2283" _null_ _null_ _null_ _null_ array_replace _null_ _null_ _null_ ));
+ DESCR("replace any occurrence of an element in an array");
+
DATA(insert OID = 760 ( smgrin PGNSP PGUID 12 1 0 0
0 f f f t f s 1 0 210 "2275" _null_
_null_ _null_ _null_smgrin _null_ _null_ _null_ ));
DESCR("I/O");
DATA(insert OID = 761 ( smgrout PGNSP PGUID 12 1 0 0 0 f f f
t f s 1 0 2275 "210" _null_
_null_ _null_ _null_smgrout _null_ _null_ _null_ ));
***
*** 1980,1985
DATA(insert OID = 1655 ( RI_FKey_noaction_upd PGNSP PGUID 12 1 0 0 0 f f f t
f v 0 0 2279 ""
_null_ _null_ _null_ _null_ RI_FKey_noaction_upd _null_ _null_ _null_ ));
DESCR("referential integrity ON UPDATE NO ACTION");
DATA(insert OID = 1666 ( varbiteqPGNSP PGUID 12 1 0 0 0
f f f t f i 2 0 16 "1562 1562"
_null_ _null_ _null_ _null_ biteq _null_ _null_ _null_ ));
DATA(insert OID = 1667 ( varbitnePGNSP PGUID 12 1 0 0 0
f f f t f i 2 0 16 "1562 1562"
_null_ _null_ _null_ _null_ bitne _null_ _null_ _null_ ));
DATA(insert OID = 1668 ( varbitgePGNSP PGUID 12 1 0 0 0
f f f t f i 2 0 16 "1562 1562"
_null_ _null_ _null_ _null_ bitge _null_ _null_ _null_ ));
--- 1985,1999
DATA(insert OID = 1655 ( RI_FKey_noaction_upd PGNSP PGUID 12 1 0 0 0 f f f t
f v 0 0 2279 ""
_null_ _null_ _null_ _null_ RI_FKey_noaction_upd _null_ _null_ _null_ ));
DESCR("referential integrity ON UPDATE NO ACTION");
+ DATA(insert OID = 3159 ( RI_FKey_eachcascade_del PGNSP PGUID 12 1 0 0 0
f f f t f v 0 0 2279 ""
_null_ _null_ _null_ _null_ RI_FKey_eachcascade_del _null_ _null_ _null_ ));
+ DESCR("referential integrity ON DELETE EACH CASCADE");
+ DATA(insert OID = 3160 ( RI_FKey_eachcascade_upd PGNSP PGUID 12 1 0 0 0
f f f t f v 0 0 2279 ""
_null_ _null_ _null_ _null_ RI_FKey_eachcascade_upd _null_ _null_ _null_ ));
+ DESCR("referential integrity ON UPDATE EACH CASCADE");
+ DATA(insert OID = 3161 ( RI_FKey_eachsetnull_del PGNSP PGUID 12 1 0 0 0
f f f t f v 0 0 2279 ""
_null_ _null_ _null_ _null_ RI_FKey_eachsetnull_del _null_ _null_ _null_ ));
+ DESCR("referential integrity ON DELETE EACH SET NULL");
+ DATA(insert OID = 3162 ( RI_FKey_eachsetnull_upd PGNSP PGUID 12 1 0 0 0
f f f t f v 0 0 2279 ""
_null_ _null_ _null_ _null_ RI_FKey_eachsetnull_upd _null_ _null_ _null_ ));
+ DESCR("referential integrity ON UPDATE EACH SET NULL");
+
DATA(insert OID = 1666 ( varbiteqPGNSP PGUID 12 1 0 0 0
f f f t f i 2 0 16 "1562 1562"
_null_ _null_ _null_ _null_ biteq _null_ _null_ _null_ ));
DATA(insert OID = 1667 ( varbitnePGNSP PGUID 12 1 0 0 0
f f f t f i 2 0 16 "1562 1562"
_null_ _null_ _null_ _null_ bitne _null_ _null_ _null_ ));
DATA(insert OID = 1668 ( varbitgePGNSP PGUID 12 1 0 0 0
f f f t f i 2 0 16 "1562 1562"
_null_ _null_ _null_ _null_ bitge _null_ _null_ _null_ ));
I'd like to try this out a bit; could you see if you can fix it?
thanks,
Erik Rijkers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi guys, Please find attached version 3 of our patch. We thoroughly followed your suggestions and were able to implement "EACH foreign key constraints" with multi-column syntax as well. "EACH foreign key constraints" represent PostgreSQL implementation of what are also known as Foreign Key Arrays. Some limitations occur in this release, but as previously agreed these can be refined and defined in future release implementations. This patch adds: * support for EACH REFERENCES column constraint on array types - e.g. c1 INT[] EACH REFERENCES t1 * support for EACH foreign key table constraints - e.g. FOREIGN KEY (EACH c1) REFERENCES t1 * support for EACH foreign keys in multi-column foreign key table constraints - e.g. FOREIGN KEY (c1, EACH c2) REFERENCES t1 (u1, u2) * support for two new referential actions on update/delete operations for single-column only EACH foreign keys: ** EACH CASCADE (deletes or updates elements inside the referencing array) ** EACH SET NULL (sets to NULL referencing element inside the foreign array) * support for array_replace and array_remove functions as required by the above actions As previously said, we preferred to keep this patch simple for 9.2 and forbid EACH CASCADE and EACH SET NULL on multi-column foreign keys. After all, majority of use cases is represented by EACH foreign key column constraints (single-column foreign key arrays), and more complicated use cases can be discussed for 9.3 - should this patch make it. :) We can use multi-dimensional arrays as well as referencing columns. In that case though, ON DELETE EACH CASCADE will behave like ON DELETE EACH SET NULL. This is a safe way of implementing the action. We have some ideas on how to implement this, but we feel it is better to limit the behaviour for now. As far as documentation is concerned, we: * added actions and constraint info in the catalog * added an entire section on "EACH foreign key constraints" in the data definition language chapter (we've simplified the second example, greatly following Noah's advice - let us know if this is ok with you) * added array_remove (currently limited to single-dimensional arrays) and array_replace in the array functions chapter * modified REFERENCES/FOREIGN KEY section in the CREATE TABLE command's documentation and added a special section on the EACH REFERENCES clause (using square braces as suggested) Here follows a short list of notes for Noah: * You proposed these changes: ARRAY CASCADE -> EACH CASCADE and ARRAY SET NULL -> EACH SET NULL. We stack with EACH CASCADE and decided to prepend the "EACH" keyword to standard's CASCADE and SET NULL. Grammar is simpler and the emphasis is on the EACH keyword. * Multi-dimensional arrays: ON DELETE EACH CASCADE -> ON DELETE EACH SET NULL. We cannot determine the array's number of dimensions at definition time as it depends on the actual values. As anticipated above, we have some ideas on multi-dimensional element removal, but not for this patch for the aforementioned reasons. * Support of EACH CASCADE/SET NULL in ConvertTriggerToFK(): we decided to leave it. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it EACH-foreign-key-constraints-aka-foreign-key-arrays.v3.patch.bz2 Description: application/bzip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Noah, Il 21/01/12 21:42, Noah Misch ha scritto: On Sat, Jan 14, 2012 at 08:18:48PM +0100, Marco Nenciarini wrote: I greatly like that name; it would still make sense for other aggregate types, should we ever expand its use. Please complete the name change: the documentation, catalog entries, etc should all call them something like "each foreign key constraints" (I don't particularly like that exact wording). Ok, we'll go with "EACH Foreign Key Constraints" but I would allow the synonym "Foreign Key Array", especially in the documentation. How about: FOREIGN KEY(col_a, EACH col_b, col_c) REFERENCES pktable (a, b, c) We really like this syntax. However, as also Simon suggested, we'd go for switching to this syntax, but stick to a simpler implementation for 9.2. We will then be able to expand the functionality, by keeping the same syntax, from 9.3. To complete the ARRAY -> EACH transition, I would suggest names like CASCADE EACH/SET EACH NULL. Sounds perfect. Marco will go through all your comments and will send version 3 shortly. Thank you, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sun, Jan 22, 2012 at 09:06:49PM +, Simon Riggs wrote: > On Sat, Jan 21, 2012 at 8:42 PM, Noah Misch wrote: > > > You currently forbid multi-column EACH FKs. ?I agree that we should allow > > only > > one array column per FK; with more, the set of required PK rows would be > > something like the Cartesian product of the elements of array columns. > > However, there are no definitional problems, at least for NO ACTION, around > > a > > FK constraint having one array column and N scalar columns. ?Whether or not > > you implement that now, let's choose a table_constraint syntax leaving that > > opportunity open. ?How about: > > ? ? ? ?FOREIGN KEY(col_a, EACH col_b, col_c) REFERENCES pktable (a, b, c) > > > I don't think we should be trying to cover every possible combination > of arrays, non-arrays and all the various options. The number of > combinations is making this patch larger than it needs to be and as a > result endangers its being committed in this release just on committer > time to cope with the complexity. We have a matter of weeks to get > this rock solid. > > Yes, lets keep syntax open for future additions, but lets please > focus/edit this down to a solid, useful patch for 9.2. +1. Let's change the syntax to leave that door open but not use the flexibility at this time. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sat, Jan 21, 2012 at 8:42 PM, Noah Misch wrote: > You currently forbid multi-column EACH FKs. I agree that we should allow only > one array column per FK; with more, the set of required PK rows would be > something like the Cartesian product of the elements of array columns. > However, there are no definitional problems, at least for NO ACTION, around a > FK constraint having one array column and N scalar columns. Whether or not > you implement that now, let's choose a table_constraint syntax leaving that > opportunity open. How about: > FOREIGN KEY(col_a, EACH col_b, col_c) REFERENCES pktable (a, b, c) I don't think we should be trying to cover every possible combination of arrays, non-arrays and all the various options. The number of combinations is making this patch larger than it needs to be and as a result endangers its being committed in this release just on committer time to cope with the complexity. We have a matter of weeks to get this rock solid. Yes, lets keep syntax open for future additions, but lets please focus/edit this down to a solid, useful patch for 9.2. For me, one array column, no other non-array columns and delete restrict would cover 90+% of use cases. Bearing in mind you can cover other cases by writing your own triggers, I don't think solving every problem makes sense in a single release. Once we have a solid base we can fill in the rare cases later. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sat, Jan 14, 2012 at 08:18:48PM +0100, Marco Nenciarini wrote:
> This is our latest version of the patch. Gabriele, Gianni and I have
> discussed a lot and decided to send an initial patch which uses EACH
> REFERENCES instead of ARRAY REFERENCES. The reason behind this is that
> ARRAY REFERENCES generates an ambiguous grammar, and we all agreed that
> EACH REFERENCES makes sense (and the same time does not introduce any
> new keyword). This is however open for discussion.
I greatly like that name; it would still make sense for other aggregate types,
should we ever expand its use. Please complete the name change: the
documentation, catalog entries, etc should all call them something like "each
foreign key constraints" (I don't particularly like that exact wording).
You currently forbid multi-column EACH FKs. I agree that we should allow only
one array column per FK; with more, the set of required PK rows would be
something like the Cartesian product of the elements of array columns.
However, there are no definitional problems, at least for NO ACTION, around a
FK constraint having one array column and N scalar columns. Whether or not
you implement that now, let's choose a table_constraint syntax leaving that
opportunity open. How about:
FOREIGN KEY(col_a, EACH col_b, col_c) REFERENCES pktable (a, b, c)
You've identified that we cannot generally implement the ON DELETE ARRAY
CASCADE action on multidimensional arrays. This patch chooses to downgrade to
ON DELETE ARRAY SET NULL in that case. My initial reaction is that it would
be better to forbid multidimensional arrays in the column when the delete
action is ON DELETE ARRAY SET NULL. That's not satisfying, either, because it
makes the definition of conforming data depend on the ON DELETE action. Do we
have other options?
> --- - -
>| ON| ON|
> Action | DELETE | UPDATE |
> --- - -
> CASCADE| Row | Error |
> SET NULL | Row | Row |
> SET DEFAULT| Row | Row |
> ARRAY CASCADE | Element | Element |
> ARRAY SET NULL | Element | Element |
> NO ACTION |-|-|
> RESTRICT |-|-|
> --- - -
To complete the ARRAY -> EACH transition, I would suggest names like CASCADE
EACH/SET EACH NULL.
I like the extensive test cases you have included. There's one more thing
they should do: leave tables having EACH REFERENCES relationships in the
regression database. This way, pg_dump tests of the regression database will
exercise pg_dump with respect to this feature.
The patch emits several warnings:
heap.c: In function `StoreRelCheck':
heap.c:1947: warning: passing argument 17 of `CreateConstraintEntry' makes
integer from pointer without a cast
index.c: In function `index_constraint_create':
index.c:1160: warning: passing argument 17 of `CreateConstraintEntry' makes
integer from pointer without a cast
In file included from gram.y:13051:
scan.c: In function `yy_try_NUL_trans':
scan.c:16243: warning: unused variable `yyg'
trigger.c: In function `CreateTrigger':
trigger.c:454: warning: passing argument 17 of `CreateConstraintEntry' makes
integer from pointer without a cast
typecmds.c: In function `domainAddConstraint':
typecmds.c:2960: warning: passing argument 17 of `CreateConstraintEntry' makes
integer from pointer without a cast
arrayfuncs.c: In function `array_remove':
arrayfuncs.c:5197: warning: unused variable `dimresult'
ri_triggers.c: In function `RI_FKey_check':
ri_triggers.c:484: warning: too many arguments for format
This test case, copied from my previous review except for updating the syntax,
still fails:
BEGIN;
CREATE TABLE parent (c int PRIMARY KEY);
CREATE TABLE child (c int[]);
INSERT INTO parent VALUES (1);
INSERT INTO child VALUES ('{3,1,2}');
ALTER TABLE child ADD FOREIGN KEY (c) EACH REFERENCES parent; -- should error
INSERT INTO child VALUES ('{3,1,2}'); -- does error, as expected
ROLLBACK;
Most of my code comments concern minor matters:
> *** a/doc/src/sgml/ddl.sgml
> --- b/doc/src/sgml/ddl.sgml
> *** CREATE TABLE order_items (
> *** 852,857
> --- 882,931
>
>
>
> + When working with foreign key arrays, you have two more
> + options that can be used with your
> + EACH REFERENCES definition:
> + ARRAY CASCADE and
> + ARRAY SET NULL. Depending on
> + the triggering action (DELETE or
> + UPDATE) on the referenced table,
> + every element in the referencing array will be either
> + deleted/updated or set to NULL.
> + For more detailed information on foreign key arrays
> + options and special cases, please refer to the
> + documentation for .
> +
> +
> +
> + For instance, in the example below, a DELETE
> + from the countries table will remove
> + the referencing elements in the citizenship_ids
> + array.
> +
> +
> + CREATE TABL
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hello, Il giorno dom, 11/12/2011 alle 19.45 -0500, Noah Misch ha scritto: > On Sat, Dec 10, 2011 at 09:47:53AM +0100, Gabriele Bartolini wrote: > > So, here is a summary: > > > > --- - - > >| ON| ON| > > Action | DELETE | UPDATE | > > --- - - > > CASCADE| Row | Error | > > SET NULL | Row | Row | > > SET DEFAULT| Row | Row | > > ARRAY CASCADE | Element | Element | > > ARRAY SET NULL | Element | Element | > > NO ACTION |-|-| > > RESTRICT |-|-| > > --- - - > > > > If that's fine with you guys, Marco and I will refactor the development > > based on these assumptions. > > Looks fine. > This is our latest version of the patch. Gabriele, Gianni and I have discussed a lot and decided to send an initial patch which uses EACH REFERENCES instead of ARRAY REFERENCES. The reason behind this is that ARRAY REFERENCES generates an ambiguous grammar, and we all agreed that EACH REFERENCES makes sense (and the same time does not introduce any new keyword). This is however open for discussion. The patch now includes the following clauses on the delete/update actions - as per previous emails: --- - - | ON| ON| Action | DELETE | UPDATE | --- - - CASCADE| Row | Error | SET NULL | Row | Row | SET DEFAULT| Row | Row | ARRAY CASCADE | Element | Element | ARRAY SET NULL | Element | Element | NO ACTION |-|-| RESTRICT |-|-| --- - - We will resubmit the patch for the 2012-01 commit fest. Thank you, Marco -- Marco Nenciarini - System manager @ Devise.IT marco.nenciar...@devise.it | http://www.devise.it foreign-key-array-v2.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hello, Il giorno dom, 11/12/2011 alle 19.45 -0500, Noah Misch ha scritto: > On Sat, Dec 10, 2011 at 09:47:53AM +0100, Gabriele Bartolini wrote: > > So, here is a summary: > > > > --- - - > >| ON| ON| > > Action | DELETE | UPDATE | > > --- - - > > CASCADE| Row | Error | > > SET NULL | Row | Row | > > SET DEFAULT| Row | Row | > > ARRAY CASCADE | Element | Element | > > ARRAY SET NULL | Element | Element | > > NO ACTION |-|-| > > RESTRICT |-|-| > > --- - - > > > > If that's fine with you guys, Marco and I will refactor the development > > based on these assumptions. > > Looks fine. > This is our latest version of the patch. Gabriele, Gianni and I have discussed a lot and decided to send an initial patch which uses EACH REFERENCES instead of ARRAY REFERENCES. The reason behind this is that ARRAY REFERENCES generates an ambiguous grammar, and we all agreed that EACH REFERENCES makes sense (and the same time does not introduce any new keyword). This is however open for discussion. The patch now includes the following clauses on the delete/update actions - as per previous emails: --- - - | ON| ON| Action | DELETE | UPDATE | --- - - CASCADE| Row | Error | SET NULL | Row | Row | SET DEFAULT| Row | Row | ARRAY CASCADE | Element | Element | ARRAY SET NULL | Element | Element | NO ACTION |-|-| RESTRICT |-|-| --- - - We will resubmit the patch for the 2012-01 commit fest. Thank you, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it foreign-key-array-v2.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Thu, Nov 17, 2011 at 12:08:32AM -0500, Tom Lane wrote: > The least we could do is invent some non-spec syntax that makes the > intention clear, rather than having the system assume that an error case > was intended to mean something else. Maybe > > pids INTEGER[] ARRAY REFERENCES pt, +1. Perhaps this for the table_constraint syntax: FOREIGN KEY (ARRAY foo, bar, ARRAY pids) REFERENCES pt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sat, Dec 10, 2011 at 09:47:53AM +0100, Gabriele Bartolini wrote: > Il 20/11/11 14:05, Noah Misch ha scritto: >> What about making ON UPDATE CASCADE an error? That way, we can say that >> ARRAY >> always applies to array elements, and plain always >> applies to >> entire rows. >> >> SET DEFAULT should now be fine to allow. It's ARRAY SET DEFAULT, in your new >> terminology, that wouldn't make sense. > > I have tried to gather your ideas with Gianni's and come to a > compromise, which I hope you can both agree on. > > The reason why I would be inclined to leave CASCADE act on rows (rather > than array elements as Gianni suggests) is for backward compatibility > (people that are already using referential integrity based on array > values). For the same reason, I am not sure whether we should raise an > error on update, but will leave this for later. Your conclusion is reasonable, but I don't understand this argument for it. The patch does not change the meaning of any SQL that works today. > So, here is a summary: > > --- - - >| ON| ON| > Action | DELETE | UPDATE | > --- - - > CASCADE| Row | Error | > SET NULL | Row | Row | > SET DEFAULT| Row | Row | > ARRAY CASCADE | Element | Element | > ARRAY SET NULL | Element | Element | > NO ACTION |-|-| > RESTRICT |-|-| > --- - - > > If that's fine with you guys, Marco and I will refactor the development > based on these assumptions. Looks fine. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Noah, thanks for your feedback. Il 20/11/11 14:05, Noah Misch ha scritto: What about making ON UPDATE CASCADE an error? That way, we can say that ARRAY always applies to array elements, and plain always applies to entire rows. SET DEFAULT should now be fine to allow. It's ARRAY SET DEFAULT, in your new terminology, that wouldn't make sense. I have tried to gather your ideas with Gianni's and come to a compromise, which I hope you can both agree on. The reason why I would be inclined to leave CASCADE act on rows (rather than array elements as Gianni suggests) is for backward compatibility (people that are already using referential integrity based on array values). For the same reason, I am not sure whether we should raise an error on update, but will leave this for later. So, here is a summary: --- - - | ON| ON| Action | DELETE | UPDATE | --- - - CASCADE| Row | Error | SET NULL | Row | Row | SET DEFAULT| Row | Row | ARRAY CASCADE | Element | Element | ARRAY SET NULL | Element | Element | NO ACTION |-|-| RESTRICT |-|-| --- - - If that's fine with you guys, Marco and I will refactor the development based on these assumptions. Thanks, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On mån, 2011-11-21 at 10:30 -0600, Merlin Moncure wrote: > I like the idea of being able to define more flexible foreign keys, > but are we gilding the lily here? The proposed solution is really > quite specific to the nuances of arrays. Perhaps there is a more > general expression based syntax that leaves the door open for other > types conditions such as dealing fields dependent on other fields? Yeah, basically you'd just need a contains and/or is-contained-by operator between the two types. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Fri, Nov 4, 2011 at 7:48 AM, Gabriele Bartolini wrote: > This patch adds basic support of arrays in foreign keys, by allowing to > define a referencing column as an array of elements having the same type as > the referenced column in the referenced table. > Every NOT NULL element in the referencing array is matched against the > referenced table. I like the idea of being able to define more flexible foreign keys, but are we gilding the lily here? The proposed solution is really quite specific to the nuances of arrays. Perhaps there is a more general expression based syntax that leaves the door open for other types conditions such as dealing fields dependent on other fields? merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Sun, Nov 20, 2011 at 10:36:15AM +0100, Gabriele Bartolini wrote: > I would agree with what Tom is saying here, given that SQL specs do not > say anything about this feature. We could leave standard REFERENCES > keyword handling the array value as it is now. If a user wants to take > advantage of in-array referential integrity, we could implement the > special keyword "ARRAY REFERENCES" as Tom proposes (or a similar > keyword). No objection to that. > --- - - >| ON| ON| > Action | DELETE | UPDATE | > --- - - > CASCADE| Row | Element | > SET NULL | Row | Row | > ARRAY CASCADE | Element | Element | > ARRAY SET NULL | Element | Element | > SET DEFAULT| Error | Error | > NO ACTION |-|-| > RESTRICT |-|-| > --- - - I like this. > CASCADE and ARRAY CASCADE are > synonyms, as they would work in individual elements (which is the action > that makes more sense anyway). What about making ON UPDATE CASCADE an error? That way, we can say that ARRAY always applies to array elements, and plain always applies to entire rows. SET DEFAULT should now be fine to allow. It's ARRAY SET DEFAULT, in your new terminology, that wouldn't make sense. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Gabriele and Marco, On Sun, Nov 20, 2011 at 10:36:15AM +0100, Gabriele Bartolini wrote: > --- - - >| ON| ON| > Action | DELETE | UPDATE | > --- - - > CASCADE| Row | Element | > SET NULL | Row | Row | > ARRAY CASCADE | Element | Element | > ARRAY SET NULL | Element | Element | > SET DEFAULT| Error | Error | > NO ACTION |-|-| > RESTRICT |-|-| > --- - - thank you for this very clear and concise summary! I agree with your appeal for a broad discussion on the proposed syntax, and I will use the same language to express my proposal (for clarity and to simplify the discussion): -- - --- | ON| ON | Action| DELETE | UPDATE| -- - --- CASCADE | Element | Element | SET NULL | Element | Element | SET DEFAULT | Error | Error| ARRAY CASCADE | Row | Element = Row | ARRAY SET NULL| Row | Row | ARRAY SET DEFAULT | Row | Row | NO ACTION |-|- | RESTRICT |-|- | -- - --- I have swapped your syntax in the following way which looks cleaner to me: the ARRAY (CASCADE | SET NULL | SET DEFAULT) syntax denote operations that happen on the whole array, and CASCADE | SET NULL | SET DEFAULT denote instead operations that happen on the elements of the array. Associating the "Element" behaviour with the ON DELETE CASCADE syntax is also consistent with the case where the referencing table A is constructed as GROUP BY of another table B, and the array reference on A is built by aggregating a non-array reference on B with ON DELETE CASCADE syntax. In other words, the same syntax (ON DELETE CASCADE) would denote the same behaviour in both the aggregated case ( = one row per object, using array references) and the non-aggregated case (multiple rows for one object, using equality references), which represent two distinct implementations of the same abstraction. The "Row" behaviour would instead be associated to a new syntax (ON DELETE ARRAY CASCADE), which cannot be obtained via the existing syntax in the non-aggregated implementation, on the grounds that it might be useful for some semantics (for instance: if you remove a vertex from a polygon, you can either destroy the polygon or transform it into a polygon with less vertices). The same principle of considering the two implementations as the same abstraction would also confirm your choice to raise an exception on ON (DELETE | UPDATE) SET DEFAULT. It would also suggest to enable ON (DELETE | UPDATE) ARRAY SET DEFAULT. The reasoning is that we can actually specify a default array in the referencing column, but we can't specify a default element. Before I briefly thought to use the referenced column default as a default for the single element, but it seems a bad idea: a default is an expression (possibly non-constant) which is evaluated only when a new row is created in the referenced table, and using it outside of that context looks inappropriate. Regarding ON UPDATE ARRAY CASCADE, I agree to make it a synonym, since updating the whole array to take into account the update on the referenced field is equivalent to updating the single element to take into account the same fact. Finally, ON UPDATE ARRAY SET NULL would still have an use case as a different behaviour than ON UPDATE SET NULL, which we make available to the database designer: instead of replacing the updated element in the array with a NULL, we replace the whole array with a NULL. This is essentially the same difference that we have between ON DELETE ARRAY CASCADE and ON DELETE CASCADE. Thanks, Dr. Gianni Ciolli - 2ndQuadrant Italia PostgreSQL Training, Services and Support gianni.cio...@2ndquadrant.it | www.2ndquadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Noah, thanks for your unvaluable review, rich of useful and thorough comments and notes. Marco and myself will add your proposed tests as soon as possible (most likely after the Italian PGDay which is this week). However, given the feedback received from other developers too (including Tom), I would first concentrate on defining the syntax and how referential integrity actions should work. Il 17/11/11 05:28, Noah Misch ha scritto: Removing values from the array seems best to me. There's no doubt about what ON UPDATE CASCADE should do, and having ON DELETE CASCADE excise individual array elements is consistent with that. It's less clear for SET NULL, but I'd continue with a per-element treatment. I'd continue to forbid SET DEFAULT. However, Jeff Davis did expect ON DELETE CASCADE to remove entire rows: http://archives.postgresql.org/message-id/1288119207.15279.24.camel@jdavis-ux.asterdata.local So, perhaps the behavior needs to be user-selectable. I would agree with what Tom is saying here, given that SQL specs do not say anything about this feature. We could leave standard REFERENCES keyword handling the array value as it is now. If a user wants to take advantage of in-array referential integrity, we could implement the special keyword "ARRAY REFERENCES" as Tom proposes (or a similar keyword). Consequently, we need to agree on what the actions on delete and update operations are. In case of ARRAY REFERENCES, I would be inclined to leave the same meaning of ROW scope actions to CASCADE and SET NULL actions, while disallowing the SET DEFAULT action (as Noah suggests too). At the same time, I would add two actions for ARRAY REFERENCES which will be processing array elements: * ARRAY CASCADE * ARRAY SET NULL (Of course if you are welcome to propose a better naming convention). This table summarises the scope of the actions. --- - - | ON| ON| Action | DELETE | UPDATE | --- - - CASCADE| Row | Element | SET NULL | Row | Row | ARRAY CASCADE | Element | Element | ARRAY SET NULL | Element | Element | SET DEFAULT| Error | Error | NO ACTION |-|-| RESTRICT |-|-| --- - - For instance, with an "ARRAY REFERENCES ... ON DELETE CASCADE", I would expect that the whole row is deleted (as Jeff et al. say). However, if I specify "ARRAY REFERENCES ... ON DELETE ARRAY CASCADE", I would expect that elements in the referencing array are removed. Similary the "ARRAY REFERENCES ... ON DELETE SET NULL" will set the row to NULL, whereas "ARRAY REFERENCES ... ON DELETE ARRAY SET NULL" will set individual elements in the referencing array to NULL. In case of updates, SET NULL and ARRAY SET NULL works the same (updating the whole row or the single elements). CASCADE and ARRAY CASCADE are synonyms, as they would work in individual elements (which is the action that makes more sense anyway). I believe that, before we proceed with one implementation or another, it is important we discuss this sort of things and agree on a possible long-term path (so that we can organise intermediate deliverables). Thanks, Gabriele -- Gabriele Bartolini - 2ndQuadrant Italia PostgreSQL Training, Services and Support gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Josh Berkus writes: >> BTW, has anyone thought through whether this is a sane idea at all? >> It seems to me to be full of cases that will require rather arbitrary >> decisions, like whether ON DELETE CASCADE should involve deleting the >> whole row or just one array element. > One array element, presumably. Um, why? One reasonable interpretation of an array reference is that the row depends on *all* of the referenced pkeys. Also, if you do delete one array element at a time, what do you do when the array becomes empty --- delete the row, or not, and in each case what's your semantic justification for that choice? In short, "presumably" doesn't cut it here. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
On Wed, Nov 16, 2011 at 11:28 PM, Noah Misch wrote: > > Removing values from the array seems best to me. There's no doubt about what > ON UPDATE CASCADE should do, and having ON DELETE CASCADE excise individual > array elements is consistent with that. It's less clear for SET NULL, but I'd > continue with a per-element treatment. I'd continue to forbid SET DEFAULT. > > However, Jeff Davis did expect ON DELETE CASCADE to remove entire rows: > http://archives.postgresql.org/message-id/1288119207.15279.24.camel@jdavis-ux.asterdata.local > So, perhaps the behavior needs to be user-selectable. > i will agree with Jeff on this... i mean, on the normal case it will delete the row. no? the docs says about the CASCADE action """ CASCADE Delete any rows referencing the deleted row, or update the value of the referencing column to the new value of the referenced column, respectively. """ so, that is what i will expect -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Folks, BTW, I don't want to block this patch. However, it occurs to me that a more generalized FK based on non-equality conditions (i.e. expressions) would be nice if it were possible. Then we could have FKs from all kinds of complex structures. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
2011/11/17 Tom Lane : > Noah Misch writes: >> On Fri, Nov 04, 2011 at 01:48:02PM +0100, Gabriele Bartolini wrote: >>> CREATE TABLE pt ( >>> id INTEGER PRIMARY KEY, >>> >>> CREATE TABLE ft ( >>> id SERIAL PRIMARY KEY, >>> pids INTEGER[] REFERENCES pt, > >> I'm assuming the SQL spec says nothing about a feature like this? > > I'm pretty certain that the SQL spec flat out forbids this. > > The least we could do is invent some non-spec syntax that makes the > intention clear, rather than having the system assume that an error case > was intended to mean something else. Maybe > > pids INTEGER[] ARRAY REFERENCES pt, > > or something like that. (ARRAY is a fully reserved word already, > so I think this syntax should work, but I've not tried it.) +1 Regards Pavel Stehule > > BTW, has anyone thought through whether this is a sane idea at all? > It seems to me to be full of cases that will require rather arbitrary > decisions, like whether ON DELETE CASCADE should involve deleting the > whole row or just one array element. > > regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
> BTW, has anyone thought through whether this is a sane idea at all? > It seems to me to be full of cases that will require rather arbitrary > decisions, like whether ON DELETE CASCADE should involve deleting the > whole row or just one array element. One array element, presumably. Does the patch implement CASCADE? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Noah Misch writes: > On Fri, Nov 04, 2011 at 01:48:02PM +0100, Gabriele Bartolini wrote: >> CREATE TABLE pt ( >> id INTEGER PRIMARY KEY, >> >> CREATE TABLE ft ( >> id SERIAL PRIMARY KEY, >> pids INTEGER[] REFERENCES pt, > I'm assuming the SQL spec says nothing about a feature like this? I'm pretty certain that the SQL spec flat out forbids this. The least we could do is invent some non-spec syntax that makes the intention clear, rather than having the system assume that an error case was intended to mean something else. Maybe pids INTEGER[] ARRAY REFERENCES pt, or something like that. (ARRAY is a fully reserved word already, so I think this syntax should work, but I've not tried it.) BTW, has anyone thought through whether this is a sane idea at all? It seems to me to be full of cases that will require rather arbitrary decisions, like whether ON DELETE CASCADE should involve deleting the whole row or just one array element. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hello
2011/11/17 Noah Misch :
> Hi Gabriele,
>
> On Fri, Nov 04, 2011 at 01:48:02PM +0100, Gabriele Bartolini wrote:
>> CREATE TABLE pt (
>> id INTEGER PRIMARY KEY,
>> ...
>> );
>>
>> CREATE TABLE ft (
>> id SERIAL PRIMARY KEY,
>> pids INTEGER[] REFERENCES pt,
>> ...
>> );
>
> This seems useful.
>
will be supported situation
CREATE TABLE main(
id int[] PRIMARY KEY,
...
);
CREATE TABLE child(
main_id int[] REFERENCES main(id),
??
Regards
Pavel Stehule
> I'm assuming the SQL spec says nothing about a feature like this?
>
>> This patch is for discussion and has been built against HEAD.
>> It compiles and passes all regressions tests (including specific ones -
>> see the src/test/regress/sql/foreign_key.sql file).
>> Empty arrays, multi-dimensional arrays, duplicate elements and NULL
>> values are allowed.
>
> With this patch, RI_Initial_Check does not detect a violation in an array that
> contains at least one conforming element:
>
> BEGIN;
> CREATE TABLE parent (c int PRIMARY KEY);
> CREATE TABLE child (c int[]);
> INSERT INTO parent VALUES (1);
> INSERT INTO child VALUES ('{3,1,2}');
> ALTER TABLE child ADD FOREIGN KEY (c) REFERENCES parent; -- should error
> INSERT INTO child VALUES ('{3,1,2}'); -- does error, as expected
> ROLLBACK;
>
> The error message DETAIL on constraint violation would benefit from
> array-FK-specific language. Example of current message:
>
> ERROR: insert or update on table "child" violates foreign key constraint
> "child_c_fkey"
> DETAIL: Key (c)=({3,1,2}) is not present in table "parent".
>
>
> The patch is missing a change to the code that does FK=FK checks when a user
> updates the FK side:
>
> \set VERBOSITY verbose
> BEGIN;
> CREATE TABLE parent (c int PRIMARY KEY);
> CREATE TABLE child (c int[] REFERENCES parent);
> INSERT INTO parent VALUES (1);
> INSERT INTO child VALUES ('{1,1}');
> COMMIT;
> -- ERROR: XX000: no conversion function from integer[] to integer
> -- LOCATION: ri_HashCompareOp, ri_triggers.c:4097
> UPDATE child SET c = '{1,1}';
> DROP TABLE parent, child;
> COMMIT;
>
> Please audit each ri_triggers.c entry point for further problems like this.
>
>> We had to enforce some limitations, due to the lack (yet) of a clear and
>> universally accepted behaviour and strategy.
>> For example, consider the ON DELETE action on the above tables: in case
>> of delete of a record in the 'pt' table, should we remove the whole row
>> or just the values from the array?
>> We hope we can start a discussion from here.
>
> Removing values from the array seems best to me. There's no doubt about what
> ON UPDATE CASCADE should do, and having ON DELETE CASCADE excise individual
> array elements is consistent with that. It's less clear for SET NULL, but I'd
> continue with a per-element treatment. I'd continue to forbid SET DEFAULT.
>
> However, Jeff Davis did expect ON DELETE CASCADE to remove entire rows:
> http://archives.postgresql.org/message-id/1288119207.15279.24.camel@jdavis-ux.asterdata.local
> So, perhaps the behavior needs to be user-selectable.
>
>> Current limitations:
>>
>> * Only arrays of the same type as the primary key in the referenced
>> table are supported
>
> This is understandable for a WIP, but the final patch will need to use our
> existing, looser foreign key type match requirement.
>
>> * multi-column foreign keys are not supported (only single column)
>
> Any particular reason for this?
>
>> *** a/doc/src/sgml/ddl.sgml
>> --- b/doc/src/sgml/ddl.sgml
>> ***
>> *** 764,769 CREATE TABLE order_items (
>> --- 764,796
>> the last table.
>>
>>
>> +
>> + Another option you have with foreign keys is to use a referencing
>> column
>> + which is an array of elements with the same type as the referenced
>> column
>> + in the related table. This feature, also known as foreign
>> key arrays,
>> + is described in the following example:
>
> Please wrap your documentation paragraphs.
>
>> *** a/src/backend/commands/tablecmds.c
>> --- b/src/backend/commands/tablecmds.c
>> ***
>> *** 5705,5710 ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation
>> rel,
>> --- 5705,5735
>> Oid ffeqop;
>> int16 eqstrategy;
>>
>> + /* Check if foreign key is an array of primary key types */
>> + const bool is_foreign_key_array = (fktype ==
>> get_array_type (pktype));
>
> We don't declare non-pointer, local variables "const". Also, [not positive on
> this one] when an initial assignment requires a comment, declare the variable
> with no assignment and no comment. Then, assign it later with the comment.
> This keeps the per-block declarations packed together.
>
>
> This test wrongly rejects FK types that are domains over the array type:
>
> BEGIN;
> CREATE TABLE parent (c int PRIMARY KEY);
> CREATE DOMAIN intarrdom AS int[];
> CREATE TABLE child (c intarrdom REFERENCES parent);
> R
Re: [HACKERS] [PATCH] Support for foreign keys with arrays
Hi Gabriele,
On Fri, Nov 04, 2011 at 01:48:02PM +0100, Gabriele Bartolini wrote:
> CREATE TABLE pt (
> id INTEGER PRIMARY KEY,
> ...
> );
>
> CREATE TABLE ft (
> id SERIAL PRIMARY KEY,
> pids INTEGER[] REFERENCES pt,
> ...
> );
This seems useful.
I'm assuming the SQL spec says nothing about a feature like this?
> This patch is for discussion and has been built against HEAD.
> It compiles and passes all regressions tests (including specific ones -
> see the src/test/regress/sql/foreign_key.sql file).
> Empty arrays, multi-dimensional arrays, duplicate elements and NULL
> values are allowed.
With this patch, RI_Initial_Check does not detect a violation in an array that
contains at least one conforming element:
BEGIN;
CREATE TABLE parent (c int PRIMARY KEY);
CREATE TABLE child (c int[]);
INSERT INTO parent VALUES (1);
INSERT INTO child VALUES ('{3,1,2}');
ALTER TABLE child ADD FOREIGN KEY (c) REFERENCES parent; -- should error
INSERT INTO child VALUES ('{3,1,2}'); -- does error, as expected
ROLLBACK;
The error message DETAIL on constraint violation would benefit from
array-FK-specific language. Example of current message:
ERROR: insert or update on table "child" violates foreign key constraint
"child_c_fkey"
DETAIL: Key (c)=({3,1,2}) is not present in table "parent".
The patch is missing a change to the code that does FK=FK checks when a user
updates the FK side:
\set VERBOSITY verbose
BEGIN;
CREATE TABLE parent (c int PRIMARY KEY);
CREATE TABLE child (c int[] REFERENCES parent);
INSERT INTO parent VALUES (1);
INSERT INTO child VALUES ('{1,1}');
COMMIT;
-- ERROR: XX000: no conversion function from integer[] to integer
-- LOCATION: ri_HashCompareOp, ri_triggers.c:4097
UPDATE child SET c = '{1,1}';
DROP TABLE parent, child;
COMMIT;
Please audit each ri_triggers.c entry point for further problems like this.
> We had to enforce some limitations, due to the lack (yet) of a clear and
> universally accepted behaviour and strategy.
> For example, consider the ON DELETE action on the above tables: in case
> of delete of a record in the 'pt' table, should we remove the whole row
> or just the values from the array?
> We hope we can start a discussion from here.
Removing values from the array seems best to me. There's no doubt about what
ON UPDATE CASCADE should do, and having ON DELETE CASCADE excise individual
array elements is consistent with that. It's less clear for SET NULL, but I'd
continue with a per-element treatment. I'd continue to forbid SET DEFAULT.
However, Jeff Davis did expect ON DELETE CASCADE to remove entire rows:
http://archives.postgresql.org/message-id/1288119207.15279.24.camel@jdavis-ux.asterdata.local
So, perhaps the behavior needs to be user-selectable.
> Current limitations:
>
> * Only arrays of the same type as the primary key in the referenced
> table are supported
This is understandable for a WIP, but the final patch will need to use our
existing, looser foreign key type match requirement.
> * multi-column foreign keys are not supported (only single column)
Any particular reason for this?
> *** a/doc/src/sgml/ddl.sgml
> --- b/doc/src/sgml/ddl.sgml
> ***
> *** 764,769 CREATE TABLE order_items (
> --- 764,796
> the last table.
>
>
> +
> + Another option you have with foreign keys is to use a referencing column
> + which is an array of elements with the same type as the referenced
> column
> + in the related table. This feature, also known as foreign
> key arrays,
> + is described in the following example:
Please wrap your documentation paragraphs.
> *** a/src/backend/commands/tablecmds.c
> --- b/src/backend/commands/tablecmds.c
> ***
> *** 5705,5710 ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation
> rel,
> --- 5705,5735
> Oid ffeqop;
> int16 eqstrategy;
>
> + /* Check if foreign key is an array of primary key types */
> + const bool is_foreign_key_array = (fktype ==
> get_array_type (pktype));
We don't declare non-pointer, local variables "const". Also, [not positive on
this one] when an initial assignment requires a comment, declare the variable
with no assignment and no comment. Then, assign it later with the comment.
This keeps the per-block declarations packed together.
This test wrongly rejects FK types that are domains over the array type:
BEGIN;
CREATE TABLE parent (c int PRIMARY KEY);
CREATE DOMAIN intarrdom AS int[];
CREATE TABLE child (c intarrdom REFERENCES parent);
ROLLBACK;
> +
> + /* Enforce foreign key array restrictions */
> + if (is_foreign_key_array)
> + {
> + /*
> + * Foreign key array must not be part of a multi-column
> foreign key
> + */
> + if (is_foreign_key_array && numpks > 1)
> +
