Re: tighten input to float4/float8/oid

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: tighten input to float4/float8/oid
Date: 2004-03-04 04:05:03
Message-ID: 21040.1078373103@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)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

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2004-03-04 04:07:07 Re: minor doc improvements
Previous Message Neil Conway 2004-03-04 02:32:12 Re: minor doc improvements