Horton

Thread

Commits

  1. Install errcodes.txt for use by extensions.

  2. Adjust input routines for float4, float8 and oid to reject the empty string

  3. Emit a warning when an empty string is input to the oid, float4, and

  1. Unrecognized exception condition "deprecated_feature"

    Kouber Saparev <kouber@gmail.com> — 2018-03-09T18:55:24Z

    For some reason using an error code of 'deprecated_feature' does not work,
while its equivalent '01P01' works just fine.

kouber=# DO $$
BEGIN
 RAISE WARNING 'Deprecated'
   USING ERRCODE = 'deprecated_feature';
END;
$$;
ERROR:  unrecognized exception condition "deprecated_feature"
CONTEXT:  PL/pgSQL function inline_code_block line 3 at RAISE

kouber=# DO $$
BEGIN
 RAISE WARNING 'Deprecated'
   USING ERRCODE = '01P01';
END;
$$;
WARNING:  Deprecated
DO

The thing that is making this issue quite strange is that I see that string
well defined in the source code (and copy/pasted it from there, as well as
from the docs, yet the result remains the same).

kouber@spectre:~/src/postgres$ grep -r deprecated_feature .
./src/backend/utils/errcodes.txt:01P01    W
 ERRCODE_WARNING_DEPRECATED_FEATURE
deprecated_feature

I am using PostgreSQL 10.1, I apologize in case it has already been fixed
in 10.2 or 10.3.

Regards,
--
Kouber Saparev

  2. Re: Unrecognized exception condition "deprecated_feature"

    David G. Johnston <david.g.johnston@gmail.com> — 2018-03-09T19:00:33Z

    On Fri, Mar 9, 2018 at 11:55 AM, Kouber Saparev <kouber@gmail.com> wrote:

> For some reason using an error code of 'deprecated_feature' does not work,
> while its equivalent '01P01' works just fine.
>
> kouber=# DO $$
> BEGIN
>  RAISE WARNING 'Deprecated'
>    USING ERRCODE = 'deprecated_feature';
> END;
> $$;
> ERROR:  unrecognized exception condition "deprecated_feature"
> CONTEXT:  PL/pgSQL function inline_code_block line 3 at RAISE
>
> kouber=# DO $$
> BEGIN
>  RAISE WARNING 'Deprecated'
>    USING ERRCODE = '01P01';
>
> ​[...]​

> kouber@spectre:~/src/postgres$ grep -r deprecated_feature .
>
> ./src/backend/utils/errcodes.txt:01P01    W    ERRCODE_WARNING_DEPRECATED_FEATURE
>                             deprecated_feature
>
> I am using PostgreSQL 10.1, I apologize in case it has already been fixed
> in 10.2 or 10.3.
>

​While I haven't looked at the source code or docs for this I'm wondering
why you believe this is a bug in the first place.  If "01P01" is a valid
and expected value for ERRCODE I would expect that the textual string label
representing it would not be.​  The two are not likely to be
interchangeable.  Have you found working code where this is does work as
you expect?

David J.

  3. Re: Unrecognized exception condition "deprecated_feature"

    Kouber Saparev <kouber@gmail.com> — 2018-03-09T22:28:48Z

    Here's a working example with 'unique_violation' for instance:

postgres=# DO $$
BEGIN
 RAISE WARNING 'Example of working exception condition'

   USING ERRCODE = 'unique_violation';
END;
$$;
WARNING:  Example of working exception condition
DO

postgres=# DO $$
BEGIN
 RAISE WARNING 'Example of working exception condition (with code)'
   USING ERRCODE = '23505';
END;
$$;
WARNING:  Example of working exception condition (with code)
DO

postgres=# select error_severity, message from sqlog.log('today') where
sql_state_code = '23505';
 error_severity |                      message
----------------+----------------------------------------------------
 WARNING        | Example of working exception condition
 WARNING        | Example of working exception condition (with code)
(2 rows)

2018-03-09 21:00 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:

