Re: Proposed patch for error locations

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> My only comment is: Wouldn't it be clearer in the long run to create a
> ParseNode base structure which is used in place of NodeTag for
> parsenodes, and put the "int location" in there.

Only if we really wanted *every* parse node to carry a location, which
I think is overkill. If we took it far enough we'd be needing location
decoration on List nodes too, and that is definitely not happening.

There's a tradeoff here between completeness of information and
performance. In some quick tests with pgbench I don't detect any
noticeable performance loss with the patch as proposed, but it'd be
very easy to cross the threshold to where it does cost performance,
if we get too excited about decorating parse nodes with more than a
minimal amount of overhead.

> Finally, is the count in characters or bytes (w.r.t. multibyte
> encodings)?

Bytes; see the comments. Converting to a character count only happens
if an actual error message has to be emitted. (This is uglier than I
would like, but I don't see any sufficiently-cheap way to get the lexer
to count multibyte characters correctly.)

regards, tom lane