Re: [PATCH] bugfix for int2vectorin

From: Caleb Welton <cwelton(at)greenplum(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] bugfix for int2vectorin
Date: 2009-12-02 19:59:54
Message-ID: C73C073B.3D4C%cwelton@greenplum.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

New patch attached:

1. Does not add a new error message (though the pg_atoi's error message is a little goofy looking).
2. Handles int2 overflow cases.
3. oidvectorin does NOT suffer from the same problems as int2vectorin, someone already fixed it.

As for the use-case I'm not completely sure... I'm not an end-user, I'm just responding to a bug report.

My stance here is that returning an error (even a bad error) on trying to convert data in is better
doing something "wrong" with bogus input. In the first case a user scratches their head, maybe
files a bug report, you tell them the correct syntax and they go on. In the second case they input
a bunch of data and then start complaining about "data corruption", "loss of data", etc. and the
support case is 100x worse.

The amount of code we are talking about here is less than 5 lines of code...

Regards,
Caleb

On 12/1/09 9:24 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

Caleb Welton <cwelton(at)greenplum(dot)com> writes:
> On 12/1/09 7:38 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Under what circumstances would users (or anyone at all) be putting data into an int2vector?

> What exactly is your objection to having the int2arrayin parser handle its input conversion reasonably?

I'm trying to gauge what the actual use-case is for having a slightly
nicer error behavior. The proposed patch adds another translatable
error string, which is no skin off my own nose but does create ongoing
work for our translation team. And presumably, if we're going to fix
this, we ought to fix the about-equally-stupid parsing logic in oidvectorin.
While we're at it, should we trouble to detect overflow in int2vectorin?
You could spend quite a bit of time and code making these functions more
bulletproof, but I'm not convinced it's worth any work.

regards, tom lane

Attachment Content-Type Size
int2vector.patch application/octet-stream 1.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2009-12-02 20:18:35 Re: Page-level version upgrade (was: Block-level CRC checks)
Previous Message Joshua D. Drake 2009-12-02 19:57:04 Re: YAML Was: CommitFest status/management