Horton
Menu

Thread

  1. BUG #18190: PL/pgSQL does not document comparison operations on RECORD (or ROW) types

    PG Bug reporting form <noreply@postgresql.org> — 2023-11-10T19:47:30Z

    The following bug has been logged on the website:

Bug reference:      18190
Logged by:          Karl Pinc
Email address:      kop@karlpinc.com
PostgreSQL version: 15.4
Operating system:   Linux
Description:        

Nothing is said about how to compare entire RECORD type variables, or ROW
type variables.

This leads to surprising behavior in the code below.  The row in table "foo"
that has foo.col2 = NULL does not raise an error, but the row that has
foo.col2 = 1 does.

CREATE OR REPLACE FUNCTION checktime (tbl TEXT, col TEXT)
  RETURNS void
  LANGUAGE plpgsql
  AS $$
  DECLARE
    qry TEXT;
    problem RECORD := NULL;
  BEGIN
    qry := '
    SELECT *
      FROM ' || quote_ident(tbl) || '
      WHERE ' || quote_ident(col) || '::TIME <> $tim$00:00:00$tim$';
    EXECUTE qry INTO problem;
    IF problem IS NOT NULL THEN
      RAISE EXCEPTION data_exception USING
            MESSAGE = tbl
                      || ' has a time part in column '
                      || col
                      || ' that is not 00:00:00'
          , DETAIL = 'The row is: ' || problem;
    END IF;
  END;
$$;

CREATE TABLE foo (stamp TIMESTAMP WITHOUT TIME ZONE, col2 INT NULL);

INSERT INTO foo (stamp, col2) VALUES ('1979-01-01 00:01:00', NULL);

SELECT checktime('foo', 'stamp');

TRUNCATE foo;

INSERT INTO foo (stamp, col2) VALUES ('1979-01-01 00:01:00', 1);

SELECT checktime('foo', 'stamp');

Probably the right way to write the code is with GET DIAGNOSTICS..., but
that's neither here nor there.

Or maybe an error should be raised when trying to use IS NULL with RECORD
type variables.  Or the code needs changing so IS NULL/IS NOT NULL works
with RECORD type variables.  Or something else needs doing.

Note that I've not tried anything with ROW type variables but it seems worth
mentioning them since it's not obvious how to tell if they've ever been
assigned a value, which is vaguely related to the intention of the code
above.  Likewise, equality comparisons on ROWs and RECORDs (not mixed) seem
like they should work, but what of other comparisions?

If the issue is documentation, the PL/pgSQL docs should link to the
composite type docs, or the expression docs, or type conversion, or where
ever the current documentation is that I can't find or that needs to be
written.

  2. Re: BUG #18190: PL/pgSQL does not document comparison operations on RECORD (or ROW) types

    David G. Johnston <david.g.johnston@gmail.com> — 2023-11-12T01:13:35Z

    On Friday, November 10, 2023, PG Bug reporting form <noreply@postgresql.org>
wrote:

> The following bug has been logged on the website:
>
> Bug reference:      18190
> Logged by:          Karl Pinc
> Email address:      kop@karlpinc.com
> PostgreSQL version: 15.4
> Operating system:   Linux
> Description:
>
> Nothing is said about how to compare entire RECORD type variables, or ROW
> type variables.


There is, in the chapters with the word compare in their titles.

https://www.postgresql.org/docs/current/functions-comparison.html#FUNCTIONS-COMPARISON

https://www.postgresql.org/docs/current/functions-comparisons.html#FUNCTIONS-COMPARISONS


This leads to surprising behavior in the code below.


You couldn’t make something less difficult to read?  Like a self-contained
select that shows true when you expected false?  Or even a DO block with no
dynamic SQL?

    IF problem IS NOT NULL THEN


As noted in the docs, “not (composite is null)” is not equivalent to
“composite is not null”; you usually want the former.


> If the issue is documentation, the PL/pgSQL docs should link to the
> composite type docs, or the expression docs, or type conversion, or where
> ever the current documentation is that I can't find or that needs to be
> written.
>
>
The behavior involved is in no way specific to pl/pgsql; it doesn’t seem
warranted to choose to link to SQL documentation from pl/pgsql generally.
The writer of such code can reference the SQL docs for any SQL they need to
write.

David J.

  3. Re: BUG #18190: PL/pgSQL does not document comparison operations on RECORD (or ROW) types

    Karl O. Pinc <kop@karlpinc.com> — 2023-11-12T15:22:44Z

    Hi David,

Thanks for your thoughtful reply.

tldr; My complaint is spurious.  Maybe the docs could
be further improved in some way anyway.

On Sat, 11 Nov 2023 18:13:35 -0700
"David G. Johnston" <david.g.johnston@gmail.com> wrote:

> On Friday, November 10, 2023, PG Bug reporting form
> <noreply@postgresql.org> wrote:

