Обсуждение: s/UNSPECIFIED/SIMPLE/ in foreign key code?
Our foreign-key-related code uses MATCH_UNSPECIFIED to denote the default foreign key match behavior. This corresponds to the wording used in the SQL92 spec, for instance "If <match type> is not specified or if FULL is specified, ...". But I always found it rather confusing; it sounds like we don't know what match behavior we're supposed to implement. I notice that in SQL99 and later, the SQL committee introduced "MATCH SIMPLE" as a way to name the behavior that formerly had no name. So now they can write things like "If M specifies SIMPLE or FULL, ..." which seems much nicer to me. I think it would be a useful advance in readability if we replaced UNSPECIFIED by SIMPLE throughout the FK code, and barring objections I will go do that. A small flaw in this plan is that in pg_constraint.confmatchtype, MATCH_UNSPECIFIED is stored as 'u'. In a green field I'd just rename that to 's' for SIMPLE, but it seems possible that this would confuse client-side code such as pg_dump or psql. A quick look shows that neither of those programs actually look directly at pg_constraint.confmatchtype, instead relying on backend functions when they want to deconstruct a foreign key constraint. But there could well be other client code that would notice the change. So I'm a bit torn as to whether to change it and create a release-note-worthy compatibility issue, or to leave it as-is (with documentation notes that "u" for MATCH_SIMPLE is a historical accident). Thoughts? regards, tom lane
> A small flaw in this plan is that in pg_constraint.confmatchtype, > MATCH_UNSPECIFIED is stored as 'u'. In a green field I'd just rename > that to 's' for SIMPLE, but it seems possible that this would confuse > client-side code such as pg_dump or psql. A quick look shows that > neither of those programs actually look directly at > pg_constraint.confmatchtype, instead relying on backend functions when > they want to deconstruct a foreign key constraint. But there could well > be other client code that would notice the change. So I'm a bit torn > as to whether to change it and create a release-note-worthy > compatibility issue, or to leave it as-is (with documentation notes that > "u" for MATCH_SIMPLE is a historical accident). As user can also query system tables, so might be some of the application have also used this column's value. However I don't know if any has used. I believe as this is not helping in a big way to adhere to standards, so it is okay to keep "u" for MATCH SIMPLE. > I notice that in SQL99 and later, the SQL committee introduced "MATCH > SIMPLE" as a way to name the behavior that formerly had no name. One of the documents which I referred as SQL-2003 specs says option is NONE. The document which I referred is attached in mail. I am sorry, if this is not the right document or I have mis-interpreted it. I have downloaded SQL-2003 specs by following site. http://en.wikipedia.org/wiki/SQL:2003 -----Original Message----- From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tom Lane Sent: Sunday, June 17, 2012 3:08 AM To: pgsql-hackers@postgreSQL.org Subject: [HACKERS] s/UNSPECIFIED/SIMPLE/ in foreign key code? Our foreign-key-related code uses MATCH_UNSPECIFIED to denote the default foreign key match behavior. This corresponds to the wording used in the SQL92 spec, for instance "If <match type> is not specified or if FULL is specified, ...". But I always found it rather confusing; it sounds like we don't know what match behavior we're supposed to implement. I notice that in SQL99 and later, the SQL committee introduced "MATCH SIMPLE" as a way to name the behavior that formerly had no name. So now they can write things like "If M specifies SIMPLE or FULL, ..." which seems much nicer to me. I think it would be a useful advance in readability if we replaced UNSPECIFIED by SIMPLE throughout the FK code, and barring objections I will go do that. A small flaw in this plan is that in pg_constraint.confmatchtype, MATCH_UNSPECIFIED is stored as 'u'. In a green field I'd just rename that to 's' for SIMPLE, but it seems possible that this would confuse client-side code such as pg_dump or psql. A quick look shows that neither of those programs actually look directly at pg_constraint.confmatchtype, instead relying on backend functions when they want to deconstruct a foreign key constraint. But there could well be other client code that would notice the change. So I'm a bit torn as to whether to change it and create a release-note-worthy compatibility issue, or to leave it as-is (with documentation notes that "u" for MATCH_SIMPLE is a historical accident). Thoughts? 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
On Sat, Jun 16, 2012 at 5:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
For some reason I thought this change would break pg_upgrade, but then I came to me senses and realized pg_upgrade does not migrate/upgrade system tables.
The other candidate to look for possible breakage would be pgAdmin. As Amit says, there might be code out in the wild that does look at this column, so not worth breaking them for this small gain.
A small flaw in this plan is that in pg_constraint.confmatchtype,
MATCH_UNSPECIFIED is stored as 'u'. In a green field I'd just rename
that to 's' for SIMPLE, but it seems possible that this would confuse
client-side code such as pg_dump or psql. A quick look shows that
neither of those programs actually look directly at
pg_constraint.confmatchtype, instead relying on backend functions when
they want to deconstruct a foreign key constraint. But there could well
be other client code that would notice the change. So I'm a bit torn
as to whether to change it and create a release-note-worthy
compatibility issue, or to leave it as-is (with documentation notes that
"u" for MATCH_SIMPLE is a historical accident).
Thoughts?
For some reason I thought this change would break pg_upgrade, but then I came to me senses and realized pg_upgrade does not migrate/upgrade system tables.
The other candidate to look for possible breakage would be pgAdmin. As Amit says, there might be code out in the wild that does look at this column, so not worth breaking them for this small gain.
Regards,
--
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Gurjeet Singh <singh.gurjeet@gmail.com> writes: > The other candidate to look for possible breakage would be pgAdmin. As Amit > says, there might be code out in the wild that does look at this column, so > not worth breaking them for this small gain. Well, I already did it the other way ;-). It's probably not that big a deal either way, since in practice it's a sure thing that 9.3 will have many bigger and more-painful-to-deal-with catalog changes than this one. But the fact that both pg_dump and psql go through pg_get_constraintdef rather than looking directly at the catalogs, and always have (where "always" = "since the pg_constraint catalog was invented, in 7.3") makes me think that other clients probably do likewise. It's rather painful to get the list of FK column names out of pg_constraint's array representation in pure SQL. regards, tom lane