Re: Change error code for hstore syntax error

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Sherrylyn Branchaw <sbranchaw(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Change error code for hstore syntax error
Date: 2016-03-11 19:49:06
Message-ID: 32686.1457725746@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Sherrylyn Branchaw <sbranchaw(at)gmail(dot)com> writes:
> The hstore module uses elog() to default to ERRCODE_INTERNAL_ERROR
> (SQLSTATE XX000) when the error message reads "Syntax error near '%c' at
> position %d".

Yeah, that is entirely bogus. No user-facing error report should ever
return ERRCODE_INTERNAL_ERROR.

> I propose to switch to ereport() to return ERRCODE_SYNTAX_ERROR (SQLSTATE
> 42601), on the grounds that it's more transparent.

Hm, class 42 is generally meant for SQL-level syntax errors. Are these
errors not coming from subroutines of hstore_in()? I think our usual
convention is to use ERRCODE_INVALID_TEXT_REPRESENTATION for complaints
that a data value does not meet its type's conventions. In any case
it seems closer to class 22 than 42.

While you're at it, please make the error message texts conform to
our style guidelines:
http://www.postgresql.org/docs/devel/static/error-style-guide.html

These seem to have multiple problems, starting with incorrect
capitalization and extending to failure to quote the whole string
being complained of.

In short, though, +1 for improving this stuff.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Salvador Fandiño 2016-03-11 20:19:21 Saving SRF context
Previous Message Sherrylyn Branchaw 2016-03-11 19:33:34 Change error code for hstore syntax error