> > Nothing is said about how to compare entire RECORD type variables,
> > or ROW type variables.  
> 
> 
> There is, in the chapters with the word compare in their titles.
> 
> https://www.postgresql.org/docs/current/functions-comparison.html#FUNCTIONS-COMPARISON
> 
> https://www.postgresql.org/docs/current/functions-comparisons.html#FUNCTIONS-COMPARISONS

It turns out I was not having a problem comparing composite variables
with composite variables but composite variables with NULL.
As you point out below.

> You couldn’t make something less difficult to read?  Like a
> self-contained select that shows true when you expected false?  Or
> even a DO block with no dynamic SQL?

Apologies.  I was excited about showing a (somewhat) practical example
of where I got surprised.  I didn't stop to think about simpler ways
to assign a RECORD variable different values to come up with the
comparison operation which was then true focus of the issue.  I'd been
looking at the code for some time and stripping it to the simplest
test of an EXECUTE result I could come up with since that was where
I was having a problem.  When I finally figured out the problem was
with the comparison the code seemed simple and I didn't think to
strip out the EXECUTE.

>     IF problem IS NOT NULL THEN
> 
> 
> As noted in the docs, “not (composite is null)” is not equivalent to
> “composite is not null”; you usually want the former.

Are you referring to here
https://www.postgresql.org/docs/current/functions-comparison.html#FUNCTIONS-COMPARISON
where it says:

"If the expression is row-valued, then IS NULL is true when the row
expression itself is null or when all the row's fields are null, while IS NOT NULL is true when the row expression itself is non-null and all the row's fields are non-null. Because of this behavior, IS NULL and IS NOT NULL do not always return inverse results for row-valued expressions; in particular, a row-valued expression that contains both null and non-null fields will return false for both tests. In some cases, it may be preferable to write row IS DISTINCT FROM NULL or row IS NOT DISTINCT FROM NULL, which will simply check whether the overall row value is null without any additional tests on the row fields."

The above is what I should have been reading.  I believe what
I would want (if my particular application wasn't better suited to
GET DIAGNOSITCS) is the IS DISTINCT FROM NULL construct.

I do see a very minor problem here, in that the text writes about
"row expressions" not composite data types.  The problem being that
it's not entirely clear (to me) that a RECORD type is a row expression,
but it is clearly a composite.  Or so it seems to me.

> The behavior involved is in no way specific to pl/pgsql; it doesn’t
> seem warranted to choose to link to SQL documentation from pl/pgsql
> generally. The writer of such code can reference the SQL docs for any
> SQL they need to write.

FWIW, I spent a long time looking for the relevant part of the docs
and for some reason didn't find it.  This is unusual.  I usually
find the organization of the docs very easy to navigate.  I don't
have a good explanation of what went wrong but spent enough time at
it that filing a bug report seemed worthwhile.

Perhaps I'm too used to reading the PL/pgSQL chapter, which
makes it sound like RECORD variables are strictly a PL/pgSQL
construct.  (And maybe they are.)  It's unclear why this would matter,
and how such a thought would throw off my searching.  But something
did.  

(FWIW, The sentence "Note that RECORD is not a true data type,
only a placeholder." is pretty mysterious.  I don't see anything
that informs me about what a RECORD variable is, composite,
ROW type, or whatever.)


You can close this bug, or whatever else makes sense to you.

Thank you for your attention.  I hope the above is in some way
useful and the whole process is not a complete waste of your time.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  4. Re: BUG #18190: PL/pgSQL does not document comparison operations on RECORD (or ROW) types

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-11-12T15:53:34Z

    "Karl O. Pinc" <kop@karlpinc.com> writes:
> Perhaps I'm too used to reading the PL/pgSQL chapter, which
> makes it sound like RECORD variables are strictly a PL/pgSQL
> construct.  (And maybe they are.)  It's unclear why this would matter,
> and how such a thought would throw off my searching.  But something
> did.

Variables in general are a PL/pgSQL construct, but RECORD can be
used in other contexts; for example as the argument or return type
of a function, whether it's in PL/pgSQL or not.  (Some PLs may
reject that, but that's an implementation shortcoming of the PL.)
Also, the result of any ROW() constructor is of type RECORD unless
it's cast to some named composite type.

> (FWIW, The sentence "Note that RECORD is not a true data type,
> only a placeholder." is pretty mysterious.  I don't see anything
> that informs me about what a RECORD variable is, composite,
> ROW type, or whatever.)

I think you are on to something here.  I'm not very sure what we
should write instead of what's there, but I think that that sentence
predates our invention of polymorphic types.  It would likely be
better to frame RECORD as a polymorphic type that can range over
(only) composite types, and to provide cross-references to the
main docs' sections about composites (and maybe polymorphics).

Too caffeine-deprived to propose any specific wording, but
I think this area could use some work.

			regards, tom lane