> On Fri, Mar 9, 2018 at 11:55 AM, Kouber Saparev <kouber@gmail.com> wrote:
>
>> For some reason using an error code of 'deprecated_feature' does not
>> work, while its equivalent '01P01' works just fine.
>>
>> kouber=# DO $$
>> BEGIN
>>  RAISE WARNING 'Deprecated'
>>    USING ERRCODE = 'deprecated_feature';
>> END;
>> $$;
>> ERROR:  unrecognized exception condition "deprecated_feature"
>> CONTEXT:  PL/pgSQL function inline_code_block line 3 at RAISE
>>
>> kouber=# DO $$
>> BEGIN
>>  RAISE WARNING 'Deprecated'
>>    USING ERRCODE = '01P01';
>>
>> ​[...]​
>
>> kouber@spectre:~/src/postgres$ grep -r deprecated_feature .
>>
>> ./src/backend/utils/errcodes.txt:01P01    W
>>  ERRCODE_WARNING_DEPRECATED_FEATURE
>> deprecated_feature
>>
>> I am using PostgreSQL 10.1, I apologize in case it has already been fixed
>> in 10.2 or 10.3.
>>
>
> ​While I haven't looked at the source code or docs for this I'm wondering
> why you believe this is a bug in the first place.  If "01P01" is a valid
> and expected value for ERRCODE I would expect that the textual string label
> representing it would not be.​  The two are not likely to be
> interchangeable.  Have you found working code where this is does work as
> you expect?
>
> David J.
>
>

  4. Re: Unrecognized exception condition "deprecated_feature"

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-03-09T22:44:15Z

    "David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Fri, Mar 9, 2018 at 11:55 AM, Kouber Saparev <kouber@gmail.com> wrote:
>> For some reason using an error code of 'deprecated_feature' does not work,
>> while its equivalent '01P01' works just fine.

> While I haven't looked at the source code or docs for this I'm wondering
> why you believe this is a bug in the first place.

This isn't specific to ERRCODE_WARNING_DEPRECATED_FEATURE; if you look
at generate-plerrcodes.pl you'll see it only collects error codes, not
warning codes, for use in plpgsql's mapping table.  It's documented, too:
Appendix A says

    (Note that PL/pgSQL does not recognize warning, as opposed to error,
    condition names; those are classes 00, 01, and 02.)

I'm not entirely sure why we have this errcode at all, as it doesn't
seem to be used anywhere.

			regards, tom lane

  5. Re: Unrecognized exception condition "deprecated_feature"

    Kouber Saparev <kouber@gmail.com> — 2018-03-09T23:07:30Z

    2018-03-10 0:44 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

> This isn't specific to ERRCODE_WARNING_DEPRECATED_FEATURE; if you look
> at generate-plerrcodes.pl you'll see it only collects error codes, not
> warning codes, for use in plpgsql's mapping table.  It's documented, too:
> Appendix A says
>
>     (Note that PL/pgSQL does not recognize warning, as opposed to error,
>     condition names; those are classes 00, 01, and 02.)
>

Indeed.

next unless $type eq 'E';

The only exception is 'string_data_right_truncation' (01004) because
actually it is duplicated further (22001).

Thanks for the clarification!

--
Kouber Saparev

  6. Re: Unrecognized exception condition "deprecated_feature"

    Andres Freund <andres@anarazel.de> — 2018-03-10T00:17:48Z

    On 2018-03-09 17:44:15 -0500, Tom Lane wrote:
> "David G. Johnston" <david.g.johnston@gmail.com> writes:
> > On Fri, Mar 9, 2018 at 11:55 AM, Kouber Saparev <kouber@gmail.com> wrote:
> >> For some reason using an error code of 'deprecated_feature' does not work,
> >> while its equivalent '01P01' works just fine.
>
> > While I haven't looked at the source code or docs for this I'm wondering
> > why you believe this is a bug in the first place.
>
> This isn't specific to ERRCODE_WARNING_DEPRECATED_FEATURE; if you look
> at generate-plerrcodes.pl you'll see it only collects error codes, not
> warning codes, for use in plpgsql's mapping table.  It's documented, too:
> Appendix A says
>
>     (Note that PL/pgSQL does not recognize warning, as opposed to error,
>     condition names; those are classes 00, 01, and 02.)
>
> I'm not entirely sure why we have this errcode at all, as it doesn't
> seem to be used anywhere.

