WIP patch: add parser location info to most expression node types

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: WIP patch: add parser location info to most expression node types
Date: 2008-08-28 17:59:59
Message-ID: 25677.1219946399@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I've been working away at the project I mentioned here:
http://archives.postgresql.org/pgsql-hackers/2008-08/msg01131.php
Attached is a patch that's not quite ready to commit, but getting
close. Basically it implements the infrastructure of adding a location
field to nearly all Node types that can be found in expression trees.

I concluded that there was no value in attaching locations to
statement-level nodes: it'd pretty much always be "column 1", with the
possible exception of sub-SELECTs, and for those the load is carried
by the parent SubLink node. Left undone for now is attaching location
to FROM-tree nodes such as RangeVar and JoinExpr; this might be useful
but I haven't worked out exactly what we'd do with it. (It's possible
that what really could do with location info is Alias nodes. In any
case it'd probably be best to land the WITH patch before touching that
part of the code much.) Also undone is attaching location to node types
used only in utility commands, such as ColumnDef; I'm not sure how
important that is either.

So far, as you can see, the patch doesn't affect the regression test
expected outputs very much. This is partly because the tests don't
actually spend much effort on testing parse-analysis error responses,
and partly because I haven't made a concerted effort to add
parser_errposition calls to all the ereport's that could now usefully
have one. However the latter work can be done incrementally once the
infrastructure is in place, so I'd like to commit this in something
close to its current state and then make a separate pass over the
error reports.

The patch does already make for a noticeable improvement in our ability
to localize problems with implicit coercions, eg

regression=# select (1 < 2) and 3;
ERROR: argument of AND must be type boolean, not type integer
LINE 1: select (1 < 2) and 3;
^
regression=#

One thing I do intend to change before committing is to revise
select_common_type and related routines so that we can point at the
specific subexpression that's causing an error, as per Peter's attempted
patch that started off this whole project.

Comments, objections?

regards, tom lane

Attachment Content-Type Size
more-parser-locations-1.patch.gz application/octet-stream 31.6 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2008-08-28 18:21:41 Re: A smaller default postgresql.conf
Previous Message Alvaro Herrera 2008-08-28 17:08:58 [patch] GUC source file and line number