Re: Typing Records

Hackers,

I've been trying to come up with a simpler way to iterate over a series of values in pgTAP tests than by creating a table, inserting rows, and then selecting from the table. The best I've come up with so far is:

CREATE TYPE vcmp AS ( lv semver, op text, rv semver);

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2', '=', '1.2.2')::vcmp,
ROW('1.2.23', '=', '1.2.23')::vcmp
]);

Not bad, but I was hoping that I could cast all the rows at once, without the type, like so:

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2', '=', '1.2.2'),
ROW('1.2.23', '=', '1.2.23')
]) AS f(lv semver, op text, rv semver);

But this gives me an error (9.0b4):

psql:t/types.pg:205: ERROR: function return row and query-specified return row do not match
DETAIL: Returned type unknown at ordinal position 1, but query expects semver.

Pity it doesn't default to casting to text there. So I tried to just cast the first row:

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2'::semver, '='::text, '1.2.2'::semver),
ROW('1.2.23', '=', '1.2.23')
]) AS f(lv semver, op text, rv semver);

That's even worse!

psql:t/types.pg:205: ERROR: invalid memory alloc request size 18446744071604011012

Wha??

That seems like a bug.

Aside from that, might there be another way to do this without an explicit composite type? Maybe with VALUES() or something?

Thanks,

David

On 08/23/2010 08:33 PM, David E. Wheeler wrote:
> I've been trying to come up with a simpler way to iterate over a
> series of values in pgTAP tests than by creating a table, inserting
> rows, and then selecting from the table. The best I've come up with
> so far is:

<snip>

> Aside from that, might there be another way to do this without an
> explicit composite type? Maybe with VALUES() or something?

Maybe something like this?

select cmp_ok(a,b,c)
from
(
values('1.2.2'::varchar, '='::text, '1.2.2'::varchar),
('1.2.23', '=', '1.2.23'),
('1.2.42', '=', '1.2.32')
) as ss(a, b, c);
cmp_ok
--------
t
t
f
(3 rows)

HTH,

Joe

--
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> I've been trying to come up with a simpler way to iterate over a series of values in pgTAP tests than by creating a table, inserting rows, and then selecting from the table. The best I've come up with so far is:

> CREATE TYPE vcmp AS ( lv semver, op text, rv semver);

> SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
> ROW('1.2.2', '=', '1.2.2')::vcmp,
> ROW('1.2.23', '=', '1.2.23')::vcmp
> ]);

> Not bad, but I was hoping that I could cast all the rows at once,

You could do it like this:

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2', '=', '1.2.2'),
ROW('1.2.23', '=', '1.2.23')
]::vcmp[]);

> psql:t/types.pg:205: ERROR: invalid memory alloc request size 18446744071604011012
> Wha??
> That seems like a bug.

I get a core dump on that one ... looking ...

regards, tom lane

On Aug 23, 2010, at 11:24 PM, Joe Conway wrote:

> Maybe something like this?
>
> select cmp_ok(a,b,c)
> from
> (
> values('1.2.2'::varchar, '='::text, '1.2.2'::varchar),
> ('1.2.23', '=', '1.2.23'),
> ('1.2.42', '=', '1.2.32')
> ) as ss(a, b, c);
> cmp_ok
> --------
> t
> t
> f
> (3 rows)

Yes, exactly what I wanted. I knew I was missing something subtle (I had too many parens when I did it). Thanks Joe!

Best,

David

On Aug 24, 2010, at 7:05 AM, Tom Lane wrote:

> You could do it like this:
>
> SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
> ROW('1.2.2', '=', '1.2.2'),
> ROW('1.2.23', '=', '1.2.23')
> ]::vcmp[]);

Oh, duh. :-)

>> psql:t/types.pg:205: ERROR: invalid memory alloc request size 18446744071604011012
>> Wha??
>> That seems like a bug.
>
> I get a core dump on that one ... looking ...

Well I'm glad I reported it, then.

Best,

David

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> On Aug 24, 2010, at 7:05 AM, Tom Lane wrote:
>> I get a core dump on that one ... looking ...

> Well I'm glad I reported it, then.

The issue seems to be that given a construct like

ARRAY[
ROW('1.2.2'::semver, '='::text, '1.2.2'::semver),
ROW('1.2.23', '=', '1.2.23')
]

