Thread: Minor inheritance/check bug: Inconsistent behavior
Hi;
I figured I would report this as well, primarily because people
getting into table inheritance may try to use this to solve the set
exclusion problem (i.e. partly required to prevent duplicate key
values in an inheritance tree). I see this as minor because I don't
see a lot of people using these aspects of the software in this way
now.
or_examples=# create table cities (city text, state text, is_capital
bool, altitude int, check(not(is_capital and tableoid::regclass::text
= 'cities')));
The intent of the check constraint is to force rows in the parent
table to use only a part of the key domain, while another portion
(where is_capital is true) can be reserved for child tables.
or_examples=# insert into cities values ('Seattle', 'Washington', false, 100);
INSERT 0 1
or_examples=# insert into cities values ('Olympia', 'Washington', true, 100);
INSERT 0 1
Ok, note that the check constraint was violated by the second row but
apparently this wasn't caught.
or_examples=# select *, tableoid::regclass::text from cities;
city | state | is_capital | altitude | tableoid
---------+------------+------------+----------+----------
Seattle | Washington | f | 100 | cities
Olympia | Washington | t | 100 | cities
(2 rows)
And indeed if we try to add the constraint again over the top
PostgreSQL will complain loudly.
or_examples=# alter table cities add check(not(is_capital and
tableoid::regclass::name = 'cities'));
ERROR: check constraint "cities_check1" is violated by some row
My guess is that tableoid is not known when the check constraint is
checked. It seems to me one option would be to either disallow
checking tableoid in the check constraint or making this known.
However as it is, PostgreSQL will not raise an error until after the
insert has already been made and the check constraint is re-applied.
or_examples=# select version();
version
---------------------------------------------------------------------------------
--------------------------
PostgreSQL 9.1.4 on i386-redhat-linux-gnu, compiled by gcc (GCC)
4.7.0 20120507
(Red Hat 4.7.0-5), 32-bit
(1 row)
Chris Travers <chris@metatrontech.com> writes:
> My guess is that tableoid is not known when the check constraint is
> checked.
None of the system columns are set at the time check constraints are
checked.
regards, tom lane
From: pgsql-bugs-owner@postgresql.org [mailto:pgsql-bugs-owner@postgresql.org] On Behalf Of Tom Lane Sent: Friday, August 24, 2012 10:33 AM Chris Travers <chris@metatrontech.com> writes: >> My guess is that tableoid is not known when the check constraint is >> checked. >None of the system columns are set at the time check constraints are >checked. Is there any problem if set tableOID before calling ExecConstarints()? I am not sure may be this is not so important problem for which we don't want to change existing code. With Regards, Amit Kapila.
Amit Kapila <amit.kapila@huawei.com> writes:
> From: pgsql-bugs-owner@postgresql.org
> [mailto:pgsql-bugs-owner@postgresql.org] On Behalf Of Tom Lane
>> None of the system columns are set at the time check constraints are
>> checked.
> Is there any problem if set tableOID before calling ExecConstarints()?
Well, possibly we could kluge things to make that particular case work,
but if someone expects that to be valid then why not oid, ctid, xmin,
etc? And more to the point, what's the value of examining tableoid in
a check constraint? The constraint is attached to a particular table,
so the tableoid would be a constant for it anyway.
regards, tom lane
On Fri, Aug 24, 2012 at 3:52 AM, Amit Kapila <amit.kapila@huawei.com> wrote: > From: pgsql-bugs-owner@postgresql.org > [mailto:pgsql-bugs-owner@postgresql.org] On Behalf Of Tom Lane > Sent: Friday, August 24, 2012 10:33 AM > Chris Travers <chris@metatrontech.com> writes: >>> My guess is that tableoid is not known when the check constraint is >>> checked. > >>None of the system columns are set at the time check constraints are >>checked. > > Is there any problem if set tableOID before calling ExecConstarints()? > I am not sure may be this is not so important problem for which we don't > want to > change existing code. Just to be sure my initial concern is this: Table inheritance would be easier if there was a way to declare a constraint such that it prevents insert for some rows on the parent but not for a child and/or vice versa. This can be used to partition the primary key between a parent and child tables to avoid some key overlap issues. My concern was that this was the "clever" solution that someone would try, insufficiently test, and find out that their clever solution in fact did nothing except preventing the constraint from being dropped and later re-applied. As for the ideal way of looking at addressing this possibility: I am further not going to speak for the developers here but I do wonder about system columns generally and check constraints, and whether the same solution is just to check the check constraint and error if a system column is checked. Some things seem obvious but what happens if someone says "this table cannot grow beyind 5 pages and we will do this by checking against ctid"? If we start pulling out some system columns for special treatment I am not sure where it ends. I am assuming that ctid cannot be safely known before the row is formally stored on disk. I think the cleanest fix interface-wise is to prevent check constraints from being added to tables in this case. I don't see it as a high priority though. My larger priority is to flag this as a possible thing someone is likely to try to get around the issues storing rows in both parent and child tables. Best Wishes, Chris Travers
Chris Travers <chris@metatrontech.com> writes:
> Table inheritance would be easier if there was a way to declare a
> constraint such that it prevents insert for some rows on the parent
> but not for a child and/or vice versa.
FWIW, 9.2 adds support for constraints that only apply to the named
table and don't get inherited to children. I think use of such
constraints is probably better design than mucking around with tableoid,
even if we were to make that work.
regards, tom lane
From: Tom Lane [mailto:tgl@sss.pgh.pa.us] Sent: Friday, August 24, 2012 7:46 PM Amit Kapila <amit.kapila@huawei.com> writes: > From: pgsql-bugs-owner@postgresql.org > [mailto:pgsql-bugs-owner@postgresql.org] On Behalf Of Tom Lane >>> None of the system columns are set at the time check constraints are >>> checked. >> Is there any problem if set tableOID before calling ExecConstarints()? > Well, possibly we could kluge things to make that particular case work, > but if someone expects that to be valid then why not oid, ctid, xmin, > etc? Initially, I thought of saying like that but the same is done in heap_prepare_insert() So doing it 2 times doesn't make sense and if we move them out then all places from where heap_insert gets called, we have to do it like that. However why I have asked to set tableOID, as for it, already functions CopyFrom and AtRewriteTable does it(set tableOid before Constraints check). > And more to the point, what's the value of examining tableoid in > a check constraint? The constraint is attached to a particular table, > so the tableoid would be a constant for it anyway. Here what you are suggesting if I understood correctly is while defining such constraints make sure its not allowed, because once defined, during execution we might not be able to identify. Please correct me if I have misunderstood? With Regards, Amit Kapila.
On Fri, Aug 24, 2012 at 10:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Chris Travers <chris@metatrontech.com> writes: >> Table inheritance would be easier if there was a way to declare a >> constraint such that it prevents insert for some rows on the parent >> but not for a child and/or vice versa. > > FWIW, 9.2 adds support for constraints that only apply to the named > table and don't get inherited to children. I think use of such > constraints is probably better design than mucking around with tableoid, > even if we were to make that work. Maybe, but in that case shouldn't referencing a system column chuck an error? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> Maybe, but in that case shouldn't referencing a system column chuck an error?
Yeah, possibly. I think none of them are populated with anything useful
during INSERT checks (though OID might be an exception?). Less sure
about the state during UPDATE checks, though.
regards, tom lane
On Mon, Aug 27, 2012 at 1:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > Maybe, but in that case shouldn't referencing a system column chuck an > error? > > Yeah, possibly. I think none of them are populated with anything useful > during INSERT checks (though OID might be an exception?). Less sure > about the state during UPDATE checks, though. > > My vote honestly would be to make it be an error. A check constraint which fails only after it has been saved and then updated strikes me as behavior outside the role of a check constraint and dangerously so. It doesn't work as advertised, and people will find this out only after their data is shown not to be consistent with the check constraint. This being said, again, my sense is that no inherit check constraints will make it quite unlikely that this will ever affect production servers. So failing this it's sufficient I think in future versions (maybe 9.3 forward) to add a paragraph to the docs. Something like: Warning: The behavior of a check constraint operating against a system column is undefined. Check constraints are not intended to be used this way and behavior may change without notice. Maybe worth bringing up on the docs list. I don't mind the fact that behavior is undefined in some cases. However, it might be a good idea to let people know that they are moving into "we won't commit not to breaking your app even if you get this to work" territory. Best Wishes, Chris Travers
From: Tom Lane [mailto:tgl@sss.pgh.pa.us] Sent: Tuesday, August 28, 2012 2:04 AM Robert Haas <robertmhaas@gmail.com> writes: >> Maybe, but in that case shouldn't referencing a system column chuck an error? > Yeah, possibly. I think none of them are populated with anything useful > during INSERT checks (though OID might be an exception?). Less sure > about the state during UPDATE checks, though. AFAICT during Update also, it doesn't contain useful. The only chance it would have contain something useful is when it goes for EvalPlanQual and then again comes to check for constraints. However these attributes get filled in heap_update much later. So now should the fix be that it returns an error for system column reference except for OID case? With Regards, Amit Kapila.
On Tue, Aug 28, 2012 at 6:40 AM, Amit Kapila <amit.kapila@huawei.com> wrote: > AFAICT during Update also, it doesn't contain useful. The only chance it > would have contain something useful is when it goes for EvalPlanQual and > then again comes to check for constraints. However these attributes get > filled in heap_update much later. > > So now should the fix be that it returns an error for system column > reference except for OID case? +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Friday, September 07, 2012 12:20 AM Amit Kapila wrote: On Tue, Aug 28, 2012 at 6:40 AM, Amit Kapila <amit.kapila@huawei.com> wrote: >> AFAICT during Update also, it doesn't contain useful. The only chance it >> would have contain something useful is when it goes for EvalPlanQual and >> then again comes to check for constraints. However these attributes get >> filled in heap_update much later. > >> So now should the fix be that it returns an error for system column >> reference except for OID case? > +1. I will start working on preparing a patch for the fix, if any problems, I will discuss. With Regards, Amit Kapila.