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

On Sat, Dec 17, 2011 at 12:36 AM, Alvaro Herrera <alvhe...@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:
> > >
> > >     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?
>
> I think I misread what that was trying to do.  I thought it would turn
> 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.
>
> > 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.
>
> Yeah.  Nikhil, Alex, this is the merged patch.  Have a look that it
> 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 <alvhe...@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
>
>

Reply via email to