Thread
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix pg_get_constraintdef to cope with NOT VALID constraints
- 048417511aef 9.1.0 cited
-
creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2011-05-31T16:04:07Z
This patch allows you to initially declare a CHECK constraint as NOT VALID, similar to what we already allow for foreign keys. That is, you create the constraint without scanning the table and after it is committed, it is enforced for new rows; later, all rows are checked by running ALTER TABLE VALIDATE CONSTRAINT, which doesn't need AccessExclusive thus allowing for better concurrency. The trickiest bit here was realizing that unlike FKs, check constraints do inherit, and so needed special treatment for recursion. Other than that I think this was pretty straightforward. I intend to attempt to apply this to NOT NULL constraints as well, once the patch to add them to pg_constraint is in. Thoughts? This patch courtesy of Enova Financial. -- Álvaro Herrera <alvherre@alvh.no-ip.org>
-
Re: creating CHECK constraints as NOT VALID
Jaime Casanova <jaime@2ndquadrant.com> — 2011-05-31T16:24:09Z
On Tue, May 31, 2011 at 11:04 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > This patch allows you to initially declare a CHECK constraint as NOT > VALID seems you forgot to add the patch itself -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte y capacitación de PostgreSQL
-
Re: creating CHECK constraints as NOT VALID
Kevin Grittner <kevin.grittner@wicourts.gov> — 2011-05-31T16:35:01Z
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > This patch allows you to initially declare a CHECK constraint as > NOT VALID, similar to what we already allow for foreign keys. > That is, you create the constraint without scanning the table and < after it is committed, it is enforced for new rows; later, all > rows are checked by running ALTER TABLE VALIDATE CONSTRAINT, which > doesn't need AccessExclusive thus allowing for better concurrency. I think it's a valuable feature, not just in terms of timing and concurrency, but in terms of someone starting with less-than-perfect data who wants to prevent further degradation while cleaning up the existing problems. This feature is present in other databases I've used. -Kevin
-
Re: creating CHECK constraints as NOT VALID
Robert Haas <robertmhaas@gmail.com> — 2011-05-31T16:36:26Z
On Tue, May 31, 2011 at 12:04 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > This patch allows you to initially declare a CHECK constraint as NOT > VALID, similar to what we already allow for foreign keys. That is, you > create the constraint without scanning the table and after it is > committed, it is enforced for new rows; later, all rows are checked by > running ALTER TABLE VALIDATE CONSTRAINT, which doesn't need > AccessExclusive thus allowing for better concurrency. > > The trickiest bit here was realizing that unlike FKs, check constraints > do inherit, and so needed special treatment for recursion. Other than > that I think this was pretty straightforward. > > I intend to attempt to apply this to NOT NULL constraints as well, once > the patch to add them to pg_constraint is in. Seems like a logical extension of what we have now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-05-31T16:39:48Z
Excerpts from Jaime Casanova's message of mar may 31 12:24:09 -0400 2011: > On Tue, May 31, 2011 at 11:04 AM, Alvaro Herrera > <alvherre@alvh.no-ip.org> wrote: > > This patch allows you to initially declare a CHECK constraint as NOT > > VALID > > seems you forgot to add the patch itself oops ... another bug in my email client, it seems. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-05-31T16:48:02Z
Excerpts from Alvaro Herrera's message of mar may 31 12:39:48 -0400 2011: > Excerpts from Jaime Casanova's message of mar may 31 12:24:09 -0400 2011: > > On Tue, May 31, 2011 at 11:04 AM, Alvaro Herrera > > <alvherre@alvh.no-ip.org> wrote: > > > This patch allows you to initially declare a CHECK constraint as NOT > > > VALID > > > > seems you forgot to add the patch itself > > oops ... another bug in my email client, it seems. Hmm, found an inconsistency in the way recursion is handled -- other commands have a AT_DoFooRecurse case. Weird. I'll change this to be like that, though I don't readily see why we do it that way. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-05-31T17:43:48Z
Here it is -- as a context patch this time, as well. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Ross Reedstrom <reedstrm@rice.edu> — 2011-05-31T18:02:04Z
On Tue, May 31, 2011 at 11:35:01AM -0500, Kevin Grittner wrote: > Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > > This patch allows you to initially declare a CHECK constraint as > > NOT VALID, similar to what we already allow for foreign keys. > > That is, you create the constraint without scanning the table and > < after it is committed, it is enforced for new rows; later, all > > rows are checked by running ALTER TABLE VALIDATE CONSTRAINT, which > > doesn't need AccessExclusive thus allowing for better concurrency. > > I think it's a valuable feature, not just in terms of timing and > concurrency, but in terms of someone starting with less-than-perfect > data who wants to prevent further degradation while cleaning up the > existing problems. This feature is present in other databases I've > used. Yup, the ER triage approach to data integrity: "Stop the major bleeding, we'll go back and make it a pretty scar later" Follows from one of the practical maxims of databases: "The data is always dirty" Being able to have the constraints enforced at least for new data allows you to at least fence the bad data, and have a shot at fixing it all. Right now, you may be forced into running with constraints effectively 'off', depending on the app to get new data right, while attempting to catch up. And the app probably put the bad data in there in the first place. One of the thankless, important but seemingly never urgent tasks. Ross -- Ross Reedstrom, Ph.D. reedstrm@rice.edu Systems Engineer & Admin, Research Scientist phone: 713-348-6166 Connexions http://cnx.org fax: 713-348-3665 Rice University MS-375, Houston, TX 77005 GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-05-31T20:07:03Z
Excerpts from Ross J. Reedstrom's message of mar may 31 14:02:04 -0400 2011: > Follows from one of the practical maxims of databases: "The data is > always dirty" Being able to have the constraints enforced at least for > new data allows you to at least fence the bad data, and have a shot at > fixing it all. Right now, you may be forced into running with > constraints effectively 'off', depending on the app to get new data > right, while attempting to catch up. And the app probably put the bad > data in there in the first place. One of the thankless, important but > seemingly never urgent tasks. Interesting point of view. I have to admit that I didn't realize I was allowing that, even though I have wished for it in the past myself. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Greg Stark <gsstark@mit.edu> — 2011-05-31T23:03:56Z
On Tue, May 31, 2011 at 1:07 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Ross J. Reedstrom's message of mar may 31 14:02:04 -0400 2011: > >> Follows from one of the practical maxims of databases: "The data is >> always dirty" Being able to have the constraints enforced at least for >> new data allows you to at least fence the bad data, and have a shot at >> fixing it all. > > Interesting point of view. I have to admit that I didn't realize I was > allowing that, even though I have wished for it in the past myself. What happens when there's bad data that the new transaction touches in some minor way? For example updating some other column of the row or just locking the row? What about things like cluster or table rewrites? Also I think NOT NULL might be used in the join elimination patch. Make sure it understands the "valid" flag and doesn't drop joins that aren't needed. It would be nice to have this for unique constraints as well which would *definitely* need to have the planner understand whether they're valid or not. -- greg
-
Re: creating CHECK constraints as NOT VALID
Thom Brown <thom@linux.com> — 2011-06-01T00:18:18Z
On 31 May 2011 18:43, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Here it is -- as a context patch this time, as well. > > -- > Álvaro Herrera <alvherre@commandprompt.com> > The PostgreSQL Company - Command Prompt, Inc. > PostgreSQL Replication, Consulting, Custom Development, 24x7 support There is this scenario: test=# CREATE DOMAIN things AS INT CHECK (VALUE > 5); CREATE DOMAIN test=# CREATE TABLE abc (id SERIAL, stuff things); NOTICE: CREATE TABLE will create implicit sequence "abc_id_seq" for serial column "abc.id" CREATE TABLE test=# INSERT INTO abc (stuff) VALUES (55); INSERT 0 1 test=# ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID; ERROR: column "stuff" of table "abc" contains values that violate the new constraint STATEMENT: ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID; -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Robert Haas <robertmhaas@gmail.com> — 2011-06-01T01:00:59Z
On Tue, May 31, 2011 at 7:03 PM, Greg Stark <gsstark@mit.edu> wrote: > On Tue, May 31, 2011 at 1:07 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: >> Excerpts from Ross J. Reedstrom's message of mar may 31 14:02:04 -0400 2011: >> >>> Follows from one of the practical maxims of databases: "The data is >>> always dirty" Being able to have the constraints enforced at least for >>> new data allows you to at least fence the bad data, and have a shot at >>> fixing it all. >> >> Interesting point of view. I have to admit that I didn't realize I was >> allowing that, even though I have wished for it in the past myself. > > What happens when there's bad data that the new transaction touches in > some minor way? For example updating some other column of the row or > just locking the row? Updating some other column should fail unless the constraint is satisfied for the resulting row, I think. The rule should be simple and easy to understand: old row (versions) aren't checked, but new ones must satisfy all constraints, whether validated or not. There's no question that this feature has a certain amount of foot-gun potential. But it's also really useful. And there are plenty of people who know how to use a gun safely, without shooting themselves in the foot. We shouldn't aim for the lowest common denominator. > What about things like cluster or table > rewrites? > > Also I think NOT NULL might be used in the join elimination patch. > Make sure it understands the "valid" flag and doesn't drop joins that > aren't needed. It would be nice to have this for unique constraints as > well which would *definitely* need to have the planner understand > whether they're valid or not. Yeah. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-01T01:29:52Z
Excerpts from Thom Brown's message of mar may 31 20:18:18 -0400 2011: > On 31 May 2011 18:43, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > > > Here it is -- as a context patch this time, as well. > There is this scenario: > > test=# CREATE DOMAIN things AS INT CHECK (VALUE > 5); > CREATE DOMAIN > test=# CREATE TABLE abc (id SERIAL, stuff things); > NOTICE: CREATE TABLE will create implicit sequence "abc_id_seq" for > serial column "abc.id" > CREATE TABLE > test=# INSERT INTO abc (stuff) VALUES (55); > INSERT 0 1 > test=# ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID; > ERROR: column "stuff" of table "abc" contains values that violate the > new constraint > STATEMENT: ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) > NOT VALID; Oooh, I hadn't realized that I was opening the door for domains and check constraints therein. I'll have a look at this. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
David Fetter <david@fetter.org> — 2011-06-01T01:42:08Z
On Tue, May 31, 2011 at 12:04:07PM -0400, Alvaro Herrera wrote: > This patch allows you to initially declare a CHECK constraint as NOT > VALID, similar to what we already allow for foreign keys. That is, you > create the constraint without scanning the table and after it is > committed, it is enforced for new rows; later, all rows are checked by > running ALTER TABLE VALIDATE CONSTRAINT, which doesn't need > AccessExclusive thus allowing for better concurrency. > > The trickiest bit here was realizing that unlike FKs, check constraints > do inherit, and so needed special treatment for recursion. Other than > that I think this was pretty straightforward. > > I intend to attempt to apply this to NOT NULL constraints as well, once > the patch to add them to pg_constraint is in. > > Thoughts? > > This patch courtesy of Enova Financial. Great stuff! A colleague brought up an interesting idea that I think is worth exploring for all NOT VALID constraints, to wit, is there some way (via SQL) to find which rows violate which constraints? I'm picturing some kind of function that could be aggregated into some structure for each violating row... Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-01T02:55:03Z
Excerpts from David Fetter's message of mar may 31 21:42:08 -0400 2011: > A colleague brought up an interesting idea that I think is worth > exploring for all NOT VALID constraints, to wit, is there some way > (via SQL) to find which rows violate which constraints? I'm picturing > some kind of function that could be aggregated into some structure for > each violating row... Seems like a job for a plpgsql function with a bunch of exception handlers ... Some details like the violated constraint name would be hard to extract, probably, though. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-01T17:53:12Z
Excerpts from Thom Brown's message of mar may 31 20:18:18 -0400 2011: > test=# CREATE DOMAIN things AS INT CHECK (VALUE > 5); > CREATE DOMAIN > test=# CREATE TABLE abc (id SERIAL, stuff things); > NOTICE: CREATE TABLE will create implicit sequence "abc_id_seq" for > serial column "abc.id" > CREATE TABLE > test=# INSERT INTO abc (stuff) VALUES (55); > INSERT 0 1 > test=# ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID; > ERROR: column "stuff" of table "abc" contains values that violate the > new constraint > STATEMENT: ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) > NOT VALID; Okay, fixed that and added ALTER DOMAIN VALIDATE CONSTRAINT too. Thanks for the review. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-01T22:47:26Z
Here's a complete patch with all this stuff, plus doc additions and simple regression tests for the new ALTER DOMAIN commands. Enable CHECK constraints to be declared NOT VALID This means that they can initially be added to a large existing table without checking its initial contents, but new tuples must comply to them; a separate pass invoked by ALTER TABLE / VALIDATE can verify existing data and ensure it complies with the constraint, at which point it is marked validated and becomes a normal part of the table ecosystem. This patch also enables domains to have unvalidated CHECK constraints attached to them as well by way of ALTER DOMAIN / ADD CONSTRAINT / NOT VALID, which can later be validated with ALTER DOMAIN / VALIDATE CONSTRAINT. This patch was sponsored by Enova Financial. doc/src/sgml/catalogs.sgml | 2 +- doc/src/sgml/ref/alter_domain.sgml | 39 +++++- doc/src/sgml/ref/alter_table.sgml | 4 +- src/backend/catalog/heap.c | 13 +- src/backend/commands/tablecmds.c | 227 ++++++++++++++++++++++++----- src/backend/commands/typecmds.c | 140 ++++++++++++++++-- src/backend/parser/gram.y | 22 +++ src/backend/tcop/utility.c | 4 + src/include/catalog/heap.h | 1 + src/include/commands/typecmds.h | 1 + src/include/nodes/parsenodes.h | 3 + src/test/regress/expected/alter_table.out | 36 +++++ src/test/regress/expected/domain.out | 11 ++ src/test/regress/sql/alter_table.sql | 29 ++++ src/test/regress/sql/domain.sql | 10 ++ 15 files changed, 480 insertions(+), 62 deletions(-) -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -
Re: creating CHECK constraints as NOT VALID
Thom Brown <thom@linux.com> — 2011-06-01T23:48:44Z
On 1 June 2011 23:47, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Here's a complete patch with all this stuff, plus doc additions and > simple regression tests for the new ALTER DOMAIN commands. > > Enable CHECK constraints to be declared NOT VALID > > This means that they can initially be added to a large existing table > without checking its initial contents, but new tuples must comply to > them; a separate pass invoked by ALTER TABLE / VALIDATE can verify > existing data and ensure it complies with the constraint, at which point > it is marked validated and becomes a normal part of the table ecosystem. > > This patch also enables domains to have unvalidated CHECK constraints > attached to them as well by way of ALTER DOMAIN / ADD CONSTRAINT / NOT > VALID, which can later be validated with ALTER DOMAIN / VALIDATE > CONSTRAINT. Is this expected? postgres=# CREATE TABLE a (num INT); CREATE TABLE postgres=# INSERT INTO a (num) VALUES (90); INSERT 0 1 postgres=# ALTER TABLE a ADD CONSTRAINT meow CHECK (num < 20) NOT VALID; ALTER TABLE postgres=# CREATE DATABASE test; CREATE DATABASE postgres=# \q postgresql thom$ pg_dump -f /tmp/test.sql postgres postgresql thom$ psql test < /tmp/test.sql SET SET SET SET SET COMMENT CREATE EXTENSION COMMENT SET SET SET CREATE TABLE ALTER TABLE ERROR: new row for relation "a" violates check constraint "meow" CONTEXT: COPY a, line 1: "90" STATEMENT: COPY a (num) FROM stdin; ERROR: new row for relation "a" violates check constraint "meow" CONTEXT: COPY a, line 1: "90" REVOKE REVOKE GRANT GRANT Shouldn't the constraint be dumped as not valid too?? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-02T00:56:12Z
Excerpts from Thom Brown's message of mié jun 01 19:48:44 -0400 2011: > Is this expected? > [ pg_dump fails to preserve not-valid status of constraints ] Certainly not. > Shouldn't the constraint be dumped as not valid too?? Sure, I'll implement that tomorrow. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-02T16:48:42Z
Excerpts from Alvaro Herrera's message of mié jun 01 20:56:12 -0400 2011: > Excerpts from Thom Brown's message of mié jun 01 19:48:44 -0400 2011: > > > Is this expected? > > [ pg_dump fails to preserve not-valid status of constraints ] > > Certainly not. > > > Shouldn't the constraint be dumped as not valid too?? > > Sure, I'll implement that tomorrow. Actually, it turns out that NOT VALID foreign keys were already buggy here, and fixing them automatically fixes this case as well, because the fix involves touching pg_get_constraintdef to dump the flag. This also gets it into psql's \d. Patch attached. (Maybe the changes in psql's describe.c should be reverted, not sure.) -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Thom Brown <thom@linux.com> — 2011-06-03T16:47:58Z
On 2 June 2011 17:48, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Alvaro Herrera's message of mié jun 01 20:56:12 -0400 2011: >> Excerpts from Thom Brown's message of mié jun 01 19:48:44 -0400 2011: >> >> > Is this expected? >> > [ pg_dump fails to preserve not-valid status of constraints ] >> >> Certainly not. >> >> > Shouldn't the constraint be dumped as not valid too?? >> >> Sure, I'll implement that tomorrow. > > Actually, it turns out that NOT VALID foreign keys were already buggy > here, and fixing them automatically fixes this case as well, because the > fix involves touching pg_get_constraintdef to dump the flag. This also > gets it into psql's \d. Patch attached. > > (Maybe the changes in psql's describe.c should be reverted, not sure.) Nice work Alvaro :) Shouldn't patches be sent to -hackers instead of the obsolete -patches list? Plus I'm a bit confused as to why the patch looks like an email instead of a patch. According to the SQL:2011 standard: "The SQL Standard allows you to turn the checking on and off for CHECK constraints, UNIQUE constraints and FOREIGN KEYS." So is it much work to also add the ADD CONSTRAINT UNIQUE (column, ...) NOT VALID syntax to this too? This would mean we're completely covered for this standards requirement. Cheers -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-03T16:58:11Z
Excerpts from Thom Brown's message of vie jun 03 12:47:58 -0400 2011: > On 2 June 2011 17:48, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Actually, it turns out that NOT VALID foreign keys were already buggy > > here, and fixing them automatically fixes this case as well, because the > > fix involves touching pg_get_constraintdef to dump the flag. This also > > gets it into psql's \d. Patch attached. > > > > (Maybe the changes in psql's describe.c should be reverted, not sure.) > > Nice work Alvaro :) Shouldn't patches be sent to -hackers instead of > the obsolete -patches list? Plus I'm a bit confused as to why the > patch looks like an email instead of a patch. Did I really email pgsql-patches? If so, I didn't notice -- but I don't see it (and the archives seem to agree with me, there's no email after 2008-10). The patch looks like an email because that's what git format-patch produced, and I attached it instead of putting it inline. > According to the SQL:2011 standard: "The SQL Standard allows you to > turn the checking on and off for CHECK constraints, UNIQUE constraints > and FOREIGN KEYS." > > So is it much work to also add the ADD CONSTRAINT UNIQUE (column, ...) > NOT VALID syntax to this too? This would mean we're completely > covered for this standards requirement. Yeah, UNIQUE is a completely different beast. There's already some work on making them accept invalid (duplicate) values temporarily, but making that more general, even if it was acceptable to the community at large (which I'm not sure it is) is way beyond what I set to do here :-) -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Thom Brown <thom@linux.com> — 2011-06-03T17:45:57Z
On 3 June 2011 17:58, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Thom Brown's message of vie jun 03 12:47:58 -0400 2011: >> On 2 June 2011 17:48, Alvaro Herrera <alvherre@commandprompt.com> wrote: > >> > Actually, it turns out that NOT VALID foreign keys were already buggy >> > here, and fixing them automatically fixes this case as well, because the >> > fix involves touching pg_get_constraintdef to dump the flag. This also >> > gets it into psql's \d. Patch attached. >> > >> > (Maybe the changes in psql's describe.c should be reverted, not sure.) >> >> Nice work Alvaro :) Shouldn't patches be sent to -hackers instead of >> the obsolete -patches list? Plus I'm a bit confused as to why the >> patch looks like an email instead of a patch. > > Did I really email pgsql-patches? If so, I didn't notice -- but I don't > see it (and the archives seem to agree with me, there's no email after > 2008-10). My bad, I was reading your patch which contained an email subject beginning with [PATCH] (similar to mailing list subject prefixes) which, if I had given it any further though, doesn't mean it's on the -patches list. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-04T01:04:46Z
Excerpts from Thom Brown's message of vie jun 03 13:45:57 -0400 2011: > On 3 June 2011 17:58, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Excerpts from Thom Brown's message of vie jun 03 12:47:58 -0400 2011: > >> Nice work Alvaro :) Shouldn't patches be sent to -hackers instead of > >> the obsolete -patches list? Plus I'm a bit confused as to why the > >> patch looks like an email instead of a patch. > > > > Did I really email pgsql-patches? If so, I didn't notice -- but I don't > > see it (and the archives seem to agree with me, there's no email after > > 2008-10). > > My bad, I was reading your patch which contained an email subject > beginning with [PATCH] (similar to mailing list subject prefixes) > which, if I had given it any further though, doesn't mean it's on the > -patches list. Ah, that makes sense. The pgsql-patches tag was [PATCHES] actually, though :-) -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Simon Riggs <simon@2ndquadrant.com> — 2011-06-04T13:11:52Z
On Thu, Jun 2, 2011 at 5:48 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Alvaro Herrera's message of mié jun 01 20:56:12 -0400 2011: >> Excerpts from Thom Brown's message of mié jun 01 19:48:44 -0400 2011: >> >> > Is this expected? >> > [ pg_dump fails to preserve not-valid status of constraints ] >> >> Certainly not. >> >> > Shouldn't the constraint be dumped as not valid too?? >> >> Sure, I'll implement that tomorrow. > > Actually, it turns out that NOT VALID foreign keys were already buggy > here, and fixing them automatically fixes this case as well, because the > fix involves touching pg_get_constraintdef to dump the flag. This also > gets it into psql's \d. Patch attached. > > (Maybe the changes in psql's describe.c should be reverted, not sure.) Thanks. As soon as Thom said that, I thought "ahh... didn't do that". Patch looks fine. Will you commit this patch to 9.1 now, or would you like me to? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-04T18:21:39Z
Excerpts from Simon Riggs's message of sáb jun 04 09:11:52 -0400 2011: > On Thu, Jun 2, 2011 at 5:48 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > Actually, it turns out that NOT VALID foreign keys were already buggy > > here, and fixing them automatically fixes this case as well, because the > > fix involves touching pg_get_constraintdef to dump the flag. This also > > gets it into psql's \d. Patch attached. > > > > (Maybe the changes in psql's describe.c should be reverted, not sure.) > > Thanks. As soon as Thom said that, I thought "ahh... didn't do that". > > Patch looks fine. Will you commit this patch to 9.1 now, or would you > like me to? Thanks for the review. I already committed it on 9.1: http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=048417511aef8d5fb2d541b17b73afc730935cd5 I'd still like your opinion on the psql bits. Should they be reverted? I haven't verified what the output currently looks like. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Dean Rasheed <dean.a.rasheed@gmail.com> — 2011-06-11T13:32:15Z
On 1 June 2011 23:47, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Here's a complete patch with all this stuff, plus doc additions and > simple regression tests for the new ALTER DOMAIN commands. > > Enable CHECK constraints to be declared NOT VALID > > This means that they can initially be added to a large existing table > without checking its initial contents, but new tuples must comply to > them; a separate pass invoked by ALTER TABLE / VALIDATE can verify > existing data and ensure it complies with the constraint, at which point > it is marked validated and becomes a normal part of the table ecosystem. > I think that you also need to update the constraint exclusion code (get_relation_constraints() or nearby), otherwise the planner might exclude a relation on the basis of a CHECK constraint that is not currently VALID. Regards, Dean
-
Re: creating CHECK constraints as NOT VALID
Thom Brown <thom@linux.com> — 2011-06-11T13:40:59Z
On 11 June 2011 14:32, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > On 1 June 2011 23:47, Alvaro Herrera <alvherre@commandprompt.com> wrote: >> >> Here's a complete patch with all this stuff, plus doc additions and >> simple regression tests for the new ALTER DOMAIN commands. >> >> Enable CHECK constraints to be declared NOT VALID >> >> This means that they can initially be added to a large existing table >> without checking its initial contents, but new tuples must comply to >> them; a separate pass invoked by ALTER TABLE / VALIDATE can verify >> existing data and ensure it complies with the constraint, at which point >> it is marked validated and becomes a normal part of the table ecosystem. >> > > I think that you also need to update the constraint exclusion code > (get_relation_constraints() or nearby), otherwise the planner might > exclude a relation on the basis of a CHECK constraint that is not > currently VALID. Do the standards explicitly stipulate an expected behaviour for this? And does such a problem affect the invalid foreign key change too? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Dean Rasheed <dean.a.rasheed@gmail.com> — 2011-06-11T15:40:54Z
On 11 June 2011 14:40, Thom Brown <thom@linux.com> wrote: > On 11 June 2011 14:32, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: >> On 1 June 2011 23:47, Alvaro Herrera <alvherre@commandprompt.com> wrote: >>> >>> Here's a complete patch with all this stuff, plus doc additions and >>> simple regression tests for the new ALTER DOMAIN commands. >>> >>> Enable CHECK constraints to be declared NOT VALID >>> >>> This means that they can initially be added to a large existing table >>> without checking its initial contents, but new tuples must comply to >>> them; a separate pass invoked by ALTER TABLE / VALIDATE can verify >>> existing data and ensure it complies with the constraint, at which point >>> it is marked validated and becomes a normal part of the table ecosystem. >>> >> >> I think that you also need to update the constraint exclusion code >> (get_relation_constraints() or nearby), otherwise the planner might >> exclude a relation on the basis of a CHECK constraint that is not >> currently VALID. > > Do the standards explicitly stipulate an expected behaviour for this? No I believe that this is a PostgreSQL-specific optimisation, and we need to ensure that queries return the correct results with constraint_exclusion on. > And does such a problem affect the invalid foreign key change too? No only CHECK constraints (and possibly NOT NULL constraints in the future). Regards, Dean
-
Re: creating CHECK constraints as NOT VALID
Dean Rasheed <dean.a.rasheed@gmail.com> — 2011-06-11T16:02:44Z
On 11 June 2011 16:40, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > On 11 June 2011 14:40, Thom Brown <thom@linux.com> wrote: >> On 11 June 2011 14:32, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: >>> On 1 June 2011 23:47, Alvaro Herrera <alvherre@commandprompt.com> wrote: >>>> >>>> Here's a complete patch with all this stuff, plus doc additions and >>>> simple regression tests for the new ALTER DOMAIN commands. >>>> >>>> Enable CHECK constraints to be declared NOT VALID >>>> >>>> This means that they can initially be added to a large existing table >>>> without checking its initial contents, but new tuples must comply to >>>> them; a separate pass invoked by ALTER TABLE / VALIDATE can verify >>>> existing data and ensure it complies with the constraint, at which point >>>> it is marked validated and becomes a normal part of the table ecosystem. >>>> >>> >>> I think that you also need to update the constraint exclusion code >>> (get_relation_constraints() or nearby), otherwise the planner might >>> exclude a relation on the basis of a CHECK constraint that is not >>> currently VALID. >> >> Do the standards explicitly stipulate an expected behaviour for this? > > No I believe that this is a PostgreSQL-specific optimisation, and we > need to ensure that queries return the correct results with > constraint_exclusion on. > >> And does such a problem affect the invalid foreign key change too? > > No only CHECK constraints (and possibly NOT NULL constraints in the future). > > Regards, > Dean > Since you've mentioned the SQL spec, its worth noting that whilst I think that this feature will be very useful, it's not the feature in the SQL spec (at least not in my version). The SQL spec feature is to mark a constraint as NOT ENFORCED, which means that no data (existing or new) is checked against the constraint. It's as if the constraint were not present at all. In Oracle this corresponds to the syntax ALTER TABLE mytable ENABLE/DISABLE myconstraint which is actually quite handy during a bulk load/update - disable all your constraints, do the bulk operation and then re-enable them (automatically re-validating them). This is better than dropping and re-creating the constraints because you don't need to remember all the constraint definitions. I can see both features being quite useful in different situations. Regards, Dean
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-13T22:08:12Z
Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > On 1 June 2011 23:47, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > > > Here's a complete patch with all this stuff, plus doc additions and > > simple regression tests for the new ALTER DOMAIN commands. > > > > Enable CHECK constraints to be declared NOT VALID > > > > This means that they can initially be added to a large existing table > > without checking its initial contents, but new tuples must comply to > > them; a separate pass invoked by ALTER TABLE / VALIDATE can verify > > existing data and ensure it complies with the constraint, at which point > > it is marked validated and becomes a normal part of the table ecosystem. > > > > I think that you also need to update the constraint exclusion code > (get_relation_constraints() or nearby), otherwise the planner might > exclude a relation on the basis of a CHECK constraint that is not > currently VALID. Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix this is quite simple. I don't have it handy right now but I'll post it soon. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Josh Berkus <josh@agliodbs.com> — 2011-06-13T22:11:54Z
Alvaro, Dean, >> I think that you also need to update the constraint exclusion code >> > (get_relation_constraints() or nearby), otherwise the planner might >> > exclude a relation on the basis of a CHECK constraint that is not >> > currently VALID. > Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix > this is quite simple. I don't have it handy right now but I'll post it > soon. Hmmm. Is this the behavior we want with NOT VALID constraints though? I know that if I'm pouring 100m rows into a new partition as part of a repartitioning scheme, I don't want to *ever* check them if I know they're correct because of how I created the table (CREATE TABLE AS ...). -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
-
Re: creating CHECK constraints as NOT VALID
Marko Tiikkaja <marko.tiikkaja@2ndquadrant.com> — 2011-06-13T23:00:34Z
On 14/06/2011 01:11, Josh Berkus wrote: > Hmmm. Is this the behavior we want with NOT VALID constraints though? > > I know that if I'm pouring 100m rows into a new partition as part of a > repartitioning scheme, I don't want to *ever* check them if I know > they're correct because of how I created the table (CREATE TABLE AS ...). I can see why you would want that, but I'd say that's a separate feature you need to explicitly request when creating the constraint. Consider what happens in the "old data is garbage, but I want the new data to be validated" use case if we allow constraint exclusion on NOT VALID constraints. -- Marko Tiikkaja http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
-
Re: creating CHECK constraints as NOT VALID
Josh Berkus <josh@agliodbs.com> — 2011-06-13T23:08:46Z
> I can see why you would want that, but I'd say that's a separate feature > you need to explicitly request when creating the constraint. Consider > what happens in the "old data is garbage, but I want the new data to be > validated" use case if we allow constraint exclusion on NOT VALID > constraints. Yeah, I guess what I'm suggesting is that we should have an ALTER TABLE ... VALID DONT CHECK option. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-14T01:41:07Z
Excerpts from Josh Berkus's message of lun jun 13 18:11:54 -0400 2011: > Alvaro, Dean, > > >> I think that you also need to update the constraint exclusion code > >> > (get_relation_constraints() or nearby), otherwise the planner might > >> > exclude a relation on the basis of a CHECK constraint that is not > >> > currently VALID. > > Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix > > this is quite simple. I don't have it handy right now but I'll post it > > soon. > > Hmmm. Is this the behavior we want with NOT VALID constraints though? > > I know that if I'm pouring 100m rows into a new partition as part of a > repartitioning scheme, I don't want to *ever* check them if I know > they're correct because of how I created the table (CREATE TABLE AS ...). Well, if we don't validate the data, it's an open door for potentially corrupt query results. I'm not really sure that we want to provide support for "I don't ever want to check this data for validity" because of that. But then, I just work here. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Robert Haas <robertmhaas@gmail.com> — 2011-06-14T02:01:28Z
On Mon, Jun 13, 2011 at 9:41 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Josh Berkus's message of lun jun 13 18:11:54 -0400 2011: >> Alvaro, Dean, >> >> >> I think that you also need to update the constraint exclusion code >> >> > (get_relation_constraints() or nearby), otherwise the planner might >> >> > exclude a relation on the basis of a CHECK constraint that is not >> >> > currently VALID. >> > Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >> > this is quite simple. I don't have it handy right now but I'll post it >> > soon. >> >> Hmmm. Is this the behavior we want with NOT VALID constraints though? >> >> I know that if I'm pouring 100m rows into a new partition as part of a >> repartitioning scheme, I don't want to *ever* check them if I know >> they're correct because of how I created the table (CREATE TABLE AS ...). > > Well, if we don't validate the data, it's an open door for potentially > corrupt query results. I'm not really sure that we want to provide > support for "I don't ever want to check this data for validity" because > of that. But then, I just work here. At any rate, we can't very well have two different meanings for NOT VALID, so the 9.2 meaning vis-a-vis CHECK constraints had better match the 9.1 behavior vis-a-vis FOREIGN KEYs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-14T21:14:59Z
Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: > Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > > I think that you also need to update the constraint exclusion code > > (get_relation_constraints() or nearby), otherwise the planner might > > exclude a relation on the basis of a CHECK constraint that is not > > currently VALID. > > Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix > this is quite simple. I don't have it handy right now but I'll post it > soon. Here's the complete patch. *** a/doc/src/sgml/catalogs.sgml --- b/doc/src/sgml/catalogs.sgml *************** *** 1898,1904 **** <entry><structfield>convalidated</structfield></entry> <entry><type>bool</type></entry> <entry></entry> ! <entry>Has the constraint been validated? Can only be false for foreign keys</entry> </row> <row> --- 1898,1904 ---- <entry><structfield>convalidated</structfield></entry> <entry><type>bool</type></entry> <entry></entry> ! <entry>Has the constraint been validated? Can only be false for foreign keys and CHECK constraints</entry> </row> <row> *** a/doc/src/sgml/ref/alter_domain.sgml --- b/doc/src/sgml/ref/alter_domain.sgml *************** *** 28,37 **** ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> { SET | DROP } NOT NULL ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> ! ADD <replaceable class="PARAMETER">domain_constraint</replaceable> ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> DROP CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> [ RESTRICT | CASCADE ] ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> OWNER TO <replaceable class="PARAMETER">new_owner</replaceable> ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> SET SCHEMA <replaceable class="PARAMETER">new_schema</replaceable> --- 28,39 ---- ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> { SET | DROP } NOT NULL ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> ! ADD <replaceable class="PARAMETER">domain_constraint</replaceable> [ NOT VALID ] ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> DROP CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> [ RESTRICT | CASCADE ] ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> + VALIDATE CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> + ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> OWNER TO <replaceable class="PARAMETER">new_owner</replaceable> ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> SET SCHEMA <replaceable class="PARAMETER">new_schema</replaceable> *************** *** 70,82 **** ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> </varlistentry> <varlistentry> ! <term>ADD <replaceable class="PARAMETER">domain_constraint</replaceable></term> <listitem> <para> This form adds a new constraint to a domain using the same syntax as <xref linkend="SQL-CREATEDOMAIN">. ! This will only succeed if all columns using the domain satisfy the ! new constraint. </para> </listitem> </varlistentry> --- 72,88 ---- </varlistentry> <varlistentry> ! <term>ADD <replaceable class="PARAMETER">domain_constraint</replaceable> [ NOT VALID ]</term> <listitem> <para> This form adds a new constraint to a domain using the same syntax as <xref linkend="SQL-CREATEDOMAIN">. ! If NOT VALID is not specified, ! the command will only succeed if all columns using the ! domain satisfy the new constraint. ! The constraint is going to be enforced on new data inserted into columns ! using the domain in all cases, regardless of <literal>NOT VALID</>. ! <literal>NOT VALID</> is only accepted for <literal>CHECK</> constraints. </para> </listitem> </varlistentry> *************** *** 91,96 **** ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> --- 97,113 ---- </varlistentry> <varlistentry> + <term>VALIDATE CONSTRAINT</term> + <listitem> + <para> + This form validates a constraint previously added, that is, verify that + all data in columns using the domain satisfy the specified constraint. + </para> + </listitem> + </varlistentry> + + + <varlistentry> <term>OWNER</term> <listitem> <para> *************** *** 156,161 **** ALTER DOMAIN <replaceable class="PARAMETER">name</replaceable> --- 173,188 ---- </varlistentry> <varlistentry> + <term><replaceable class="PARAMETER">NOT VALID</replaceable></term> + <listitem> + <para> + Do not verify existing column data for constraint validity. + </para> + </listitem> + </varlistentry> + + + <varlistentry> <term><literal>CASCADE</literal></term> <listitem> <para> *************** *** 251,257 **** ALTER DOMAIN zipcode SET SCHEMA customers; <para> <command>ALTER DOMAIN</command> conforms to the <acronym>SQL</acronym> standard, ! except for the <literal>OWNER</> and <literal>SET SCHEMA</> variants, which are <productname>PostgreSQL</productname> extensions. </para> </refsect1> --- 278,286 ---- <para> <command>ALTER DOMAIN</command> conforms to the <acronym>SQL</acronym> standard, ! except for the <literal>OWNER</>, <literal>SET SCHEMA</> and ! <literal>VALIDATE CONSTRAINT</> variants, ! as well as the <literal>NOT VALID</> clause of the <literal>ADD CONSTRAINT</> variant, which are <productname>PostgreSQL</productname> extensions. </para> </refsect1> *** a/doc/src/sgml/ref/alter_table.sgml --- b/doc/src/sgml/ref/alter_table.sgml *************** *** 240,246 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable> <listitem> <para> This form adds a new constraint to a table using the same syntax as ! <xref linkend="SQL-CREATETABLE">. Newly added foreign key constraints can also be defined as <literal>NOT VALID</literal> to avoid the potentially lengthy initial check that must otherwise be performed. Constraint checks are skipped at create table time, so --- 240,246 ---- <listitem> <para> This form adds a new constraint to a table using the same syntax as ! <xref linkend="SQL-CREATETABLE">. Newly added foreign key and CHECK constraints can also be defined as <literal>NOT VALID</literal> to avoid the potentially lengthy initial check that must otherwise be performed. Constraint checks are skipped at create table time, so *************** *** 253,259 **** ALTER TABLE <replaceable class="PARAMETER">name</replaceable> <term><literal>VALIDATE CONSTRAINT</literal></term> <listitem> <para> ! This form validates a foreign key constraint that was previously created as <literal>NOT VALID</literal>. Constraints already marked valid do not cause an error response. </para> --- 253,259 ---- <term><literal>VALIDATE CONSTRAINT</literal></term> <listitem> <para> ! This form validates a foreign key or CHECK constraint that was previously created as <literal>NOT VALID</literal>. Constraints already marked valid do not cause an error response. </para> *** a/src/backend/access/common/tupdesc.c --- b/src/backend/access/common/tupdesc.c *************** *** 200,205 **** CreateTupleDescCopyConstr(TupleDesc tupdesc) --- 200,206 ---- cpy->check[i].ccname = pstrdup(constr->check[i].ccname); if (constr->check[i].ccbin) cpy->check[i].ccbin = pstrdup(constr->check[i].ccbin); + cpy->check[i].ccvalid = constr->check[i].ccvalid; } } *** a/src/backend/catalog/heap.c --- b/src/backend/catalog/heap.c *************** *** 98,104 **** static Oid AddNewRelationType(const char *typeName, Oid new_array_type); static void RelationRemoveInheritance(Oid relid); static void StoreRelCheck(Relation rel, char *ccname, Node *expr, ! bool is_local, int inhcount); static void StoreConstraints(Relation rel, List *cooked_constraints); static bool MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr, bool allow_merge, bool is_local); --- 98,104 ---- Oid new_array_type); static void RelationRemoveInheritance(Oid relid); static void StoreRelCheck(Relation rel, char *ccname, Node *expr, ! bool is_validated, bool is_local, int inhcount); static void StoreConstraints(Relation rel, List *cooked_constraints); static bool MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr, bool allow_merge, bool is_local); *************** *** 1845,1851 **** StoreAttrDefault(Relation rel, AttrNumber attnum, Node *expr) */ static void StoreRelCheck(Relation rel, char *ccname, Node *expr, ! bool is_local, int inhcount) { char *ccbin; char *ccsrc; --- 1845,1851 ---- */ static void StoreRelCheck(Relation rel, char *ccname, Node *expr, ! bool is_validated, bool is_local, int inhcount) { char *ccbin; char *ccsrc; *************** *** 1906,1912 **** StoreRelCheck(Relation rel, char *ccname, Node *expr, CONSTRAINT_CHECK, /* Constraint Type */ false, /* Is Deferrable */ false, /* Is Deferred */ ! true, /* Is Validated */ RelationGetRelid(rel), /* relation */ attNos, /* attrs in the constraint */ keycount, /* # attrs in the constraint */ --- 1906,1912 ---- CONSTRAINT_CHECK, /* Constraint Type */ false, /* Is Deferrable */ false, /* Is Deferred */ ! is_validated, RelationGetRelid(rel), /* relation */ attNos, /* attrs in the constraint */ keycount, /* # attrs in the constraint */ *************** *** 1966,1972 **** StoreConstraints(Relation rel, List *cooked_constraints) StoreAttrDefault(rel, con->attnum, con->expr); break; case CONSTR_CHECK: ! StoreRelCheck(rel, con->name, con->expr, con->is_local, con->inhcount); numchecks++; break; --- 1966,1972 ---- StoreAttrDefault(rel, con->attnum, con->expr); break; case CONSTR_CHECK: ! StoreRelCheck(rel, con->name, con->expr, !con->skip_validation, con->is_local, con->inhcount); numchecks++; break; *************** *** 2080,2085 **** AddRelationNewConstraints(Relation rel, --- 2080,2086 ---- cooked->name = NULL; cooked->attnum = colDef->attnum; cooked->expr = expr; + cooked->skip_validation = false; cooked->is_local = is_local; cooked->inhcount = is_local ? 0 : 1; cookedConstraints = lappend(cookedConstraints, cooked); *************** *** 2193,2199 **** AddRelationNewConstraints(Relation rel, /* * OK, store it. */ ! StoreRelCheck(rel, ccname, expr, is_local, is_local ? 0 : 1); numchecks++; --- 2194,2201 ---- /* * OK, store it. */ ! StoreRelCheck(rel, ccname, expr, !cdef->skip_validation, is_local, ! is_local ? 0 : 1); numchecks++; *************** *** 2202,2207 **** AddRelationNewConstraints(Relation rel, --- 2204,2210 ---- cooked->name = ccname; cooked->attnum = 0; cooked->expr = expr; + cooked->skip_validation = cdef->skip_validation; cooked->is_local = is_local; cooked->inhcount = is_local ? 0 : 1; cookedConstraints = lappend(cookedConstraints, cooked); *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *************** *** 258,264 **** static void AlterIndexNamespaces(Relation classRel, Relation rel, static void AlterSeqNamespaces(Relation classRel, Relation rel, Oid oldNspOid, Oid newNspOid, const char *newNspName, LOCKMODE lockmode); ! static void ATExecValidateConstraint(Relation rel, const char *constrName); static int transformColumnNameList(Oid relId, List *colList, int16 *attnums, Oid *atttypids); static int transformFkeyGetPrimaryKey(Relation pkrel, Oid *indexOid, --- 258,265 ---- static void AlterSeqNamespaces(Relation classRel, Relation rel, Oid oldNspOid, Oid newNspOid, const char *newNspName, LOCKMODE lockmode); ! static void ATExecValidateConstraint(Relation rel, const char *constrName, ! bool recurse, bool recursing, LOCKMODE lockmode); static int transformColumnNameList(Oid relId, List *colList, int16 *attnums, Oid *atttypids); static int transformFkeyGetPrimaryKey(Relation pkrel, Oid *indexOid, *************** *** 269,274 **** static Oid transformFkeyCheckAttrs(Relation pkrel, --- 270,277 ---- int numattrs, int16 *attnums, Oid *opclasses); static void checkFkeyPermissions(Relation rel, int16 *attnums, int natts); + static void validateCheckConstraint(char *conname, Relation rel, + HeapTuple constrtup); static void validateForeignKeyConstraint(char *conname, Relation rel, Relation pkrel, Oid pkindOid, Oid constraintOid); *************** *** 560,565 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId) --- 563,569 ---- cooked->name = NULL; cooked->attnum = attnum; cooked->expr = colDef->cooked_default; + cooked->skip_validation = false; cooked->is_local = true; /* not used for defaults */ cooked->inhcount = 0; /* ditto */ cookedDefaults = lappend(cookedDefaults, cooked); *************** *** 1567,1572 **** MergeAttributes(List *schema, List *supers, char relpersistence, --- 1571,1577 ---- cooked->name = pstrdup(name); cooked->attnum = 0; /* not used for constraints */ cooked->expr = expr; + cooked->skip_validation = false; cooked->is_local = false; cooked->inhcount = 1; constraints = lappend(constraints, cooked); *************** *** 2932,2938 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ATPrepAddInherit(rel); pass = AT_PASS_MISC; break; ! case AT_ValidateConstraint: case AT_EnableTrig: /* ENABLE TRIGGER variants */ case AT_EnableAlwaysTrig: case AT_EnableReplicaTrig: --- 2937,2950 ---- ATPrepAddInherit(rel); pass = AT_PASS_MISC; break; ! case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */ ! ATSimplePermissions(rel, ATT_TABLE); ! /* Recursion occurs during execution phase */ ! /* No command-specific prep needed except saving recurse flag */ ! if (recurse) ! cmd->subtype = AT_ValidateConstraintRecurse; ! pass = AT_PASS_MISC; ! break; case AT_EnableTrig: /* ENABLE TRIGGER variants */ case AT_EnableAlwaysTrig: case AT_EnableReplicaTrig: *************** *** 3097,3104 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ ATExecAddIndexConstraint(tab, rel, (IndexStmt *) cmd->def, lockmode); break; ! case AT_ValidateConstraint: ! ATExecValidateConstraint(rel, cmd->name); break; case AT_DropConstraint: /* DROP CONSTRAINT */ ATExecDropConstraint(rel, cmd->name, cmd->behavior, --- 3109,3120 ---- case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ ATExecAddIndexConstraint(tab, rel, (IndexStmt *) cmd->def, lockmode); break; ! case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */ ! ATExecValidateConstraint(rel, cmd->name, false, false, lockmode); ! break; ! case AT_ValidateConstraintRecurse: /* VALIDATE CONSTRAINT with ! * recursion */ ! ATExecValidateConstraint(rel, cmd->name, true, false, lockmode); break; case AT_DropConstraint: /* DROP CONSTRAINT */ ATExecDropConstraint(rel, cmd->name, cmd->behavior, *************** *** 5382,5400 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, list_make1(copyObject(constr)), recursing, !recursing); ! /* Add each constraint to Phase 3's queue */ foreach(lcon, newcons) { CookedConstraint *ccon = (CookedConstraint *) lfirst(lcon); - NewConstraint *newcon; ! newcon = (NewConstraint *) palloc0(sizeof(NewConstraint)); ! newcon->name = ccon->name; ! newcon->contype = ccon->contype; ! /* ExecQual wants implicit-AND format */ ! newcon->qual = (Node *) make_ands_implicit((Expr *) ccon->expr); ! tab->constraints = lappend(tab->constraints, newcon); /* Save the actually assigned name if it was defaulted */ if (constr->conname == NULL) --- 5398,5420 ---- list_make1(copyObject(constr)), recursing, !recursing); ! /* Add each to-be-validated constraint to Phase 3's queue */ foreach(lcon, newcons) { CookedConstraint *ccon = (CookedConstraint *) lfirst(lcon); ! if (!ccon->skip_validation) ! { ! NewConstraint *newcon; ! newcon = (NewConstraint *) palloc0(sizeof(NewConstraint)); ! newcon->name = ccon->name; ! newcon->contype = ccon->contype; ! /* ExecQual wants implicit-AND format */ ! newcon->qual = (Node *) make_ands_implicit((Expr *) ccon->expr); ! ! tab->constraints = lappend(tab->constraints, newcon); ! } /* Save the actually assigned name if it was defaulted */ if (constr->conname == NULL) *************** *** 5753,5761 **** ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, /* * ALTER TABLE VALIDATE CONSTRAINT */ static void ! ATExecValidateConstraint(Relation rel, const char *constrName) { Relation conrel; SysScanDesc scan; --- 5773,5787 ---- /* * ALTER TABLE VALIDATE CONSTRAINT + * + * XXX The reason we handle recursion here rather than at Phase 1 is because + * there's no good way to skip recursing when handling foreign keys: there is + * no need to lock children in that case, yet we wouldn't be able to avoid + * doing so at that level. */ static void ! ATExecValidateConstraint(Relation rel, const char *constrName, bool recurse, ! bool recursing, LOCKMODE lockmode) { Relation conrel; SysScanDesc scan; *************** *** 5779,5786 **** ATExecValidateConstraint(Relation rel, const char *constrName) while (HeapTupleIsValid(tuple = systable_getnext(scan))) { con = (Form_pg_constraint) GETSTRUCT(tuple); ! if (con->contype == CONSTRAINT_FOREIGN && ! strcmp(NameStr(con->conname), constrName) == 0) { found = true; break; --- 5805,5811 ---- while (HeapTupleIsValid(tuple = systable_getnext(scan))) { con = (Form_pg_constraint) GETSTRUCT(tuple); ! if (strcmp(NameStr(con->conname), constrName) == 0) { found = true; break; *************** *** 5790,5828 **** ATExecValidateConstraint(Relation rel, const char *constrName) if (!found) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), ! errmsg("foreign key constraint \"%s\" of relation \"%s\" does not exist", constrName, RelationGetRelationName(rel)))); if (!con->convalidated) { ! Oid conid = HeapTupleGetOid(tuple); ! HeapTuple copyTuple = heap_copytuple(tuple); ! Form_pg_constraint copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple); ! Relation refrel; ! /* ! * Triggers are already in place on both tables, so a concurrent write ! * that alters the result here is not possible. Normally we can run a ! * query here to do the validation, which would only require ! * AccessShareLock. In some cases, it is possible that we might need ! * to fire triggers to perform the check, so we take a lock at ! * RowShareLock level just in case. ! */ ! refrel = heap_open(con->confrelid, RowShareLock); ! validateForeignKeyConstraint((char *) constrName, rel, refrel, ! con->conindid, ! conid); /* * Now update the catalog, while we have the door open. */ copy_con->convalidated = true; simple_heap_update(conrel, ©Tuple->t_self, copyTuple); CatalogUpdateIndexes(conrel, copyTuple); heap_freetuple(copyTuple); - - heap_close(refrel, NoLock); } systable_endscan(scan); --- 5815,5918 ---- if (!found) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), ! errmsg("constraint \"%s\" of relation \"%s\" does not exist", ! constrName, RelationGetRelationName(rel)))); ! ! if (con->contype != CONSTRAINT_FOREIGN && ! con->contype != CONSTRAINT_CHECK) ! ereport(ERROR, ! (errcode(ERRCODE_WRONG_OBJECT_TYPE), ! errmsg("constraint \"%s\" of relation \"%s\" is not a foreign key or check constraint", constrName, RelationGetRelationName(rel)))); if (!con->convalidated) { ! HeapTuple copyTuple; ! Form_pg_constraint copy_con; ! if (con->contype == CONSTRAINT_FOREIGN) ! { ! Oid conid = HeapTupleGetOid(tuple); ! Relation refrel; ! /* ! * Triggers are already in place on both tables, so a concurrent write ! * that alters the result here is not possible. Normally we can run a ! * query here to do the validation, which would only require ! * AccessShareLock. In some cases, it is possible that we might need ! * to fire triggers to perform the check, so we take a lock at ! * RowShareLock level just in case. ! */ ! refrel = heap_open(con->confrelid, RowShareLock); ! ! validateForeignKeyConstraint((char *) constrName, rel, refrel, ! con->conindid, ! conid); ! heap_close(refrel, NoLock); ! ! /* ! * Foreign keys do not inherit, so we purposedly ignore the ! * recursion bit here ! */ ! } ! else if (con->contype == CONSTRAINT_CHECK) ! { ! List *children = NIL; ! ListCell *child; ! ! /* ! * If we're recursing, the parent has already done this, so skip ! * it. ! */ ! if (!recursing) ! children = find_all_inheritors(RelationGetRelid(rel), ! lockmode, NULL); ! ! /* ! * For CHECK constraints, we must ensure that we only mark the ! * constraint as validated on the parent if it's already validated ! * on the children. ! * ! * We recurse before validating on the parent, to reduce risk of ! * deadlocks. ! */ ! foreach(child, children) ! { ! Oid childoid = lfirst_oid(child); ! Relation childrel; ! ! if (childoid == RelationGetRelid(rel)) ! continue; ! ! /* ! * If we are told not to recurse, there had better not be any ! * child tables; else the addition would put them out of step. ! */ ! if (!recurse) ! ereport(ERROR, ! (errcode(ERRCODE_INVALID_TABLE_DEFINITION), ! errmsg("constraint must be validated on child tables too"))); ! ! /* find_all_inheritors already got lock */ ! childrel = heap_open(childoid, NoLock); ! ! ATExecValidateConstraint(childrel, constrName, false, ! true, lockmode); ! heap_close(childrel, NoLock); ! } ! ! validateCheckConstraint((char *) constrName, rel, tuple); ! } /* * Now update the catalog, while we have the door open. */ + copyTuple = heap_copytuple(tuple); + copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple); copy_con->convalidated = true; simple_heap_update(conrel, ©Tuple->t_self, copyTuple); CatalogUpdateIndexes(conrel, copyTuple); heap_freetuple(copyTuple); } systable_endscan(scan); *************** *** 6128,6133 **** checkFkeyPermissions(Relation rel, int16 *attnums, int natts) --- 6218,6292 ---- } /* + * Scan the existing rows in a table to verify they meet a proposed + * CHECK constraint. + * + * The caller must have opened and locked the relation appropriately. + */ + static void + validateCheckConstraint(char *conname, Relation rel, HeapTuple constrtup) + { + EState *estate; + Datum val; + char *conbin; + Expr *origexpr; + List *exprstate; + TupleDesc tupdesc; + HeapScanDesc scan; + HeapTuple tuple; + ExprContext *econtext; + MemoryContext oldcxt; + TupleTableSlot *slot; + bool isnull; + + estate = CreateExecutorState(); + /* + * XXX this tuple doesn't really come from a syscache, but this doesn't + * matter to SysCacheGetAttr, because it only wants to be able to fetch the + * tupdesc + */ + val = SysCacheGetAttr(CONSTROID, constrtup, Anum_pg_constraint_conbin, + &isnull); + if (isnull) + elog(ERROR, "null conbin for constraint %u", + HeapTupleGetOid(constrtup)); + conbin = TextDatumGetCString(val); + origexpr = (Expr *) stringToNode(conbin); + exprstate = (List *) ExecPrepareExpr((Expr *) make_ands_implicit((Expr *) origexpr), estate); + + econtext = GetPerTupleExprContext(estate); + tupdesc = RelationGetDescr(rel); + slot = MakeSingleTupleTableSlot(tupdesc); + econtext->ecxt_scantuple = slot; + + scan = heap_beginscan(rel, SnapshotNow, 0, NULL); + + /* + * Switch to per-tuple memory context and reset it for each tuple + * produced, so we don't leak memory. + */ + oldcxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); + + while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) + { + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + + if (!ExecQual(exprstate, econtext, true)) + ereport(ERROR, + (errcode(ERRCODE_CHECK_VIOLATION), + errmsg("check constraint \"%s\" is violated by some row", + conname))); + + ResetExprContext(econtext); + } + + MemoryContextSwitchTo(oldcxt); + heap_endscan(scan); + ExecDropSingleTupleTableSlot(slot); + FreeExecutorState(estate); + } + + /* * Scan the existing rows in a table to verify they meet a proposed FK * constraint. * *** a/src/backend/commands/typecmds.c --- b/src/backend/commands/typecmds.c *************** *** 86,91 **** static Oid findTypeSendFunction(List *procname, Oid typeOid); --- 86,92 ---- static Oid findTypeTypmodinFunction(List *procname); static Oid findTypeTypmodoutFunction(List *procname); static Oid findTypeAnalyzeFunction(List *procname, Oid typeOid); + static void validateDomainConstraint(Oid domainoid, char *ccbin); static List *get_rels_with_domain(Oid domainOid, LOCKMODE lockmode); static void checkDomainOwner(HeapTuple tup); static void checkEnumOwner(HeapTuple tup); *************** *** 1941,1954 **** AlterDomainAddConstraint(List *names, Node *newConstraint) Relation typrel; HeapTuple tup; Form_pg_type typTup; - List *rels; - ListCell *rt; - EState *estate; - ExprContext *econtext; - char *ccbin; - Expr *expr; - ExprState *exprstate; Constraint *constr; /* Make a TypeName so we can use standard type lookup machinery */ typename = makeTypeNameFromNameList(names); --- 1942,1949 ---- Relation typrel; HeapTuple tup; Form_pg_type typTup; Constraint *constr; + char *ccbin; /* Make a TypeName so we can use standard type lookup machinery */ typename = makeTypeNameFromNameList(names); *************** *** 2027,2036 **** AlterDomainAddConstraint(List *names, Node *newConstraint) constr, NameStr(typTup->typname)); /* ! * Test all values stored in the attributes based on the domain the ! * constraint is being added to. */ ! expr = (Expr *) stringToNode(ccbin); /* Need an EState to run ExecEvalExpr */ estate = CreateExecutorState(); --- 2022,2150 ---- constr, NameStr(typTup->typname)); /* ! * If requested to validate the constraint, test all values stored in the ! * attributes based on the domain the constraint is being added to. */ ! if (!constr->skip_validation) ! validateDomainConstraint(domainoid, ccbin); ! ! /* Clean up */ ! heap_close(typrel, RowExclusiveLock); ! } ! ! /* ! * AlterDomainValidateConstraint ! * ! * Implements the ALTER DOMAIN .. VALIDATE CONSTRAINT statement. ! */ ! void ! AlterDomainValidateConstraint(List *names, char *constrName) ! { ! TypeName *typename; ! Oid domainoid; ! Relation typrel; ! Relation conrel; ! HeapTuple tup; ! Form_pg_type typTup; ! Form_pg_constraint con; ! Form_pg_constraint copy_con; ! char *conbin; ! SysScanDesc scan; ! Datum val; ! bool found = false; ! bool isnull; ! HeapTuple tuple; ! HeapTuple copyTuple; ! ScanKeyData key; ! ! /* Make a TypeName so we can use standard type lookup machinery */ ! typename = makeTypeNameFromNameList(names); ! domainoid = typenameTypeId(NULL, typename); ! ! /* Look up the domain in the type table */ ! typrel = heap_open(TypeRelationId, AccessShareLock); ! ! tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(domainoid)); ! if (!HeapTupleIsValid(tup)) ! elog(ERROR, "cache lookup failed for type %u", domainoid); ! typTup = (Form_pg_type) GETSTRUCT(tup); ! ! /* Check it's a domain and check user has permission for ALTER DOMAIN */ ! checkDomainOwner(tup); ! ! /* ! * Find and check the target constraint ! */ ! conrel = heap_open(ConstraintRelationId, RowExclusiveLock); ! ScanKeyInit(&key, ! Anum_pg_constraint_contypid, ! BTEqualStrategyNumber, F_OIDEQ, ! ObjectIdGetDatum(domainoid)); ! scan = systable_beginscan(conrel, ConstraintTypidIndexId, ! true, SnapshotNow, 1, &key); ! ! while (HeapTupleIsValid(tuple = systable_getnext(scan))) ! { ! con = (Form_pg_constraint) GETSTRUCT(tuple); ! if (strcmp(NameStr(con->conname), constrName) == 0) ! { ! found = true; ! break; ! } ! } ! ! if (!found) ! { ! con = NULL; /* keep compiler quiet */ ! ereport(ERROR, ! (errcode(ERRCODE_UNDEFINED_OBJECT), ! errmsg("constraint \"%s\" of domain \"%s\" does not exist", ! constrName, NameStr(con->conname)))); ! } ! ! if (con->contype != CONSTRAINT_CHECK) ! ereport(ERROR, ! (errcode(ERRCODE_WRONG_OBJECT_TYPE), ! errmsg("constraint \"%s\" of domain \"%s\" is not a check constraint", ! constrName, NameStr(con->conname)))); ! ! val = SysCacheGetAttr(CONSTROID, tuple, ! Anum_pg_constraint_conbin, ! &isnull); ! if (isnull) ! elog(ERROR, "null conbin for constraint %u", ! HeapTupleGetOid(tuple)); ! conbin = TextDatumGetCString(val); ! ! validateDomainConstraint(domainoid, conbin); ! ! /* ! * Now update the catalog, while we have the door open. ! */ ! copyTuple = heap_copytuple(tuple); ! copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple); ! copy_con->convalidated = true; ! simple_heap_update(conrel, ©Tuple->t_self, copyTuple); ! CatalogUpdateIndexes(conrel, copyTuple); ! heap_freetuple(copyTuple); ! ! systable_endscan(scan); ! ! heap_close(typrel, AccessShareLock); ! heap_close(conrel, RowExclusiveLock); ! ! ReleaseSysCache(tup); ! } ! ! static void ! validateDomainConstraint(Oid domainoid, char *ccbin) ! { ! Expr *expr = (Expr *) stringToNode(ccbin); ! List *rels; ! ListCell *rt; ! EState *estate; ! ExprContext *econtext; ! ExprState *exprstate; /* Need an EState to run ExecEvalExpr */ estate = CreateExecutorState(); *************** *** 2092,2102 **** AlterDomainAddConstraint(List *names, Node *newConstraint) } FreeExecutorState(estate); - - /* Clean up */ - heap_close(typrel, RowExclusiveLock); } - /* * get_rels_with_domain * --- 2206,2212 ---- *************** *** 2416,2422 **** domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, CONSTRAINT_CHECK, /* Constraint Type */ false, /* Is Deferrable */ false, /* Is Deferred */ ! true, /* Is Validated */ InvalidOid, /* not a relation constraint */ NULL, 0, --- 2526,2532 ---- CONSTRAINT_CHECK, /* Constraint Type */ false, /* Is Deferrable */ false, /* Is Deferred */ ! !constr->skip_validation, /* Is Validated */ InvalidOid, /* not a relation constraint */ NULL, 0, *** a/src/backend/optimizer/util/plancat.c --- b/src/backend/optimizer/util/plancat.c *************** *** 552,558 **** get_relation_data_width(Oid relid, int32 *attr_widths) /* * get_relation_constraints * ! * Retrieve the CHECK constraint expressions of the given relation. * * Returns a List (possibly empty) of constraint expressions. Each one * has been canonicalized, and its Vars are changed to have the varno --- 552,558 ---- /* * get_relation_constraints * ! * Retrieve the validated CHECK constraint expressions of the given relation. * * Returns a List (possibly empty) of constraint expressions. Each one * has been canonicalized, and its Vars are changed to have the varno *************** *** 591,596 **** get_relation_constraints(PlannerInfo *root, --- 591,603 ---- { Node *cexpr; + /* + * If this constraint hasn't been fully validated yet, we must + * ignore it here. + */ + if (!constr->check[i].ccvalid) + continue; + cexpr = stringToNode(constr->check[i].ccbin); /* *************** *** 663,669 **** get_relation_constraints(PlannerInfo *root, * * Detect whether the relation need not be scanned because it has either * self-inconsistent restrictions, or restrictions inconsistent with the ! * relation's CHECK constraints. * * Note: this examines only rel->relid, rel->reloptkind, and * rel->baserestrictinfo; therefore it can be called before filling in --- 670,676 ---- * * Detect whether the relation need not be scanned because it has either * self-inconsistent restrictions, or restrictions inconsistent with the ! * relation's validated CHECK constraints. * * Note: this examines only rel->relid, rel->reloptkind, and * rel->baserestrictinfo; therefore it can be called before filling in *** a/src/backend/parser/gram.y --- b/src/backend/parser/gram.y *************** *** 2746,2751 **** ConstraintElem: --- 2746,2753 ---- n->location = @1; n->raw_expr = $3; n->cooked_expr = NULL; + n->skip_validation = false; + n->initially_valid = true; if ($5 != 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), *************** *** 2753,2758 **** ConstraintElem: --- 2755,2771 ---- parser_errposition(@5))); $$ = (Node *)n; } + | CHECK '(' a_expr ')' NOT VALID + { + Constraint *n = makeNode(Constraint); + n->contype = CONSTR_CHECK; + n->location = @1; + n->raw_expr = $3; + n->cooked_expr = NULL; + n->skip_validation = true; + n->initially_valid = false; + $$ = (Node *)n; + } | UNIQUE '(' columnList ')' opt_definition OptConsTableSpace ConstraintAttributeSpec { *************** *** 7563,7568 **** AlterDomainStmt: --- 7576,7590 ---- n->behavior = $7; $$ = (Node *)n; } + /* ALTER DOMAIN <domain> VALIDATE CONSTRAINT <name> */ + | ALTER DOMAIN_P any_name VALIDATE CONSTRAINT name + { + AlterDomainStmt *n = makeNode(AlterDomainStmt); + n->subtype = 'V'; + n->typeName = $3; + n->name = $6; + $$ = (Node *)n; + } ; opt_as: AS {} *** a/src/backend/tcop/utility.c --- b/src/backend/tcop/utility.c *************** *** 820,825 **** standard_ProcessUtility(Node *parsetree, --- 820,829 ---- stmt->name, stmt->behavior); break; + case 'V': /* VALIDATE CONSTRAINT */ + AlterDomainValidateConstraint(stmt->typeName, + stmt->name); + break; default: /* oops */ elog(ERROR, "unrecognized alter domain type: %d", (int) stmt->subtype); *** a/src/backend/utils/cache/relcache.c --- b/src/backend/utils/cache/relcache.c *************** *** 3251,3256 **** CheckConstraintFetch(Relation relation) --- 3251,3257 ---- elog(ERROR, "unexpected constraint record found for rel %s", RelationGetRelationName(relation)); + check[found].ccvalid = conform->convalidated; check[found].ccname = MemoryContextStrdup(CacheMemoryContext, NameStr(conform->conname)); *** a/src/include/access/tupdesc.h --- b/src/include/access/tupdesc.h *************** *** 29,34 **** typedef struct constrCheck --- 29,35 ---- { char *ccname; char *ccbin; /* nodeToString representation of expr */ + bool ccvalid; } ConstrCheck; /* This structure contains constraints of a tuple */ *** a/src/include/catalog/heap.h --- b/src/include/catalog/heap.h *************** *** 30,35 **** typedef struct CookedConstraint --- 30,36 ---- char *name; /* name, or NULL if none */ AttrNumber attnum; /* which attr (only for DEFAULT) */ Node *expr; /* transformed default or check expr */ + bool skip_validation; /* skip validation? (only for CHECK) */ bool is_local; /* constraint has local (non-inherited) def */ int inhcount; /* number of times constraint is inherited */ } CookedConstraint; *** a/src/include/commands/typecmds.h --- b/src/include/commands/typecmds.h *************** *** 31,36 **** extern Oid AssignTypeArrayOid(void); --- 31,37 ---- extern void AlterDomainDefault(List *names, Node *defaultRaw); extern void AlterDomainNotNull(List *names, bool notNull); extern void AlterDomainAddConstraint(List *names, Node *constr); + extern void AlterDomainValidateConstraint(List *names, char *constrName); extern void AlterDomainDropConstraint(List *names, const char *constrName, DropBehavior behavior); *** a/src/include/nodes/parsenodes.h --- b/src/include/nodes/parsenodes.h *************** *** 1190,1195 **** typedef enum AlterTableType --- 1190,1196 ---- AT_AddConstraint, /* add constraint */ AT_AddConstraintRecurse, /* internal to commands/tablecmds.c */ AT_ValidateConstraint, /* validate constraint */ + AT_ValidateConstraintRecurse, /* internal to commands/tablecmds.c */ AT_ProcessedConstraint, /* pre-processed add constraint (local in * parser/parse_utilcmd.c) */ AT_AddIndexConstraint, /* add constraint using existing index */ *************** *** 1543,1548 **** typedef struct Constraint --- 1544,1551 ---- char fk_matchtype; /* FULL, PARTIAL, UNSPECIFIED */ char fk_upd_action; /* ON UPDATE action */ char fk_del_action; /* ON DELETE action */ + + /* Fields used for constraints that allow a NOT VALID specification */ bool skip_validation; /* skip validation of existing rows? */ bool initially_valid; /* start the new constraint as valid */ } Constraint; *** a/src/test/regress/expected/alter_table.out --- b/src/test/regress/expected/alter_table.out *************** *** 196,205 **** DELETE FROM tmp3 where a=5; --- 196,241 ---- -- Try (and succeed) and repeat to show it works on already valid constraint ALTER TABLE tmp3 validate constraint tmpconstr; ALTER TABLE tmp3 validate constraint tmpconstr; + -- Try a non-verified CHECK constraint + ALTER TABLE tmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10); -- fail + ERROR: check constraint "b_greater_than_ten" is violated by some row + ALTER TABLE tmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10) NOT VALID; -- succeeds + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- fails + ERROR: check constraint "b_greater_than_ten" is violated by some row + DELETE FROM tmp3 WHERE NOT b > 10; + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- succeeds + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- succeeds + -- Test inherited NOT VALID CHECK constraints + select * from tmp3; + a | b + ---+---- + 1 | 20 + (1 row) + + CREATE TABLE tmp6 () INHERITS (tmp3); + CREATE TABLE tmp7 () INHERITS (tmp3); + INSERT INTO tmp6 VALUES (6, 30), (7, 16); + ALTER TABLE tmp3 ADD CONSTRAINT b_le_20 CHECK (b <= 20) NOT VALID; + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_le_20; -- fails + ERROR: check constraint "b_le_20" is violated by some row + DELETE FROM tmp6 WHERE b > 20; + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_le_20; -- succeeds + -- An already validated constraint must not be revalidated + CREATE FUNCTION boo(int) RETURNS int IMMUTABLE STRICT LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE 'boo: %', $1; RETURN $1; END; $$; + INSERT INTO tmp7 VALUES (8, 18); + ALTER TABLE tmp7 ADD CONSTRAINT identity CHECK (b = boo(b)); + NOTICE: boo: 18 + ALTER TABLE tmp3 ADD CONSTRAINT IDENTITY check (b = boo(b)) NOT VALID; + NOTICE: merging constraint "identity" with inherited definition + ALTER TABLE tmp3 VALIDATE CONSTRAINT identity; + NOTICE: boo: 16 + NOTICE: boo: 20 -- Try (and fail) to create constraint from tmp5(a) to tmp4(a) - unique constraint on -- tmp4 is a,b ALTER TABLE tmp5 add constraint tmpconstr foreign key(a) references tmp4(a) match full; ERROR: there is no unique constraint matching given keys for referenced table "tmp4" + DROP TABLE tmp7; + DROP TABLE tmp6; DROP TABLE tmp5; DROP TABLE tmp4; DROP TABLE tmp3; *** a/src/test/regress/expected/domain.out --- b/src/test/regress/expected/domain.out *************** *** 352,357 **** alter domain con drop constraint t; --- 352,368 ---- insert into domcontest values (-5); --fails ERROR: value for domain con violates check constraint "con_check" insert into domcontest values (42); + -- Test ALTER DOMAIN .. CONSTRAINT .. NOT VALID + create domain things AS INT; + CREATE TABLE thethings (stuff things); + INSERT INTO thethings (stuff) VALUES (55); + ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11); + ERROR: column "stuff" of table "thethings" contains values that violate the new constraint + ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID; + ALTER DOMAIN things VALIDATE CONSTRAINT meow; + ERROR: column "stuff" of table "thethings" contains values that violate the new constraint + UPDATE thethings SET stuff = 10; + ALTER DOMAIN things VALIDATE CONSTRAINT meow; -- Confirm ALTER DOMAIN with RULES. create table domtab (col1 integer); create domain dom as integer; *** a/src/test/regress/sql/alter_table.sql --- b/src/test/regress/sql/alter_table.sql *************** *** 236,247 **** DELETE FROM tmp3 where a=5; --- 236,276 ---- ALTER TABLE tmp3 validate constraint tmpconstr; ALTER TABLE tmp3 validate constraint tmpconstr; + -- Try a non-verified CHECK constraint + ALTER TABLE tmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10); -- fail + ALTER TABLE tmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10) NOT VALID; -- succeeds + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- fails + DELETE FROM tmp3 WHERE NOT b > 10; + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- succeeds + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- succeeds + + -- Test inherited NOT VALID CHECK constraints + select * from tmp3; + CREATE TABLE tmp6 () INHERITS (tmp3); + CREATE TABLE tmp7 () INHERITS (tmp3); + + INSERT INTO tmp6 VALUES (6, 30), (7, 16); + ALTER TABLE tmp3 ADD CONSTRAINT b_le_20 CHECK (b <= 20) NOT VALID; + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_le_20; -- fails + DELETE FROM tmp6 WHERE b > 20; + ALTER TABLE tmp3 VALIDATE CONSTRAINT b_le_20; -- succeeds + + -- An already validated constraint must not be revalidated + CREATE FUNCTION boo(int) RETURNS int IMMUTABLE STRICT LANGUAGE plpgsql AS $$ BEGIN RAISE NOTICE 'boo: %', $1; RETURN $1; END; $$; + INSERT INTO tmp7 VALUES (8, 18); + ALTER TABLE tmp7 ADD CONSTRAINT identity CHECK (b = boo(b)); + ALTER TABLE tmp3 ADD CONSTRAINT IDENTITY check (b = boo(b)) NOT VALID; + ALTER TABLE tmp3 VALIDATE CONSTRAINT identity; -- Try (and fail) to create constraint from tmp5(a) to tmp4(a) - unique constraint on -- tmp4 is a,b ALTER TABLE tmp5 add constraint tmpconstr foreign key(a) references tmp4(a) match full; + DROP TABLE tmp7; + + DROP TABLE tmp6; + DROP TABLE tmp5; DROP TABLE tmp4; *** a/src/test/regress/sql/domain.sql --- b/src/test/regress/sql/domain.sql *************** *** 259,264 **** alter domain con drop constraint t; --- 259,274 ---- insert into domcontest values (-5); --fails insert into domcontest values (42); + -- Test ALTER DOMAIN .. CONSTRAINT .. NOT VALID + create domain things AS INT; + CREATE TABLE thethings (stuff things); + INSERT INTO thethings (stuff) VALUES (55); + ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11); + ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID; + ALTER DOMAIN things VALIDATE CONSTRAINT meow; + UPDATE thethings SET stuff = 10; + ALTER DOMAIN things VALIDATE CONSTRAINT meow; + -- Confirm ALTER DOMAIN with RULES. create table domtab (col1 integer); create domain dom as integer; -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -
Re: creating CHECK constraints as NOT VALID
Jaime Casanova <jaime@2ndquadrant.com> — 2011-06-14T21:41:00Z
On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > >> > I think that you also need to update the constraint exclusion code >> > (get_relation_constraints() or nearby), otherwise the planner might >> > exclude a relation on the basis of a CHECK constraint that is not >> > currently VALID. >> >> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >> this is quite simple. I don't have it handy right now but I'll post it >> soon. > > Here's the complete patch. > this doesn't apply -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
-
Re: creating CHECK constraints as NOT VALID
Jaime Casanova <jaime@2ndquadrant.com> — 2011-06-15T01:24:25Z
On Tue, Jun 14, 2011 at 4:41 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: >> Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >>> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: >> >>> > I think that you also need to update the constraint exclusion code >>> > (get_relation_constraints() or nearby), otherwise the planner might >>> > exclude a relation on the basis of a CHECK constraint that is not >>> > currently VALID. >>> >>> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >>> this is quite simple. I don't have it handy right now but I'll post it >>> soon. >> >> Here's the complete patch. >> > > this doesn't apply > oops! sorry for the noise... this was gmail problem... the patch was expanded as text, instead of an attachments and when i copy/paste it it should have moved something... copy'ing from archives instead was good -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
-
Re: creating CHECK constraints as NOT VALID
Jaime Casanova <jaime@2ndquadrant.com> — 2011-06-15T06:09:15Z
On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: > >> > I think that you also need to update the constraint exclusion code >> > (get_relation_constraints() or nearby), otherwise the planner might >> > exclude a relation on the basis of a CHECK constraint that is not >> > currently VALID. >> >> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >> this is quite simple. I don't have it handy right now but I'll post it >> soon. > > Here's the complete patch. > psql \h says (among other things) for ALTER TABLE """ ADD table_constraint ADD table_constraint_using_index ADD table_constraint [ NOT VALID ] """ ADD table_constraint appears twice and isn't true that all table_constraint accept the NOT VALID syntax... maybe we can accpet the syntax and send an unimplemented feature message for the other table_constraints? attached, is a script with the examples i have tried: EXAMPLE 1: constraint_exclusion when using NOT VALID check constraints... and it works well, except when the constraint has been validated, it keeps ignoring it (which means i won't benefit from constraint_exclusion) until i execute ANALYZE on the table or close connection EXAMPLE 2: if i have a DOMAIN with a NOT VALID check constraint, and i use it as the new type of a column it checks the constraint -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
-
Re: creating CHECK constraints as NOT VALID
Dean Rasheed <dean.a.rasheed@gmail.com> — 2011-06-15T07:33:57Z
On 15 June 2011 07:09, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Tue, Jun 14, 2011 at 4:14 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: >> Excerpts from Alvaro Herrera's message of lun jun 13 18:08:12 -0400 2011: >>> Excerpts from Dean Rasheed's message of sáb jun 11 09:32:15 -0400 2011: >> >>> > I think that you also need to update the constraint exclusion code >>> > (get_relation_constraints() or nearby), otherwise the planner might >>> > exclude a relation on the basis of a CHECK constraint that is not >>> > currently VALID. >>> >>> Ouch, yeah, thanks for pointing that out. Fortunately the patch to fix >>> this is quite simple. I don't have it handy right now but I'll post it >>> soon. >> >> Here's the complete patch. >> > > psql \h says (among other things) for ALTER TABLE > """ > ADD table_constraint > ADD table_constraint_using_index > ADD table_constraint [ NOT VALID ] > """ > > ADD table_constraint appears twice and isn't true that all > table_constraint accept the NOT VALID syntax... maybe we can accpet > the syntax and send an unimplemented feature message for the other > table_constraints? > Yeah, I was just about to make the same observation about the 9.1beta docs. The 3rd line makes the 1st one redundant. Regards, Dean
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-15T16:24:19Z
Excerpts from Jaime Casanova's message of mié jun 15 02:09:15 -0400 2011: > psql \h says (among other things) for ALTER TABLE > """ > ADD table_constraint > ADD table_constraint_using_index > ADD table_constraint [ NOT VALID ] > """ > > ADD table_constraint appears twice and isn't true that all > table_constraint accept the NOT VALID syntax... maybe we can accpet > the syntax and send an unimplemented feature message for the other > table_constraints? Okay, I removed the redundant line from the synposis. As far as other types of constraints go, I don't feel we need to do anything here -- the description already says that it only works on FKs and CHECK. I'm not going to go to the trouble of fixing the redundant table_constraint line in the synopsis in HEAD -- if someone else wants to send a patch to fix that, I can apply it easily enough. > EXAMPLE 1: > constraint_exclusion when using NOT VALID check constraints... and it > works well, except when the constraint has been validated, it keeps > ignoring it (which means i won't benefit from constraint_exclusion) > until i execute ANALYZE on the table or close connection Hmm, I think this means we need to send a sinval message to invalidate cached plans when a constraint is validated. I'll see about this. > EXAMPLE 2: > if i have a DOMAIN with a NOT VALID check constraint, and i use it as > the new type of a column it checks the constraint I think this is OK. The NOT VALID declaration says that the existing columns declared using this constraint is not checked, but new columns (as well as new data in existing columns) are certainly going to require their values to pass the checks. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Robert Haas <robertmhaas@gmail.com> — 2011-06-15T16:53:59Z
On Wed, Jun 15, 2011 at 12:24 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Hmm, I think this means we need to send a sinval message to invalidate > cached plans when a constraint is validated. I'll see about this. I feel like that really ought to be happening automatically, as a result of committing the transaction that did the system catalog modification. It seems pretty strange if it isn't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-15T17:06:40Z
Excerpts from Robert Haas's message of mié jun 15 12:53:59 -0400 2011: > On Wed, Jun 15, 2011 at 12:24 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > Hmm, I think this means we need to send a sinval message to invalidate > > cached plans when a constraint is validated. I'll see about this. > > I feel like that really ought to be happening automatically, as a > result of committing the transaction that did the system catalog > modification. It seems pretty strange if it isn't. The catalog change takes place in pg_constraint, so I'm not sure that it'd cause the sort of event we need. I'm testing whether adding a call to CacheInvalidateRelcache in the appropriate place works. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-15T18:49:04Z
Alvaro Herrera <alvherre@commandprompt.com> writes: > Excerpts from Robert Haas's message of mi jun 15 12:53:59 -0400 2011: >> On Wed, Jun 15, 2011 at 12:24 PM, Alvaro Herrera >> <alvherre@commandprompt.com> wrote: >>> Hmm, I think this means we need to send a sinval message to invalidate >>> cached plans when a constraint is validated. I'll see about this. >> I feel like that really ought to be happening automatically, as a >> result of committing the transaction that did the system catalog >> modification. It seems pretty strange if it isn't. > The catalog change takes place in pg_constraint, so I'm not sure that > it'd cause the sort of event we need. I'm testing whether adding a call > to CacheInvalidateRelcache in the appropriate place works. Currently, only updates in pg_class, pg_attribute, and pg_index cause automatic relcache invalidations --- see the logic in PrepareForTupleInvalidation. If you want to force replanning after an update elsewhere, you need to call CacheInvalidateRelcache. I've occasionally thought about extending the number of cases that get handled automatically by PrepareForTupleInvalidation, but not gotten off my duff to change it. I doubt that we want to make that routine know about *every* possible case, so it's a matter of tradeoffs ... regards, tom lane
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-15T19:21:09Z
Excerpts from Tom Lane's message of mié jun 15 14:49:04 -0400 2011: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Excerpts from Robert Haas's message of mié jun 15 12:53:59 -0400 2011: > >> On Wed, Jun 15, 2011 at 12:24 PM, Alvaro Herrera > >> <alvherre@commandprompt.com> wrote: > >>> Hmm, I think this means we need to send a sinval message to invalidate > >>> cached plans when a constraint is validated. I'll see about this. > > >> I feel like that really ought to be happening automatically, as a > >> result of committing the transaction that did the system catalog > >> modification. It seems pretty strange if it isn't. > > > The catalog change takes place in pg_constraint, so I'm not sure that > > it'd cause the sort of event we need. I'm testing whether adding a call > > to CacheInvalidateRelcache in the appropriate place works. > > Currently, only updates in pg_class, pg_attribute, and pg_index cause > automatic relcache invalidations --- see the logic in > PrepareForTupleInvalidation. If you want to force replanning after an > update elsewhere, you need to call CacheInvalidateRelcache. I've > occasionally thought about extending the number of cases that get > handled automatically by PrepareForTupleInvalidation, but not gotten off > my duff to change it. I doubt that we want to make that routine know > about *every* possible case, so it's a matter of tradeoffs ... I think pg_trigger is closer to needing a new case in PrepareForTupleInvalidation than pg_constraint, at this point -- triggers seem to be involved rather more with CacheInvalidateRelcache (and close relatives) calls than constraints. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-15T20:51:11Z
Excerpts from Jaime Casanova's message of mié jun 15 02:09:15 -0400 2011: > psql \h says (among other things) for ALTER TABLE > """ > ADD table_constraint > ADD table_constraint_using_index > ADD table_constraint [ NOT VALID ] > """ > > ADD table_constraint appears twice and isn't true that all > table_constraint accept the NOT VALID syntax... maybe we can accpet > the syntax and send an unimplemented feature message for the other > table_constraints? > > attached, is a script with the examples i have tried: > > EXAMPLE 1: > constraint_exclusion when using NOT VALID check constraints... and it > works well, except when the constraint has been validated, it keeps > ignoring it (which means i won't benefit from constraint_exclusion) > until i execute ANALYZE on the table or close connection Here's an updated patch fixing all of the above. I stole your first test case and added it to regression, after some editorialization. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-15T23:32:29Z
Alvaro Herrera <alvherre@commandprompt.com> writes: > Here's an updated patch fixing all of the above. I stole your first > test case and added it to regression, after some editorialization. I've probably created some merge conflicts for you in process of fixing the FOREIGN KEY NOT VALID patch, but in any case you need to change this to use ConstraintAttributeSpec rather than a duplicate production. regards, tom lane
-
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-16T00:08:09Z
Excerpts from Tom Lane's message of mié jun 15 19:32:29 -0400 2011: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Here's an updated patch fixing all of the above. I stole your first > > test case and added it to regression, after some editorialization. > > I've probably created some merge conflicts for you in process of fixing > the FOREIGN KEY NOT VALID patch, but in any case you need to change this > to use ConstraintAttributeSpec rather than a duplicate production. Yeah, nothing serious. Updated patch attached. The wording in the doc changes could probably use some look over. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
-
Re: creating CHECK constraints as NOT VALID
Jaime Casanova <jaime@2ndquadrant.com> — 2011-06-16T09:10:43Z
On Wed, Jun 15, 2011 at 7:08 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > > Yeah, nothing serious. Updated patch attached. The wording in the doc > changes could probably use some look over. > looks good to me... at least it compiles, and function as i would expect... tomorrow i will read the code more carefully and look at the docs, but probably this is just fine to be commited... -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
-
Re: creating CHECK constraints as NOT VALID
Jaime Casanova <jaime@2ndquadrant.com> — 2011-06-18T06:57:22Z
On Thu, Jun 16, 2011 at 4:10 AM, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Wed, Jun 15, 2011 at 7:08 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: >> >> Yeah, nothing serious. Updated patch attached. The wording in the doc >> changes could probably use some look over. >> > > looks good to me... at least it compiles, and function as i would expect... > tomorrow i will read the code more carefully and look at the docs, but > probably this is just fine to be commited... > I think that this paragraph is confusing, but not being an natural english speaker i will let others give their opinion here: ! If NOT VALID is not specified, ! the command will only succeed if all columns using the ! domain satisfy the new constraint. ! The constraint is going to be enforced on new data inserted into columns ! using the domain in all cases, regardless of <literal>NOT VALID</>. ! <literal>NOT VALID</> is only accepted for <literal>CHECK</> constraints. Even if it is redundant maybe here should say "This form validates a constraint previously added as NOT VALID", otherwise someone could think that by default constraints are not enforced and should be validated + <term>VALIDATE CONSTRAINT</term> + <listitem> + <para> + This form validates a constraint previously added, that is, verify that + all data in columns using the domain satisfy the specified constraint. + </para> + </listitem> + </varlistentry> otherwise the patch looks good, and works fine in every test i could imagine... -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
-
Re: creating CHECK constraints as NOT VALID
Robert Haas <robertmhaas@gmail.com> — 2011-06-19T03:53:17Z
On Sat, Jun 18, 2011 at 2:57 AM, Jaime Casanova <jaime@2ndquadrant.com> wrote: >> looks good to me... at least it compiles, and function as i would expect... >> tomorrow i will read the code more carefully and look at the docs, but >> probably this is just fine to be commited... > > I think that this paragraph is confusing, but not being an natural > english speaker i will let others give their opinion here: > ! If NOT VALID is not specified, > ! the command will only succeed if all columns using the > ! domain satisfy the new constraint. > ! The constraint is going to be enforced on new data inserted into columns > ! using the domain in all cases, regardless of <literal>NOT VALID</>. > ! <literal>NOT VALID</> is only accepted for <literal>CHECK</> > constraints. I agree. That's pretty contorted. How about something like this: When a new constraint is added to a domain, all columns using that domain will be checked against the newly added constraint. These checks can be suppressed by adding the new constraint using the NOT VALID option; the constraint can later be made valid using ALTER DOMAIN .. VALIDATE CONSTRAINT. Newly inserted or updated rows are always checked against all constraints, even those marked NOT VALID. In lieu of: <command>ALTER DOMAIN</command> conforms to the <acronym>SQL</acronym> standard, ! except for the <literal>OWNER</>, <literal>SET SCHEMA</> and ! <literal>VALIDATE CONSTRAINT</> variants, ! as well as the <literal>NOT VALID</> clause of the <literal>ADD CONSTRAINT</> variant, which are <productname>PostgreSQL</productname> extensions. </para> I suggest: ALTER DOMAIN conforms to the SQL standard, except for the OWNER, SET SCHEMA, and VALIDATE CONSTRAINT variants, which are PostgreSQL extensions. The NOT VALID clause of the ADD CONSTRAINT variant is also a PostgreSQL extension. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -
Re: creating CHECK constraints as NOT VALID
Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-30T16:00:34Z
Excerpts from Robert Haas's message of sáb jun 18 23:53:17 -0400 2011: > I agree. That's pretty contorted. How about something like this: > Thanks Jaime and Robert. I have pushed this patch with these fixes. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support