It used to be used till:

commit 975e27377aadcabab6504155c091d27ea2fa4c53
Author: Neil Conway <neilc@samurai.com>
Date:   2005-02-11 04:09:05 +0000

    Adjust input routines for float4, float8 and oid to reject the empty string
    as valid input (it was previously treated as 0). This input was deprecated
    in 8.0 (and a warning was emitted). Regression tests updated.

and was introduced in

commit 2146bfc869bfd4967b0bbf260f386344f02506b9
Author: Neil Conway <neilc@samurai.com>
Date:   2004-03-04 21:47:18 +0000

    Emit a warning when an empty string is input to the oid, float4, and
    float8 types. This begins the deprecation of this feature: in 7.6,
    this input will be rejected.

    Also added a new error code for warnings about deprecated features,
    and updated the regression tests.


there appears to never have been another user.


While it wasn't used for ~12 years, it seems like a useful
concept. OTOH, we've tried hard to avoid warnings like that due to the
log spam...

- Andres

  7. Re: Unrecognized exception condition "deprecated_feature"

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-03-10T00:42:26Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-03-09 17:44:15 -0500, Tom Lane wrote:
>> I'm not entirely sure why we have this errcode at all, as it doesn't
>> seem to be used anywhere.

> It used to be used till:
> ...
> While it wasn't used for ~12 years, it seems like a useful
> concept. OTOH, we've tried hard to avoid warnings like that due to the
> log spam...

Yeah, I'm not that excited about removing it.  Probably a more useful
thing to debate is whether we ought to change the policy about omitting
warning conditions from plpgsql's conversion list.  While the core code
hasn't got a use for that (since we never actually throw errors with such
SQLSTATEs), this complaint seems to indicate that users would like to use
them in RAISE.

(I'd still skip "success" conditions, like 00000, in any case.)

			regards, tom lane

  8. Re: Unrecognized exception condition "deprecated_feature"

    Andres Freund <andres@anarazel.de> — 2018-03-10T00:55:37Z

    On 2018-03-09 19:42:26 -0500, Tom Lane wrote:
> While the core code hasn't got a use for that (since we never actually
> throw errors with such SQLSTATEs), this complaint seems to indicate
> that users would like to use them in RAISE.

I think that's a fair argument, and given we don't duplicate the lists
anymore, the effort for doing so seems trivial enough.  +0.5

- Andres

  9. Re: Unrecognized exception condition "deprecated_feature"

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2018-03-10T02:12:39Z

    >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> Yeah, I'm not that excited about removing it. Probably a more
 Tom> useful thing to debate is whether we ought to change the policy
 Tom> about omitting warning conditions from plpgsql's conversion list.
 Tom> While the core code hasn't got a use for that (since we never
 Tom> actually throw errors with such SQLSTATEs), this complaint seems
 Tom> to indicate that users would like to use them in RAISE.

While we're on the topic, I know of at least one other author of a
non-core PL (besides myself) that has been frustrated by the fact that
errcodes.txt is not installed anywhere, making it impossible to
autogenerate a plerrcodes.h file in the extension build process.

-- 
Andrew (irc:RhodiumToad)

  10. Re: Unrecognized exception condition "deprecated_feature"

    Andres Freund <andres@anarazel.de> — 2018-03-10T02:19:25Z

    On 2018-03-10 02:12:39 +0000, Andrew Gierth wrote:
> >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
> 
>  Tom> Yeah, I'm not that excited about removing it. Probably a more
>  Tom> useful thing to debate is whether we ought to change the policy
>  Tom> about omitting warning conditions from plpgsql's conversion list.
>  Tom> While the core code hasn't got a use for that (since we never
>  Tom> actually throw errors with such SQLSTATEs), this complaint seems
>  Tom> to indicate that users would like to use them in RAISE.
> 
> While we're on the topic, I know of at least one other author of a
> non-core PL (besides myself) that has been frustrated by the fact that
> errcodes.txt is not installed anywhere, making it impossible to
> autogenerate a plerrcodes.h file in the extension build process.

