Обсуждение: BUG #2704: pg_class.relchecks overflow problem
The following bug has been logged online: Bug reference: 2704 Logged by: Toru SHIMOGAKI Email address: shimogaki.toru@oss.ntt.co.jp PostgreSQL version: 8.1.4 Operating system: Red Hat Enterprise Linux AS 4 Description: pg_class.relchecks overflow problem Details: Hi, pg_class.relchecks is defined as int2. But the upper bound of this value is not checked and it overflows. I found it at the following case: 1. I tried to add check constraints: "alter table test_a add check (aaa > i);" (0 <= i <= 32767) 2. When I added the 32768th check constraint, the value of pg_class.relchecs became -32768. postgres=# alter table test_a add check ( aaa > 32768 ); ALTER TABLE postgres=# select relname, relchecks from pg_class where relname = 'test_a'; relname | relchecks ---------+----------- test_a | -32768 (1 row) 3. The following error message was found when I added the next one: postgres=# alter table test_a add check ( aaa > 32769 ); ERROR: unexpected constraint record found for rel test_a postgres=# select relname, relchecks from pg_class where relname = 'test_a'; relname | relchecks ---------+----------- test_a | -32768 (1 row) Best regards,
How about this patch? Of course, it might be a rare case that such check is necessary... Toru SHIMOGAKI wrote: > The following bug has been logged online: > > Bug reference: 2704 > Logged by: Toru SHIMOGAKI > Email address: shimogaki.toru@oss.ntt.co.jp > PostgreSQL version: 8.1.4 > Operating system: Red Hat Enterprise Linux AS 4 > Description: pg_class.relchecks overflow problem > Details: > > Hi, > > pg_class.relchecks is defined as int2. But the upper bound of this value is > not checked and it overflows. > > > I found it at the following case: > > 1. I tried to add check constraints: > > "alter table test_a add check (aaa > i);" (0 <= i <= 32767) > > > 2. When I added the 32768th check constraint, the value of pg_class.relchecs > became -32768. > > postgres=# alter table test_a add check ( aaa > 32768 ); > ALTER TABLE > postgres=# select relname, relchecks from pg_class where relname = > 'test_a'; > relname | relchecks > ---------+----------- > test_a | -32768 > (1 row) > > > 3. The following error message was found when I added the next one: > > postgres=# alter table test_a add check ( aaa > 32769 ); > ERROR: unexpected constraint record found for rel test_a > postgres=# select relname, relchecks from pg_class where relname = > 'test_a'; > relname | relchecks > ---------+----------- > test_a | -32768 > (1 row) > > > Best regards, > > ---------------------------(end of broadcast)--------------------------- > TIP 1: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > > -- Toru SHIMOGAKI<shimogaki.toru@oss.ntt.co.jp> diff -cpr postgresql-8.1.5-orig/src/backend/catalog/heap.c postgresql-8.1.5/src/backend/catalog/heap.c *** postgresql-8.1.5-orig/src/backend/catalog/heap.c 2006-04-24 10:40:39.000000000 +0900 --- postgresql-8.1.5/src/backend/catalog/heap.c 2006-10-23 16:50:22.000000000 +0900 *************** AddRelationRawConstraints(Relation rel, *** 1525,1530 **** --- 1525,1535 ---- continue; Assert(cdef->cooked_expr == NULL); + if (numchecks == 0x7FFF) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("cannot have more than 2^15-1 checks in a table"))); + /* * Transform raw parsetree to executable expression. */
Toru SHIMOGAKI <shimogaki.toru@oss.ntt.co.jp> writes: > + if (numchecks == 0x7FFF) > + ereport(ERROR, > + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), > + errmsg("cannot have more than 2^15-1 checks in a table"))); While there's not anything wrong with this proposed patch in itself, I have to admit that I don't see the point. There are probably thousands of places in the backend where we increment an integer value without checking for overflow. Is this one particularly more likely to occur than other ones, or does it have worse consequences than other ones? I don't see a security issue here (since the backend doesn't crash) and I also don't see that this limit is close enough to real practice to be important to guard against. It's not that the check imposes any significant addition in code space or runtime, but what it *would* impose is a nontrivial extra burden on our message translators. Scale this up by a few hundred or thousand equally unlikely conditions with their own error messages, and we'd have a revolt ... regards, tom lane
Tom Lane wrote: > While there's not anything wrong with this proposed patch in itself, > I have to admit that I don't see the point. The points are: 1. It is just unpleasant to leave the overflow. 2. It is not easy for users to understand what they should do when they encounter the error message. At least users can't understand that it is because of the upper limit: ERROR: unexpected constraint record found for rel test_a I haven't found such a case in real practice. But I think the limit will be a little closer than that is now, because constraint exclusion is expanded for UPDATE/DELETE in 8.2 and the opportunity of using check constraint will increase . So I investigated the upper limit and found it. By the way, as you said, it would impose an extra burden on message translators and I can understand your opinion. But will it lead directly to the reason that we don't fix it? Best regards, -- Toru SHIMOGAKI<shimogaki.toru@oss.ntt.co.jp>