INSTEAD rule bug?

Dmitry Tkach <dmitry@openratings.com> writes:

testdb=# create table test_view as select * from test;
SELECT

That is not a view, it's only a static copy of the original table.

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

testdb=# create table test_view as select * from test;
SELECT

That is not a view, it's only a static copy of the original table.

regards, tom lane

I know... That was a typo in my sql :-)
But for this example it doesn't matter - that view/table is only needed
to illustrate the rules behaviour on insert.
You can just replace 'table' with 'view' in that statement - the
behaviour of the insert is exactly the same anyway.

Dima

Dmitry Tkach <dmitry@openratings.com> writes:

I know... That was a typo in my sql :-)
But for this example it doesn't matter - that view/table is only needed
to illustrate the rules behaviour on insert.

Oh, I see what you're on about. Sorry, a "DO INSTEAD NOTHING" only
suppresses the original command, it does not suppress other rules.
I think what you want is to make the insert_test rule conditional
on x being not null.

regards, tom lane

Tom Lane wrote:

Oh, I see what you're on about. Sorry, a "DO INSTEAD NOTHING" only
suppresses the original command, it does not suppress other rules.
I think what you want is to make the insert_test rule conditional
on x being not null.

Yeah... that's what I was afraid of :-(
The problem is that in the 'real life' situation the condition is a lot
more complicated than this simple is null test... I hate having to
duplicate it, and I hate even more having to evaluate it twice on every
insert :-(

Dima

Dmitry Tkach <dmitry@openratings.com> writes:

The problem is that in the 'real life' situation the condition is a lot
more complicated than this simple is null test... I hate having to
duplicate it, and I hate even more having to evaluate it twice on every
insert :-(

Why evaluate it twice? The DO INSTEAD NOTHING rule should be
unconditional.

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

The problem is that in the 'real life' situation the condition is a lot
more complicated than this simple is null test... I hate having to
duplicate it, and I hate even more having to evaluate it twice on every
insert :-(

Why evaluate it twice? The DO INSTEAD NOTHING rule should be
unconditional.

Right. But the problem is I don't want to discard the invalid entries
completely...
So, it would have to be *three* rules, not just two - like:

create rule skip_null as on insert to test_view where x is null do instead
insert into invalid_entries ('NULL DATA', new.*);
create rule insert_test as on insert to test_view where is is not null
do instead
insert into test values (new.*);
create rule dummy_insert as on insert to test_view do instead nothing;

... so x is null ends up being evaluated twice...

Dima

Oh, I see what you're on about. Sorry, a "DO INSTEAD NOTHING" only
suppresses the original command, it does not suppress other rules.
I think what you want is to make the insert_test rule conditional
on x being not null.

regards, tom lane

Ok... What's wrong with this one then (three rules are conditional - if
x is null or y is null, the entry is rejected, if x is not null and y is
not null, it is inserted, the fourth rull is a dummy to prevent the
planner from complaining about not being able to insert into a view):

testdb=# create table test (x int not null, y int not null);
CREATE TABLE
testdb=# create view test_view as select * from test;
CREATE VIEW
testdb=# create rule reject_x as on insert to test_view where new.x is
null do instead
testdb=# create table test_reject (x int, y int, reason text);
CREATE TABLE
testdb=# create rule reject_x as on insert to test_view where x is null
do instead insert into test_reject values (new.*, 'NULL x');
CREATE RULE
testdb=# create rule reject_y as on insert to test_view where y is null
do instead insert into test_reject values (new.*, 'NULL y');
CREATE RULE
testdb=# create rule insert_test as on insert to test_view where x is
not null and y is not null do instead insert into test values (new.*);
CREATE RULE
testdb=# create rule insert_dummy as on insert to test_view do instead
nothing;
CREATE RULE
testdb=# insert into test values (null, null);
ERROR: ExecInsert: Fail to add null value in not null attribute x

Now, why does this fail?
The only rule that attempts to insert anything into test has 'x is not
null and y is not null' as the qualifier.
What's wrong?

Thanks!

Dima

Dmitry Tkach <dmitry@openratings.com> writes:

Ok... What's wrong with this one then

testdb=# insert into test values (null, null);
ERROR: ExecInsert: Fail to add null value in not null attribute x

Try inserting into test_view ...

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

Ok... What's wrong with this one then

testdb=# insert into test values (null, null);
ERROR: ExecInsert: Fail to add null value in not null attribute x

Try inserting into test_view ...

regards, tom lane

Damn!
Sorry about that....

Actually, believe it or not, I do actually have a similar problem on
7.2.4...
I was just trying to reproduce it with a simple example, and screwed up
the insert statement... :-(
Sorry...

I just checked this example on 7.2, and it works fine...

But what the hell is my problem then??? I swear, I do insert into the
view there :-)
It's a really huge view, looking at a whole bunch of different tables...
I'd hate having to post the whole thing...

Dima

Dmitry Tkach <dmitry@openratings.com> writes:

But what the hell is my problem then??? I swear, I do insert into the
view there :-)
It's a really huge view, looking at a whole bunch of different tables...
I'd hate having to post the whole thing...

All I can guess is a bug (or pilot error) that's triggered by the more
complex view. I think you'll just have to try to whittle down the
failure to something you can post.

regards, tom lane

Aha!

I got it.
This generates the 'cannot insert null...' error:

create table test (x int not null, y int not null);
create table test_reject (x int, y int, reason text);

create view test_view as select * from test;

create rule reject_x as on insert to test_view where new.x is null do
instead insert into test_reject values (new.*, 'NULL x');

create rule reject_y as on insert to test_view where new.y is null do
instead insert into test_reject values (new.*, 'NULL y');

create rule insert_test as on insert to test_view where new.x is not
null and new.y is not null do instead
(
insert into test
select new.* union
select new.*;
);

create rule insert_dummy as on insert to test_view do instead nothing;

-- insert into test_reject values (new.*,
-- case when new.x is null then 'NULL x' else 'NULL y' end);

insert into test_view values (null, null);

It looks like the UNION in the 'not null' rule is the problem.
If I change it to just insert ... select (without the union), or to two
inserts, then it works.
But union always fails, even if I add a 'where false' to the end, so
that it only returns one row...

Dima

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

create rule insert_test as on insert to test_view where new.x is not
null and new.y is not null do instead
(
insert into test
select new.* union
select new.*;
);

Mmm. In CVS tip that throws

ERROR: UNION/INTERSECT/EXCEPT member statement may not refer to other relations of same query level

which was a check added as a result of this discussion thread:
http://archives.postgresql.org/pgsql-general/2003-02/msg00693.php

I am sure you are running into some misbehavior associated with the
fact that the rule transformation generates a bogusly-structured SQL
query, and 7.2 isn't noticing.

I'd like to support this case someday, but it's not clear how...

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

create rule insert_test as on insert to test_view where new.x is not
null and new.y is not null do instead
(
insert into test
select new.* union
select new.*;
);

Mmm. In CVS tip that throws

ERROR: UNION/INTERSECT/EXCEPT member statement may not refer to other relations of same query level

Actually, I just used that new.* as an example (if I understand this
error message correctly, that's what it refers to, right?)
Something like
insert into test
select null,null union select 1,2 where false

has the same problem... and it doesn't refer to any relations.

which was a check added as a result of this discussion thread:
http://archives.postgresql.org/pgsql-general/2003-02/msg00693.php

I'll take a look at that thread, thanks!

I am sure you are running into some misbehavior associated with the
fact that the rule transformation generates a bogusly-structured SQL
query, and 7.2 isn't noticing.

Not just 7.2... I was testing this in 7.3 - it has the same problem

Dima

I'd like to support this case someday, but it's not clear how...

I don't know if it helps, but somehow if I do

insert into test select * from (select null,null union select 1,2 where
false) as dummy

... that works fine.

Thanks!

Dima

Dmitry Tkach <dmitry@openratings.com> writes:

Something like
insert into test
select null,null union select 1,2 where false
has the same problem... and it doesn't refer to any relations.

But that's parsed as

insert into test
(select null,null) union (select 1,2 where false)

so I'd expect it to bomb if test has NOT NULL constraints.

Not just 7.2... I was testing this in 7.3 - it has the same problem

Yeah, the change is post-7.3.

insert into test select * from (select null,null union select 1,2 where
false) as dummy
... that works fine.

I get
ERROR: ExecInsert: Fail to add null value in not null attribute x
which is what I'd expect.

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

Something like
insert into test
select null,null union select 1,2 where false
has the same problem... and it doesn't refer to any relations.

But that's parsed as

insert into test
(select null,null) union (select 1,2 where false)

so I'd expect it to bomb if test has NOT NULL constraints.

Sure, but it is inside the rule that has 'where x is not null and y is
not null' on it as a qualifier, so
with my test example it should just never get executed in the first place.

Not just 7.2... I was testing this in 7.3 - it has the same problem

Yeah, the change is post-7.3.

insert into test select * from (select null,null union select 1,2 where
false) as dummy
... that works fine.

I get
ERROR: ExecInsert: Fail to add null value in not null attribute x
which is what I'd expect.

Really? In 7.3?
That's weird...
Here is what I am getting exactly:

testdb=# drop table test cascade;
NOTICE: Drop cascades to rule insert_test on view test_view
NOTICE: Drop cascades to rule _RETURN on view test_view
NOTICE: Drop cascades to view test_view
DROP TABLE
testdb=# drop table test_reject cascade;
DROP TABLE
testdb=#
testdb=# create table test (x int not null, y int not null);
CREATE TABLE
testdb=# create table test_reject (x int, y int, reason text);
CREATE TABLE
testdb=#
testdb=# create view test_view as select * from test;
CREATE VIEW
testdb=#
testdb=# create rule reject_x as on insert to test_view where new.x is
null do instead insert into test_reject values (new.*, 'NULL x');
CREATE RULE
testdb=#
testdb=# create rule reject_y as on insert to test_view where new.y is
null do instead insert into test_reject values (new.*, 'NULL y');
CREATE RULE
testdb=#
testdb=# create rule insert_test as on insert to test_view where new.x
is not null and new.y is not null do instead
testdb-# (
testdb(# insert into test select * from
testdb(# (select null,null union select 1,2 where false) as dummy
testdb(# );
CREATE RULE
testdb=#
testdb=# create rule dummy_insert as on insert to test_view do instead
nothing;
CREATE RULE
testdb=#
testdb=#
testdb=# insert into test_view values (null, null);
INSERT 17648 1
testdb=# select * from test;
x | y
---+---
(0 rows)

testdb=# select * from test_reject;
x | y | reason
---+---+--------
| | NULL x
| | NULL y
(2 rows)

Dmitry Tkach <dmitry@openratings.com> writes:

Sure, but it is inside the rule that has 'where x is not null and y is
not null' on it as a qualifier, so
with my test example it should just never get executed in the first place.

You're confusing rules with triggers. The INSERT *will* get executed;
the rule's qualifier gets moved to the WHERE of the INSERT...SELECT,
and the way you get no effect is for the qual to fail on every row the
SELECT generates.

One way to think about the problem (though I'm not sure this is right in
detail) is that there's no place to hang a top-level WHERE on a UNION.

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

Sure, but it is inside the rule that has 'where x is not null and y is
not null' on it as a qualifier, so
with my test example it should just never get executed in the first place.

You're confusing rules with triggers. The INSERT *will* get executed;
the rule's qualifier gets moved to the WHERE of the INSERT...SELECT,
and the way you get no effect is for the qual to fail on every row the
SELECT generates.

One way to think about the problem (though I'm not sure this is right in
detail) is that there's no place to hang a top-level WHERE on a UNION.

Ok. If so, should UNION not be disallowed entirely inside (at least
conditional) rules, regadless of whether it has those 'cross-from'
references or not?
What you are saying makes sense to me (and I have already rewritten that
rule, and it is working now)... but it's unfortunate that I had to spend
half a day trying to figure out why the damn thing doesn't work... (even
worse really - I've written that rule a while ago, and it already made
it into the production database before anyone noticed that it did not
really work) :-(
It would have saved a lot of trouble if it just complained about that
union thing right away and refuse to create the rule...

On a different note, I think there *is* a way to add a where clause to
the union - that's exactly what I did in that last example - by
converting it into a subselect...
Can that not be done automatically for conditional rules?
(I doubt that would be very useful though... since it's no longer
possible to use old and new there... I can't really think of any useful
application of a union inside a rule, except for my obscure 'select 1,2'
example :-)

Dima

Dmitry Tkach <dmitry@openratings.com> writes:

It would have saved a lot of trouble if it just complained about that
union thing right away and refuse to create the rule...

That's what happens in CVS tip.

On a different note, I think there *is* a way to add a where clause to
the union - that's exactly what I did in that last example - by
converting it into a subselect...
Can that not be done automatically for conditional rules?

Send a patch... or at least convince us it can be done ... I'm not
convinced yet.

regards, tom lane

Tom Lane wrote:

Dmitry Tkach <dmitry@openratings.com> writes:

It would have saved a lot of trouble if it just complained about that
union thing right away and refuse to create the rule...

That's what happens in CVS tip.

I thought you said it was only complaining about references to new and
old, not about *any* union clause...
Did I get it wrong?

On a different note, I think there *is* a way to add a where clause to
the union - that's exactly what I did in that last example - by
converting it into a subselect...
Can that not be done automatically for conditional rules?

Send a patch... or at least convince us it can be done ... I'm not
convinced yet.

I am afraid, that's too complicated for me :-)
I tried to dig through the source a little bit when I was struggling to
figure out why the damn thing did not work, but I could not even find
the place where those qualifiers are evaluated. :-(

Besides, as I said earlier, I don't really think that such an
improvement would be of much use anyway, unless at the same time we find
away to allow referencing new and old (or at least new, which, I suspect
is much easier) from inside the union... I don't really understand the
reason why that cannot be supported (but I am sure, it's a good one)
:-), and without that I just can't think of any example where using the
union inside a rule would be useful for anything anyway, so, unless we
want to consider allowing new and old at the same time, it looks like
trying to make unions work isn't worth the effort... Just indicating
properly that they don't would be good enough

Dima