Seems entirely reasonable to install it from my POV.

Greetings,

Andres Freund

  11. Re: Unrecognized exception condition "deprecated_feature"

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2018-03-10T02:24:18Z

    Andres Freund wrote:
> On 2018-03-10 02:12:39 +0000, Andrew Gierth wrote:

> > While we're on the topic, I know of at least one other author of a
> > non-core PL (besides myself) that has been frustrated by the fact that
> > errcodes.txt is not installed anywhere, making it impossible to
> > autogenerate a plerrcodes.h file in the extension build process.
> 
> Seems entirely reasonable to install it from my POV.

+1

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  12. Re: Unrecognized exception condition "deprecated_feature"

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-03-29T00:41:00Z

    On Sat, Mar 10, 2018 at 3:24 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Andres Freund wrote:
>> On 2018-03-10 02:12:39 +0000, Andrew Gierth wrote:
>
>> > While we're on the topic, I know of at least one other author of a
>> > non-core PL (besides myself) that has been frustrated by the fact that
>> > errcodes.txt is not installed anywhere, making it impossible to
>> > autogenerate a plerrcodes.h file in the extension build process.
>>
>> Seems entirely reasonable to install it from my POV.
>
> +1

+1

Something like this?  Or I guess src/backend/Makefile could do it directly.

I've wanted this for an ancient PL I've been trying to restore.

-- 
Thomas Munro
http://www.enterprisedb.com

  13. Re: Unrecognized exception condition "deprecated_feature"

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-03-29T01:22:42Z

    Thomas Munro <thomas.munro@enterprisedb.com> writes:
>>> While we're on the topic, I know of at least one other author of a
>>> non-core PL (besides myself) that has been frustrated by the fact that
>>> errcodes.txt is not installed anywhere, making it impossible to
>>> autogenerate a plerrcodes.h file in the extension build process.

> Something like this?  Or I guess src/backend/Makefile could do it directly.

Patch seems reasonable as far as it goes, but what about the MSVC
infrastructure?

Also, do we need to discuss exactly where it's being installed to?
Is the choice made here easy for an extension Makefile to locate?

			regards, tom lane

  14. Re: Unrecognized exception condition "deprecated_feature"

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-03-29T01:46:47Z

    On Thu, Mar 29, 2018 at 2:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
>>>> While we're on the topic, I know of at least one other author of a
>>>> non-core PL (besides myself) that has been frustrated by the fact that
>>>> errcodes.txt is not installed anywhere, making it impossible to
>>>> autogenerate a plerrcodes.h file in the extension build process.
>
>> Something like this?  Or I guess src/backend/Makefile could do it directly.
>
> Patch seems reasonable as far as it goes, but what about the MSVC
> infrastructure?

Hmm.  Here is an untested version that makes Install.pm treat it the
same way as it treats sql_features.txt.  Does this look right?

> Also, do we need to discuss exactly where it's being installed to?
> Is the choice made here easy for an extension Makefile to locate?

${datadir} is:

  --datadir=DIR           read-only architecture-independent data [DATAROOTDIR]

The default comes from here:

  --datarootdir=DIR       read-only arch.-independent data root [PREFIX/share]

That is obtainable with pg_config --sharedir.  It's a bit obscure that
--sharedir means ${datadir}, but in my tree I see:

src/port/Makefile:      echo "#define PGSHAREDIR \"$(datadir)\"" >>$@

That makes its way into pg_config's clutches via this code:

src/port/path.c-get_share_path(const char *my_exec_path, char *ret_path)
src/port/path.c-{
src/port/path.c:        make_relative_path(ret_path, PGSHAREDIR,
PGBINDIR, my_exec_path);
src/port/path.c-}

So after "make install" I can find the file like this:

$ ls -slap $(pg_config --sharedir)/errcodes.txt
64 -rw-r--r--  1 munro  staff  31450 29 Mar 14:40
/Users/munro/install/postgres/share/errcodes.txt

Would there be a better location?

-- 
Thomas Munro
http://www.enterprisedb.com