the parser is satisfied upon finding that all the array elements are
of type RECORD. It doesn't do anything to make sure they are all of
the *same* anonymous record type ... and here they are not. The
second one is just RECORD(UNKNOWN, UNKNOWN, UNKNOWN), which doesn't
even have a compatible representation with the first one. So at runtime
we end up trying to disassemble a tuple containing three UNKNOWN fields
using a tupledesc for the other rowtype.

I think it wouldn't take too much code to defend against this in
transformArrayExpr, but I'm a tad worried about whether there are
similar cases elsewhere. The generic problem is that we suppose that
different values are compatible if they have the same type OID, but
for RECORD types that's really not true.

regards, tom lane

On Tue, Aug 24, 2010 at 11:32 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I think it wouldn't take too much code to defend against this in
> transformArrayExpr, but I'm a tad worried about whether there are
> similar cases elsewhere.  The generic problem is that we suppose that
> different values are compatible if they have the same type OID, but
> for RECORD types that's really not true.

We've argued about this before: it's not really true for array types
either. A one-dimensional array is not the same type as a
two-dimensional array, but we treat it that way because bloating
pg_type by a factor of seven is even less appealing than bloating it
by a factor of two. And then there are other kinds of types people
might want to define: hashes, sets, functions, etc. This shoe is
going to rub for so long as we keep wearing it.

Nevertheless, there's not much hope of better than a localized fix for
this particular bug.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Aug 24, 2010 at 11:32 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I think it wouldn't take too much code to defend against this in
>> transformArrayExpr, but I'm a tad worried about whether there are
>> similar cases elsewhere. The generic problem is that we suppose that
>> different values are compatible if they have the same type OID, but
>> for RECORD types that's really not true.

> We've argued about this before: it's not really true for array types
> either.

Yeah, I'm starting to feel that the typmod hack for this is just not
good enough. However ...

> Nevertheless, there's not much hope of better than a localized fix for
> this particular bug.

This is a crash in released branches, so we have to have a
back-patchable fix. Anything that gets out from under the typmod issue
isn't going to be back-patchable.

regards, tom lane

On Tue, Aug 24, 2010 at 12:27 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> This is a crash in released branches, so we have to have a
> back-patchable fix.  Anything that gets out from under the typmod issue
> isn't going to be back-patchable.

I nominate that comment for "understatement of the year".

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

I wrote:
> The issue seems to be that given a construct like

> ARRAY[
> ROW('1.2.2'::semver, '='::text, '1.2.2'::semver),
> ROW('1.2.23', '=', '1.2.23')
> ]

> the parser is satisfied upon finding that all the array elements are
> of type RECORD. It doesn't do anything to make sure they are all of
> the *same* anonymous record type ... and here they are not. The
> second one is just RECORD(UNKNOWN, UNKNOWN, UNKNOWN), which doesn't
> even have a compatible representation with the first one. So at runtime
> we end up trying to disassemble a tuple containing three UNKNOWN fields
> using a tupledesc for the other rowtype.

> I think it wouldn't take too much code to defend against this in
> transformArrayExpr, but I'm a tad worried about whether there are
> similar cases elsewhere. The generic problem is that we suppose that
> different values are compatible if they have the same type OID, but
> for RECORD types that's really not true.

On reflection, I think that the current system design for this is
predicated on the theory that RECORDs really are all the same type, and
the executor had better be prepared to cope with a series of RECORDs
that have different tupdescs, or throw error if it cannot. We can see
that theory at work in record_out() for example. On that theory, the
blame for this crash should be pinned on ExecMakeTableFunctionResult(),
which is assuming that all records returned by the SRF will have the
same typmod. It ought to check that instead of just assuming.

I like this theory mainly because it leads to a back-patchable fix in
just one place. Trying to get the parser to enforce safety at parse
time was turning into a horrendous mess :-(

regards, tom lane

On Aug 26, 2010, at 9:05 AM, Tom Lane wrote:

> On reflection, I think that the current system design for this is
> predicated on the theory that RECORDs really are all the same type, and
> the executor had better be prepared to cope with a series of RECORDs
> that have different tupdescs, or throw error if it cannot. We can see
> that theory at work in record_out() for example. On that theory, the
> blame for this crash should be pinned on ExecMakeTableFunctionResult(),
> which is assuming that all records returned by the SRF will have the
> same typmod. It ought to check that instead of just assuming.
>
> I like this theory mainly because it leads to a back-patchable fix in
> just one place. Trying to get the parser to enforce safety at parse
> time was turning into a horrendous mess :-(

Sorry for the pain, Tom.

David