Re: tighten input to float4/float8/oid - Mailing list pgsql-patches

From Tom Lane
Subject Re: tighten input to float4/float8/oid
Date
Msg-id 21040.1078373103@sss.pgh.pa.us
Whole thread Raw
In response to tighten input to float4/float8/oid  (Neil Conway <neilc@samurai.com>)
Responses Re: tighten input to float4/float8/oid  (Neil Conway <neilc@samurai.com>)
List pgsql-patches
Neil Conway <neilc@samurai.com> writes:
> - emit a warning when there is trailing whitespace in the input to the
> oid type, per suggestion from Tom (BTW, I couldn't see anything about
> this in the standard, although I may have been looking in the wrong
> place; in any case, rejecting trailing whitespace seems clearly
> correct behavior to me)

I think this is wrong.  We silently accept leading whitespace in
(IIRC) all the numeric datatypes, and I believe we should accept
trailing whitespace too.  The SQL spec does not speak anywhere that
I can see to the external representation of datatypes, but it has
plenty to say about the behavior of casts, and it seems reasonable
to me to consider the spec's description of casting to and from
character-string data as normative for I/O.  (Especially since we
tend to use the same code for both purposes.)  In SQL92 section
6.10 <cast specification> I find

         3) If TD is exact numeric, then
            ...
            b) If SD is character string, then SV is replaced by SV with any
              leading or trailing <space>s removed.
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

              Case:

              i) If SV does not comprise a <signed numeric literal> as
                 defined by the rules for <literal> in Subclause 5.3,
                 "<literal>", then an exception condition is raised: data
                 exception-invalid character value for cast.

             ii) Otherwise, let LT be that <signed numeric literal>. The
                 <cast specification> is equivalent to

                   CAST ( LT AS TD )

This looks to me like it's perfectly clear that the input can have
leading or trailing spaces, but not leading or trailing non-space
junk.

The spec means <space> to indicate the space character only, so
allowing other whitespace characters (as the existing code with
isspace() does) is perhaps a bit arguable.  I think it's reasonable
to allow any whitespace, though.

> +                  errdetail("this input will be rejected in "
> +                            "a future release of PostgreSQL")));

Minor stylistic gripe here: errdetail and errhint messages are
supposed to be complete sentences --- this is a deliberate departure
from the style for primary errmsg messages.  So the above should be

+                  errdetail("This input will be rejected in "
+                            "a future release of PostgreSQL.")));

            regards, tom lane

pgsql-patches by date:

Previous
From: Neil Conway
Date:
Subject: Re: minor doc improvements
Next
From: Tom Lane
Date:
Subject: Re: minor doc improvements