Обсуждение: BUG #2873: Function that returns an empty set with a 'not null' domain errors in 8.2 but not 8.1

The following bug has been logged online:

Bug reference:      2873
Logged by:          Jonathan Hull
Email address:      jono@fabsoftware.com
PostgreSQL version: 8.1 / 8.2
Operating system:   Linux
Description:        Function that returns an empty set with a 'not null'
domain errors in 8.2 but not 8.1
Details:

Hi,
I have been testing PostgreSQL 8.2 and have noticed that a function that
returns an empty result set errors in 8.2 but not in 8.1 - is this a bug?

The key feature for the error is that when a result structure (eg : pg_foo)
is defined with a domain type that is not null, only PG 8.2 errors if the
result is an empty set. If the domain is replaced by a standard data type,
no error occurs in either 8.1 or 8.2.

Below is some code to duplicate the issue. When the two runs are executed in
PG 8.1, both work. When the two runs are executed in PG 8.2, the second run
errors.

Run 1 - without domain [succeeds PG 8.1 and 8.2] :

-- Create test Run 1
create table pg_foo (foo_field varchar(20) not null);
create or replace function test_foo(integer) returns setof pg_foo AS '
declare
  isempty alias for $1;
  foovar  pg_foo%rowtype;
begin
  for foovar in
    select ''has a value'' as foo_field where (0 = isempty)
  loop
    return next foovar;
  end loop;
  return;
end;
' LANGUAGE 'plpgsql';
select * from test_foo(0);
select * from test_foo(1);


Run 2 - with domain [succeeds PG 8.1, errors 8.2] :

-- Remove test Run 1 first
drop function test_foo(integer);
drop table pg_foo;

-- Create test Run 1
create domain dom_foo as varchar(20) not null;
create table pg_foo (foo_field dom_foo);
create or replace function test_foo(integer) returns setof pg_foo AS '
declare
  isempty alias for $1;
  foovar  pg_foo%rowtype;
begin
  for foovar in
    select ''has a value'' as foo_field where (0 = isempty)
  loop
    return next foovar;
  end loop;
  return;
end;
' LANGUAGE 'plpgsql';

select * from test_foo(0);
select * from test_foo(1);

Is this a bug or is this a new constraint feature implemented in 8.2?



Jonathan Hull
Director (Operations)
FAB Software (NZ) Limited
Ph +64 3 365 7851
PO Box 4567
Christchurch
New Zealand

"Jonathan Hull" <jono@fabsoftware.com> writes:
> The key feature for the error is that when a result structure (eg : pg_foo)
> is defined with a domain type that is not null, only PG 8.2 errors if the
> result is an empty set.

The problem is explained well enough by this comment in plpgsql's code
for FOR-over-query:
   /*    * If the query didn't return any rows, set the target to NULL and return    * with FOUND = false.    */

At the time this code was written, there weren't any potential negative
side-effects of trying to set a row value to all NULLs, but now it's
possible that that fails because of domain constraints.

I think the idea was to ensure that a record variable would have the
correct structure (matching the query output) post-loop, even if the
query produced zero rows.  But it's not clear that that is really
useful for anything, given plpgsql's dearth of introspection facilities.
So we could make Jonathan's problem go away if we just take out the
assignment of nulls, and say that FOR over no rows leaves the record
variable unchanged.  The documentation doesn't specify the current
behavior.

Looking through the code, I see another place that does the same thing:
FETCH from a cursor, when the cursor has no more rows to return.  It's
a bit harder to argue that it's sane to leave the variable unchanged
in this case.  However, the documentation doesn't actually promise that
the target gets set to null in this case either.

Thoughts?
        regards, tom lane

On Mon, Jan 08, 2007 at 10:27:15AM -0500, Tom Lane wrote:
> "Jonathan Hull" <jono@fabsoftware.com> writes:
> > The key feature for the error is that when a result structure (eg : pg_foo)
> > is defined with a domain type that is not null, only PG 8.2 errors if the
> > result is an empty set.
> 
> The problem is explained well enough by this comment in plpgsql's code
> for FOR-over-query:
> 
>     /*
>      * If the query didn't return any rows, set the target to NULL and return
>      * with FOUND = false.
>      */
> 
> At the time this code was written, there weren't any potential negative
> side-effects of trying to set a row value to all NULLs, but now it's
> possible that that fails because of domain constraints.
> 
> I think the idea was to ensure that a record variable would have the
> correct structure (matching the query output) post-loop, even if the
> query produced zero rows.  But it's not clear that that is really
> useful for anything, given plpgsql's dearth of introspection facilities.
> So we could make Jonathan's problem go away if we just take out the
> assignment of nulls, and say that FOR over no rows leaves the record
> variable unchanged.  The documentation doesn't specify the current
> behavior.
> 
> Looking through the code, I see another place that does the same thing:
> FETCH from a cursor, when the cursor has no more rows to return.  It's
> a bit harder to argue that it's sane to leave the variable unchanged
> in this case.  However, the documentation doesn't actually promise that
> the target gets set to null in this case either.

I think it would be safer to set the record variable to something that
wipes any old data. I can't think of any examples of good code that
would fall prey to this, but I can certainly think of some nasty bugs
that users could inadvertently create. I know I'd personally like to
have the safety net...

Perhaps a means to mark the record as being null, other than setting all
the fields to null? That might also allow for a means for users to set
records to null, which I think would be useful in some cases.

BTW, are row variables also affected by this bug or is it just record
variables?
-- 
Jim Nasby                                            jim@nasby.net
EnterpriseDB      http://enterprisedb.com      512.569.9461 (cell)

"Jim C. Nasby" <jim@nasby.net> writes:
> Perhaps a means to mark the record as being null, other than setting all
> the fields to null?

We could probably bypass the call of the domain input function, thereby
avoiding the elog, but the point remains: if we do that, then we have
a NOT-NULL-constrained domain variable that is reading out as NULL.
One way or another we're going to be violating somebody's expectation.

(BTW, I suspect that the case "DECLARE foo nonnulldomain;" already has
this issue, as I think that code path just stores a null without any
ceremony.)

This is closely related to the discussion a couple weeks ago about how
a LEFT JOIN could produce nulls in an output column that was labeled as
having a non-null-domain type.  We haven't figured out what is a sane
behavior for that case, either.  I'm beginning to think that domains
constrained not null are just fundamentally a bad idea.
        regards, tom lane

Tom Lane wrote:

> This is closely related to the discussion a couple weeks ago about how
> a LEFT JOIN could produce nulls in an output column that was labeled as
> having a non-null-domain type.  We haven't figured out what is a sane
> behavior for that case, either.  I'm beginning to think that domains
> constrained not null are just fundamentally a bad idea.
>

I think we just expect left joins to produce nulls regardless of
constraints on the underlying cols, don't we? Concluding that not null in
domains is bad seems a bit drastic.

cheers

andrew