pgsql-server/src backend/utils/adt/numutils.c ...

momjian@postgresql.org (Bruce Momjian - CVS) writes:

Throw error on pg_atoi(''), regression adjustments.

Have you considered the backward compatibility implications of this
change?

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Well, not really. It is more of a tightening up, where '' is not really
zero, it is ''. Regression tests pass. Let's see how it flies during
beta. If we get pushback, then we can back out the patch, but we will
not know if we can tighten it unless we put it into the code.

---------------------------------------------------------------------------

Neil Conway wrote:

momjian@postgresql.org (Bruce Momjian - CVS) writes:

Throw error on pg_atoi(''), regression adjustments.

Have you considered the backward compatibility implications of this
change?

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Well, not really. It is more of a tightening up, where '' is not
really zero, it is ''.

Sure, but "tightening up" is just another way to say "backward
incompatible change". I'd agree this is a change we should make, I'm
just not convinced that it's worth the backward compatibility hit.

What if we just added an elog(WARNING) that said "Zero-length integer
conversion is deprecated and will be removed in the near future", and
then fixed this properly in 7.4? I usually dislike adding warnings,
but I don't think that noting in the documentation that this feature
is deprecated is sufficient in this case: information on this kind of
behavior is nearly impossible to find in the docs.

Cheers,

Neil

--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Neil Conway <neilc@samurai.com> writes:

momjian@postgresql.org (Bruce Momjian - CVS) writes:

Throw error on pg_atoi(''), regression adjustments.

Have you considered the backward compatibility implications of this
change?

Do you want to argue against it? If so, let's hear it ...

regards, tom lane

Neil Conway wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Well, not really. It is more of a tightening up, where '' is not
really zero, it is ''.

Sure, but "tightening up" is just another way to say "backward
incompatible change". I'd agree this is a change we should make, I'm
just not convinced that it's worth the backward compatibility hit.

What if we just added an elog(WARNING) that said "Zero-length integer
conversion is deprecated and will be removed in the near future", and
then fixed this properly in 7.4? I usually dislike adding warnings,
but I don't think that noting in the documentation that this feature
is deprecated is sufficient in this case: information on this kind of
behavior is nearly impossible to find in the docs.

I don't even think it was ever a feature. We are changing COPY's
handling of missing trailing fields, so this will fit right in. I
assume that will be the place it hits most.

Let's see if we get any pushback. We can always downgrade it to a
warning during beta.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I don't even think it was ever a feature.

It was certainly never documented as something that would work, unless
you chanced to read the one comment in numutils.c. A normal person
would not expect an empty string to be taken as a valid representation
of "zero".

Let's see if we get any pushback. We can always downgrade it to a
warning during beta.

I agree; tighten up and wait to see if anyone yowls.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I don't even think it was ever a feature.

It was certainly never documented as something that would work, unless
you chanced to read the one comment in numutils.c. A normal person
would not expect an empty string to be taken as a valid representation
of "zero".

Well, assuming we are matching atoi, it would make sense to return 0 for
'', I think.

Let's see if we get any pushback. We can always downgrade it to a
warning during beta.

I agree; tighten up and wait to see if anyone yowls.

Yes, we can loosen during beta, but not tighten.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

It was certainly never documented as something that would work, unless
you chanced to read the one comment in numutils.c. A normal person
would not expect an empty string to be taken as a valid representation
of "zero".

Well, assuming we are matching atoi, it would make sense to return 0 for
'', I think.

If we wanted to match atoi, we'd use atoi. pg_atoi exists to provide
more bulletproof behavior --- such as detecting overflow and invalid
input and erroring out when it happens.

regards, tom lane