Thread: Table constraint ordering disrupted by pg_dump
Tricia Holben (pholben@greatbridge.com) reports a bug with a severity of 2
The lower the number the more severe it is.
Short Description
Table constraint ordering disrupted by pg_dump
Long Description
When running a backup/restore version of the regression tests on 7.0.3 and 7.1RCD1 the following was noted: when the
createtable command contained two constraints, the error messages would reflect that the constraints were being checked
ina consistent order and error messages would show either the constraint name or $1 or $2 if no name was used for the
constraintwhich generated the first error. After dumping and restoring the database, the order was reversed. As a
resultthe meaning of error messages changed.
Sample Code
Original create statement:
CREATE TABLE INSERT_TBL (x INT DEFAULT nextval('insert_seq'),
y TEXT DEFAULT '-NULL-',
z INT DEFAULT -1 * currval('insert_seq'),
CONSTRAINT INSERT_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8),
CHECK (x + z = 0));
NOTE - if the first constraint fails the error message reads:
ERROR: ExecAppend: rejected due to CHECK constraint insert_con
NOTE - if the first passes but the second fails the message reads:
ERROR: ExecAppend: rejected due to CHECK constraint $2
The code in the pg_dump file is:
CREATE TABLE "insert_tbl" (
"x" integer DEFAULT nextval('insert_seq'::text),
"y" text DEFAULT '-NULL-',
"z" integer DEFAULT (-1 * currval('insert_seq'::text)),
CHECK (((x + z) = 0)),
CONSTRAINT "insert_con" CHECK ((((x >= 3) AND (y <> 'check failed'::text)) AND (x < 8)))
);
The constraints are checked in a different order now and the one error message becomes:
ERROR: ExecAppend: rejected due to CHECK constraint $1
No file was uploaded with this report
pgsql-bugs@postgresql.org writes:
> When running a backup/restore version of the regression tests on 7.0.3
> and 7.1RCD1 the following was noted: when the create table command
> contained two constraints, the error messages would reflect that the
> constraints were being checked in a consistent order and error
> messages would show either the constraint name or $1 or $2 if no name
> was used for the constraint which generated the first error. After
> dumping and restoring the database, the order was reversed. As a
> result the meaning of error messages changed.
I don't believe this is a bug. There is no guarantee that constraints
are checked in any particular order.
regards, tom lane
At 16:40 2/04/01 -0400, Tom Lane wrote:
>
>I don't believe this is a bug. There is no guarantee that constraints
>are checked in any particular order.
>
While it't not a bug, it would be nice if pg_dump reproduced definitions as
faithfully as possible. To that end, would it be worth selecting the
constraints in OID order (using oid from pg_relcheck)?
----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/
Philip Warner <pjw@rhyme.com.au> writes:
> While it't not a bug, it would be nice if pg_dump reproduced definitions as
> faithfully as possible. To that end, would it be worth selecting the
> constraints in OID order (using oid from pg_relcheck)?
If it's just another clause in a query, you might as well. I wouldn't
take any risks for it though...
regards, tom lane
At 23:55 2/04/01 -0400, Tom Lane wrote:
>Philip Warner <pjw@rhyme.com.au> writes:
>> While it't not a bug, it would be nice if pg_dump reproduced definitions as
>> faithfully as possible. To that end, would it be worth selecting the
>> constraints in OID order (using oid from pg_relcheck)?
>
>If it's just another clause in a query, you might as well. I wouldn't
>take any risks for it though...
Just an ORDER BY. This has been done in CVS, but since I could not
reproduce the original problem, I can't check it, so let me know how it works.
----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/
Patricia Holben <pholben@greatbridge.com> writes:
> I have loaded this new version and re-tested but don't see it fixed.
It looks fixed to me. Original SQL in constraints.sql:
CREATE TABLE INSERT_TBL (x INT DEFAULT nextval('insert_seq'),
y TEXT DEFAULT '-NULL-',
z INT DEFAULT -1 * currval('insert_seq'),
CONSTRAINT INSERT_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8),
CHECK (x + z = 0));
As dumped by current-CVS pg_dump:
CREATE TABLE "insert_tbl" (
"x" integer DEFAULT nextval('insert_seq'::text),
"y" text DEFAULT '-NULL-',
"z" integer DEFAULT (-1 * currval('insert_seq'::text)),
CONSTRAINT "insert_con" CHECK ((((x >= 3) AND (y <> 'check failed'::text)) AND (x < 8))),
CHECK (((x + z) = 0))
);
> I do have a separate routine which I am running - it isn't complete which
> is why I haven't shared it yet - which proves that when the dump is
> reloaded the constraints are not applied in the original order.
I notice that the backend tends to apply the constraints in the reverse
order of declaration, but this should be consistent between the original
database and the dumped/reloaded one.
regards, tom lane
Patricia Holben <pholben@greatbridge.com> writes:
> improvement but as usual there is a related error. In the constraints=20
> test, there is a table created "INSERT_CHILD" which has a constraint and=
> then inherits from "INSERT_TBL" the constraint "insert_con" and the=20
> unnamed check. If you look at the expected output from this test, when =
> the inherited unnamed check fails, the error is $1. After doing the=20
> dump/reload, this error is called $3.
Hm. There are a couple of things going on here. The one that may be
worth fixing is that pg_dump isn't reliably recognizing nameless
inherited constraints as duplicates. It looks for matches on both
rcname and rcsrc, but the rcname may get reassigned (particularly if
there is multiple inheritance). This will lead to multiple instances
of the same constraint, which is inefficient, and becomes more and more
so with repeated dump/reload cycles.
Rather than using "c.rcname = pg_relcheck.rcname" as part of the match
condition, consider
(c.rcname = pg_relcheck.rcname OR
(c.rcname[0] = '$' AND pg_relcheck.rcname[0] = '$'))
so that any two nameless constraints will be considered duplicate if
their rcsrcs match.
The other thing is that the backend won't necessarily assign a nameless
constraint the same $-index in different tables, so even if pg_dump is
changed there's not a guarantee that you won't get different error
messages in the above example. I don't consider that a bug, however.
If you're depending on the name of a constraint then you should name it.
regards, tom lane
PS: Philip, it seems to me that lines 2071-2121 in pg_dump.c are largely
a waste of time, since the subsequent query to fetch the constraints
will do all the same work over again. Just have to relax the check at
line 2157 to allow ntups2 <= ncheck, and update ncheck after that.
At 17:51 3/04/01 -0400, Tom Lane wrote:
>
>Rather than using "c.rcname = pg_relcheck.rcname" as part of the match
>condition, consider
> (c.rcname = pg_relcheck.rcname OR
> (c.rcname[0] = '$' AND pg_relcheck.rcname[0] = '$'))
>so that any two nameless constraints will be considered duplicate if
>their rcsrcs match.
Looks reasonable, but users can define constraints with names starting with
'$', so there is a small chance we will discard a redundant constraint,
which is probably OK.
BTW, it looks like 'ALTER TABLE ADD CONSTRAINT' on a parent does not affect
the already created children. Is this intended behaviour? It seems like a
but and it certainly will cause an issue for pg_dump.
>PS: Philip, it seems to me that lines 2071-2121 in pg_dump.c are largely
>a waste of time, since the subsequent query to fetch the constraints
>will do all the same work over again. Just have to relax the check at
>line 2157 to allow ntups2 <= ncheck, and update ncheck after that.
I was planning to clean up that area when 7.1 was out the door since I
assumed that there was an obscure reason for it's existence.
Looking at the code more closely, I think it is part of the sanity check
pg_dump tries to do. The first piece of SQL finds inherited constraints,
and the second one finds non-inherited constraints, but the way the SQL is
structured, I can't really see how they would fail to come to the correct
total, so it's value as a sanity check seems minimal. Do you agree, or
should I leave it in?
----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/
Philip Warner <pjw@rhyme.com.au> writes:
> At 17:51 3/04/01 -0400, Tom Lane wrote:
>> Rather than using "c.rcname = pg_relcheck.rcname" as part of the match
>> condition, consider
>> (c.rcname = pg_relcheck.rcname OR
>> (c.rcname[0] = '$' AND pg_relcheck.rcname[0] = '$'))
>> so that any two nameless constraints will be considered duplicate if
>> their rcsrcs match.
> Looks reasonable, but users can define constraints with names starting with
> '$', so there is a small chance we will discard a redundant constraint,
> which is probably OK.
Actually, I was seriously considering the fascist alternative: if the
rcsrc fields match then discard the child's constraint, never mind what
its name is. Functionally the result is the same...
> BTW, it looks like 'ALTER TABLE ADD CONSTRAINT' on a parent does not affect
> the already created children. Is this intended behaviour? It seems like a
> but and it certainly will cause an issue for pg_dump.
This seems a bug, but I do not think we should address it for 7.1.
TODO list material.
>> PS: Philip, it seems to me that lines 2071-2121 in pg_dump.c are largely
>> a waste of time, since the subsequent query to fetch the constraints
>> will do all the same work over again. Just have to relax the check at
>> line 2157 to allow ntups2 <= ncheck, and update ncheck after that.
> I was planning to clean up that area when 7.1 was out the door since I
> assumed that there was an obscure reason for it's existence.
> Looking at the code more closely, I think it is part of the sanity check
> pg_dump tries to do. The first piece of SQL finds inherited constraints,
> and the second one finds non-inherited constraints, but the way the SQL is
> structured, I can't really see how they would fail to come to the correct
> total, so it's value as a sanity check seems minimal. Do you agree, or
> should I leave it in?
My thought is that it's useless as a sanity check (since there's no
orthogonal-condition test there), and it does pose a maintenance problem.
For example, to adjust the name-matching condition being discussed
above, you'd have to remember to change both queries. Change only one
and you're in trouble. Given that, the downside is bigger than the
upside.
regards, tom lane
At 17:51 3/04/01 -0400, Tom Lane wrote:
>
>Rather than using "c.rcname = pg_relcheck.rcname" as part of the match
>condition, consider
...
>it seems to me that lines 2071-2121 in pg_dump.c are largely
>a waste of time
Done and applied.
----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/
I have loaded this new version and re-tested but don't see it fixed. If=20 the standard regression tests are run and after completion a pg_dump is=20 performed: pg_dump -o regression > dumpfile, if you look in the dumpfile,= =20 you will see that the create for table insert_tbl shows the Check being=20 applied before the constraint insert_con. I do have a separate routine which I am running - it isn't complete which= =20 is why I haven't shared it yet - which proves that when the dump is=20 reloaded the constraints are not applied in the original order. Tricia Holben 757-233-5568 >>>>>>>>>>>>>>>>>> Original Message <<<<<<<<<<<<<<<<<< On 4/3/01, 3:57:02 AM, Philip Warner <pjw@rhyme.com.au> wrote regarding Re:= =20 [BUGS] Table constraint ordering disrupted by pg_dump : > At 23:55 2/04/01 -0400, Tom Lane wrote: > >Philip Warner <pjw@rhyme.com.au> writes: > >> While it't not a bug, it would be nice if pg_dump reproduced definitio= ns=20 as > >> faithfully as possible. To that end, would it be worth selecting the > >> constraints in OID order (using oid from pg_relcheck)? > > > >If it's just another clause in a query, you might as well. I wouldn't > >take any risks for it though... > Just an ORDER BY. This has been done in CVS, but since I could not > reproduce the original problem, I can't check it, so let me know how it= =20 works. > ---------------------------------------------------------------- > Philip Warner | __---_____ > Albatross Consulting Pty. Ltd. |----/ - \ > (A.B.N. 75 008 659 498) | /(@) ______---_ > Tel: (+61) 0500 83 82 81 | _________ \ > Fax: (+61) 0500 83 82 82 | ___________ | > Http://www.rhyme.com.au | / \| > | --________-- > PGP key available upon request, | / > and from pgp5.ai.mit.edu:11371 |/=