Re: Review: Non-inheritable check constraints - Mailing list pgsql-hackers
From | Nikhil Sontakke |
---|---|
Subject | Re: Review: Non-inheritable check constraints |
Date | |
Msg-id | CABamaqMQOK3kFnTFeQMBRam7cm7scEMxFwTMqy2a8GeTkFb6eQ@mail.gmail.com Whole thread Raw |
In response to | Re: Review: Non-inheritable check constraints (Alvaro Herrera <alvherre@commandprompt.com>) |
List | pgsql-hackers |
Hi Alvaro,
The patch looks ok to me. I see that we now sort the constraints by conisonly value too:
@@ -1781,12 +1781,20 @@ describeOneTableDetails(const char *schemaname,
/* print table (and column) check constraints */
if (tableinfo.checks)
{
+ char *is_only;
+
+ if (pset.sversion > 90100)
+ is_only = "r.conisonly";
+ else
+ is_only = "false AS conisonly";
+
printfPQExpBuffer(&buf,
- "SELECT r.conname, "
+ "SELECT r.conname, %s, "
"pg_catalog.pg_get_constraintdef(r.oid, true)\n"
"FROM pg_catalog.pg_constraint r\n"
- "WHERE r.conrelid = '%s' AND r.contype = 'c'\nORDER BY 1;",
- oid);
+ "WHERE r.conrelid = '%s' AND r.contype = 'c'\n"
+ "ORDER BY 2, 1;",
+ is_only, oid);
result = PSQLexec(buf.data, false);
if (!result)
goto error_return;
My preference would be to see true values first, so might want to modify it to
"ORDER BY 2 desc, 1"
to have that effect. Your call finally.
Regards,
Nikhils
The patch looks ok to me. I see that we now sort the constraints by conisonly value too:
@@ -1781,12 +1781,20 @@ describeOneTableDetails(const char *schemaname,
/* print table (and column) check constraints */
if (tableinfo.checks)
{
+ char *is_only;
+
+ if (pset.sversion > 90100)
+ is_only = "r.conisonly";
+ else
+ is_only = "false AS conisonly";
+
printfPQExpBuffer(&buf,
- "SELECT r.conname, "
+ "SELECT r.conname, %s, "
"pg_catalog.pg_get_constraintdef(r.oid, true)\n"
"FROM pg_catalog.pg_constraint r\n"
- "WHERE r.conrelid = '%s' AND r.contype = 'c'\nORDER BY 1;",
- oid);
+ "WHERE r.conrelid = '%s' AND r.contype = 'c'\n"
+ "ORDER BY 2, 1;",
+ is_only, oid);
result = PSQLexec(buf.data, false);
if (!result)
goto error_return;
My preference would be to see true values first, so might want to modify it to
"ORDER BY 2 desc, 1"
to have that effect. Your call finally.
Regards,
Nikhils
On Sat, Dec 17, 2011 at 12:36 AM, Alvaro Herrera <alvherre@commandprompt.com> wrote:
Excerpts from Greg Smith's message of vie dic 16 15:02:20 -0300 2011:> On 12/04/2011 02:22 AM, Nikhil Sontakke wrote:I think I misread what that was trying to do. I thought it would turn
> >
> > Is it okay to modify an existing constraint to mark it as "only", even
> > if it was originally inheritable? This is not clear to me. Maybe the
> > safest course of action is to raise an error. Or maybe I'm misreading
> > what it does (because I haven't compiled it yet).
> >
> >
> > Hmmm, good question. IIRC, the patch will pass is_only as true only if
> > it going to be a locally defined, non-inheritable constraint. So I
> > went by the logic that since it was ok to merge and mark a constraint
> > as locally defined, it should be ok to mark it non-inheritable from
> > this moment on with that new local definition?
on the "is only" bit on a constraint that a child had inherited from a
previous parent, but that was obviously wrong now that I think about it
again.Yeah. Nikhil, Alex, this is the merged patch. Have a look that it
> With this open question, this looks like it's back in Alvaro's hands
> again to me. This one started the CF as "Ready for Committer" and seems
> stalled there for now. I'm not going to touch its status, just pointing
> this fact out.
still works for you (particularly the pg_dump bits) and I'll commit it.
I adjusted the regression test a bit too.
Thanks.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
pgsql-hackers by date: