SEO

I'm looking at how NULLs are handled in relation to plpgsql row types.
Looking at exec_assign_value, it appears that we're supposed to be able
to handle setting a row variable to NULL:

if (*isNull)
{
/* If source is null, just assign nulls to the row */
exec_move_row(estate, NULL, row, NULL, NULL);
}

However, the test right above that means that we'll fail if the user
tries something like "row_variable := NULL;":

if (!(valtype == RECORDOID ||
get_typtype(valtype) == 'c'))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cannot assign non-composite value to a row variable")));

Presumably, I can just add code to that test to allow for *isNull.

Of course, setting a row variable to null is a lot more useful if we can
actually test for it after the fact, and I'm not really sure how to make
that happen.
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

"Jim C. Nasby" <jim@nasby.net> writes:
> However, the test right above that means that we'll fail if the user
> tries something like "row_variable := NULL;":

The patch you seem to have in mind would allow
row_variable := int_variable;
to succeed if the int_variable chanced to contain NULL, which is surely
not very desirable.

The real issue here is that the bare NULL has type UNKNOWN and we're not
making any effort to cast it. I'm not sure whether it'd work to simply
apply exec_cast_value --- that looks like it's only meant to handle
scalars, where in general you'd need something close to
ExecEvalConvertRowtype().

> Of course, setting a row variable to null is a lot more useful if we can
> actually test for it after the fact, and I'm not really sure how to make
> that happen.

Doesn't IS NULL work (as of CVS HEAD)?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

On Oct 2, 2006, at 6:28 PM, Tom Lane wrote:
> "Jim C. Nasby" <jim@nasby.net> writes:
>> However, the test right above that means that we'll fail if the user
>> tries something like "row_variable := NULL;":
>
> The patch you seem to have in mind would allow
> row_variable := int_variable;
> to succeed if the int_variable chanced to contain NULL, which is
> surely
> not very desirable.

Hrm... is there any reasonable way to catch that?

> The real issue here is that the bare NULL has type UNKNOWN and
> we're not
> making any effort to cast it. I'm not sure whether it'd work to
> simply
> apply exec_cast_value --- that looks like it's only meant to handle
> scalars, where in general you'd need something close to
> ExecEvalConvertRowtype().
>
>> Of course, setting a row variable to null is a lot more useful if
>> we can
>> actually test for it after the fact, and I'm not really sure how
>> to make
>> that happen.
>
> Doesn't IS NULL work (as of CVS HEAD)?

Ahh, so it does. Doesn't work with RECORD, though... which seems a
bit surprising. I can't really think of a good reason why they should
differ.

ERROR: record "v_row" is not assigned yet
DETAIL: The tuple structure of a not-yet-assigned record is
indeterminate.
CONTEXT: PL/pgSQL function "test" line 4 at return

--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)



---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Is there a TODO here?

---------------------------------------------------------------------------

Jim Nasby wrote:
> On Oct 2, 2006, at 6:28 PM, Tom Lane wrote:
> > "Jim C. Nasby" <jim@nasby.net> writes:
> >> However, the test right above that means that we'll fail if the user
> >> tries something like "row_variable := NULL;":
> >
> > The patch you seem to have in mind would allow
> > row_variable := int_variable;
> > to succeed if the int_variable chanced to contain NULL, which is
> > surely
> > not very desirable.
>
> Hrm... is there any reasonable way to catch that?
>
> > The real issue here is that the bare NULL has type UNKNOWN and
> > we're not
> > making any effort to cast it. I'm not sure whether it'd work to
> > simply
> > apply exec_cast_value --- that looks like it's only meant to handle
> > scalars, where in general you'd need something close to
> > ExecEvalConvertRowtype().
> >
> >> Of course, setting a row variable to null is a lot more useful if
> >> we can
> >> actually test for it after the fact, and I'm not really sure how
> >> to make
> >> that happen.
> >
> > Doesn't IS NULL work (as of CVS HEAD)?
>
> Ahh, so it does. Doesn't work with RECORD, though... which seems a
> bit surprising. I can't really think of a good reason why they should
> differ.
>
> ERROR: record "v_row" is not assigned yet
> DETAIL: The tuple structure of a not-yet-assigned record is
> indeterminate.
> CONTEXT: PL/pgSQL function "test" line 4 at return
>
> --
> Jim Nasby jim@nasby.net
> EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

