Re: Better error message for select_common_type()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Better error message for select_common_type()
Date: 2008-03-17 23:19:25
Message-ID: 2099.1205795965@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Were there any objections to changing this patch so that it reports
> the second expression's parser location, instead of some arbitrary
> numbers? The way I'm envisioning doing it is:

> 1. Invent an exprLocation() function (comparable to, say, exprType)
> that knows how to get the parser location from any subtype of Node that
> has one.

> 2. Make a variant of select_common_type() that takes a list of Exprs
> instead of just type OIDs. It can get the type IDs from these
> using exprType(), and it can get their locations using exprLocation()
> if needed.

I started to look at this and immediately found out that the above
blithe sketch has nothing to do with reality. The problem is that
the current parser location mechanism stores locations only for
nodes that appear in raw grammar trees (gram.y output), *not* in
analyzed expressions (transformExpr output). This was an intentional
choice based on a couple of factors:

* Once we no longer have the parser input string available, the location
information would be just so much wasted space.

* It would add a weird special case to the equalfuncs.c routines:
should location fields be compared? (Probably not, but it seems a bit
unprincipled to ignore them.) And other places might have comparable
uncertainties what to do with 'em.

We'd need to either go back on that decision or pass in location
information separately to select_common_type. I think I prefer the
latter, but it's messier.

(On the third hand, you could make a case that including location
info in analyzed expressions makes it feasible to point at problems
detected at higher levels of analyze.c than just the first-level
transformExpr() call. select_common_type's problem could be seen
as just one aspect of what might be a widespread need.)

Another problem is that only a rather small subset of raw-grammar
expression node types actually carry locations at all. I had always
intended to go back and extend that, but it's not done yet. One reason
it's not done is that currently a lot of expression node types are used
for both raw-grammar output and analyzed expressions, which brings us
right back up against the issue above. I'd be inclined to fix that by
extending AExpr even more, and/or inventing an analogous raw-grammar
node type for things that take variable numbers of arguments, but
still it's more work.

So this is all eminently do-able but it seems too much to be tackling
during commit fest. I'd like to throw this item back on the TODO list.

Or we could apply Peter's patch more or less as-is, but I don't like
that. I don't think it solves the stated problem: if you know that CASE
branches 3 and 5 don't match, that still doesn't help you in a monster
query with lots of CASEs. I think we can and must do better.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gregory Stark 2008-03-18 00:23:06 Re: New style of hash join proposal
Previous Message Bruce Momjian 2008-03-17 22:52:52 Re: PG 7.3 is five years old today