Re: Tablefunc crosstab error messages

Lists: pgsql-hackers
From: Mali Akmanalp <mali(at)akmanalp(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Tablefunc crosstab error messages
Date: 2012-08-28 00:07:02
Message-ID: CAFweTuNgJO3G=zk4fcnhckGQr=BmcRgkGkR-vgsr6BqJqE2E7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi All,

I was recently struggling with getting a large crosstab (
http://www.postgresql.org/docs/current/static/tablefunc.html) query to work
and noticed the error messages were not super helpful: "return and sql
tuple descriptions are incompatible".

I had to manually take the query into pieces and check each type, because
it wouldn't tell me which types were not matching. This is not fun
especially when you're using an ORM and composing queries piece by piece.
Reading into contrib/tablefunc/tablefunc.c, it looks like this shouldn't be
too hard to fix.

crosstab() calls compatCrosstabTupleDescs():

1545 /*

1546 * - attribute [1] of the sql tuple is the category; no need to
check it -
1547 * attribute [2] of the sql tuple should match attributes [1] to
[natts]
1548 * of the return tuple
1549 */
1550 sql_attr = sql_tupdesc->attrs[2];
1551 for (i = 1; i < ret_tupdesc->natts; i++)
1552 {
1553 ret_attr = ret_tupdesc->attrs[i];
1554
1555 if (ret_attr->atttypid != sql_attr->atttypid)
1556 return false;
1557 }

Returning the type information to the caller seems like a pain
but compatCrosstabTupleDescs already has instances in it where it fails
with an error message, so I propose we just do that and tell the user the
expected and actual types here, instead of returning false here and
throwing a generic error message in the caller.

What do you think? Let me know so I can write up a patch for you guys.

Also, just curious, is the reason the query passed into crosstab is a
string (rather than an SQL expression) that it's just easier to implement
that way?

Cheers,
~Mali


From: Noah Misch <noah(at)leadboat(dot)com>
To: Mali Akmanalp <mali(at)akmanalp(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Tablefunc crosstab error messages
Date: 2012-10-03 16:21:20
Message-ID: 20121003162120.GA23518@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Aug 27, 2012 at 08:07:02PM -0400, Mali Akmanalp wrote:
> Returning the type information to the caller seems like a pain
> but compatCrosstabTupleDescs already has instances in it where it fails
> with an error message, so I propose we just do that and tell the user the
> expected and actual types here, instead of returning false here and
> throwing a generic error message in the caller.
>
> What do you think? Let me know so I can write up a patch for you guys.

That would prove helpful, +1 from me.

> Also, just curious, is the reason the query passed into crosstab is a
> string (rather than an SQL expression) that it's just easier to implement
> that way?

Yes. Accepting the query as proper SQL syntax would require modifying the
backend grammar. Extensions cannot do that, but they can define functions
accepting a string and run it as a SQL command.