On Tue, Feb 13, 2007 at 05:55:11PM -0500, Bruce Momjian wrote:
>
> Is there a TODO here?
>
> ---------------------------------------------------------------------------
>
> Jim Nasby wrote:
> > On Oct 2, 2006, at 6:28 PM, Tom Lane wrote:
> > > "Jim C. Nasby" <jim@nasby.net> writes:
> > >> However, the test right above that means that we'll fail if the user
> > >> tries something like "row_variable := NULL;":
> > >
> > > The patch you seem to have in mind would allow
> > > row_variable := int_variable;
> > > to succeed if the int_variable chanced to contain NULL, which is
> > > surely
> > > not very desirable.

Well, that's Tom's objection, though I'm not sure if by 'int_variable'
he means 'internal' or 'integer'.

Personally, I think it would be useful to just allow setting a row or
record variable to NULL as I showed it above; ie: no variables involved.
This is something you might want to do to invalidate a row/record
variable after taking some action (perhaps deleting a row).

You'd also think that you should be able to detect if a record variable
is null, as you can with row.

So, I suggest:

* Allow row and record variables in plpgsql to be set to NULL

It's not clear if it's a wise idea to allow this assignment from a
variable. It may be better to only allow explicitly setting them,
ie:

row_variable := NULL;

* Allow testing a record variable to see if it's NULL

Currently works for row variables, but not record variables
--
Jim Nasby jim@nasby.net
EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Added to TODO under PL/pgSQL:

o Allow row and record variables to be set to NULL constants,
and allow NULL tests on such variables

Because a row is not scalar, do not allow assignment
from NULL-valued scalars.


---------------------------------------------------------------------------

Jim C. Nasby wrote:
> On Tue, Feb 13, 2007 at 05:55:11PM -0500, Bruce Momjian wrote:
> >
> > Is there a TODO here?
> >
> > ---------------------------------------------------------------------------
> >
> > Jim Nasby wrote:
> > > On Oct 2, 2006, at 6:28 PM, Tom Lane wrote:
> > > > "Jim C. Nasby" <jim@nasby.net> writes:
> > > >> However, the test right above that means that we'll fail if the user
> > > >> tries something like "row_variable := NULL;":
> > > >
> > > > The patch you seem to have in mind would allow
> > > > row_variable := int_variable;
> > > > to succeed if the int_variable chanced to contain NULL, which is
> > > > surely
> > > > not very desirable.
>
> Well, that's Tom's objection, though I'm not sure if by 'int_variable'
> he means 'internal' or 'integer'.
>
> Personally, I think it would be useful to just allow setting a row or
> record variable to NULL as I showed it above; ie: no variables involved.
> This is something you might want to do to invalidate a row/record
> variable after taking some action (perhaps deleting a row).
>
> You'd also think that you should be able to detect if a record variable
> is null, as you can with row.
>
> So, I suggest:
>
> * Allow row and record variables in plpgsql to be set to NULL
>
> It's not clear if it's a wise idea to allow this assignment from a
> variable. It may be better to only allow explicitly setting them,
> ie:
>
> row_variable := NULL;
>
> * Allow testing a record variable to see if it's NULL
>
> Currently works for row variables, but not record variables
> --
> Jim Nasby jim@nasby.net
> EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

On 10/2/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Jim C. Nasby" <jim@nasby.net> writes:
> > However, the test right above that means that we'll fail if the user
> > tries something like "row_variable := NULL;":
>
> The patch you seem to have in mind would allow
> row_variable := int_variable;
> to succeed if the int_variable chanced to contain NULL, which is surely
> not very desirable.
>
> The real issue here is that the bare NULL has type UNKNOWN and we're not
> making any effort to cast it. I'm not sure whether it'd work to simply
> apply exec_cast_value --- that looks like it's only meant to handle
> scalars, where in general you'd need something close to
> ExecEvalConvertRowtype().
>
> > Of course, setting a row variable to null is a lot more useful if we can
> > actually test for it after the fact, and I'm not really sure how to make
> > that happen.
>
> Doesn't IS NULL work (as of CVS HEAD)?
>

Is there a specific reason why we keep the tuple descriptor of an
unassigned record type to NULL?

Surely we don't know what tuple descriptor it will actually contain,
however, maybe we can have "special" tuple descriptors for un-assigned
record types.

For example, if for NULL/unassigned record type we create a tuple
descriptor of "VOID" type, and then initialize its corresponding (one
column) row to null, we 'll have the <row> IS NULL check working on
unassigned or NULL record types as well.

regards,
--
Sibte Abbas

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend