Re: Better error message for select_common_type() - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: Better error message for select_common_type() |
Date | |
Msg-id | 2099.1205795965@sss.pgh.pa.us Whole thread Raw |
In response to | Re: Better error message for select_common_type() (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Better error message for select_common_type()
Re: Better error message for select_common_type() |
List | 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
pgsql-hackers by date: