Thread: trouble caused by change in 7.3 handling of '' in integer context
I see in the 7.3 HISTORY file this:
* An empty string ('') is no longer allowed as the input into an
integer field. Formerly, it was silently interpreted as 0.
This is causing major issues with the Request Tracker program
<http://www.bestpractical.com/rt>.
The author has gone so far as to notify all current users that
Postgres 7.3 is not usable with RT, and to stick to 7.2, or to use
MySQL.
Apparently, there is a lot of work that will be necessary to retrofit
RT to this change. Is there any way possible to make this a logged
warning rather than a fatal so that there is time to transition the RT
code? I didn't see any notice in any prior release of the upcoming
change.
This unfortunately locks me into the 7.2 line until (if) RT can be
fixed up, and I could really use the 7.3 improvements for my other
applications.
Thanks.
--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Vivek Khera, Ph.D. Khera Communications, Inc.
Internet: khera@kciLink.com Rockville, MD +1-240-453-8497
AIM: vivekkhera Y!: vivek_khera http://www.khera.org/~vivek/
That explains why my RT started acting wierd (it's in test).
This also breaks PHPGroupware as well. (I've reported it to them).
Is there any way to get this to be a postgresql.conf option for transition?
LER
--On Wednesday, December 18, 2002 16:06:25 -0500 Vivek Khera
<khera@kcilink.com> wrote:
> I see in the 7.3 HISTORY file this:
>
> * An empty string ('') is no longer allowed as the input into an
> integer field. Formerly, it was silently interpreted as 0.
>
> This is causing major issues with the Request Tracker program
> <http://www.bestpractical.com/rt>.
>
> The author has gone so far as to notify all current users that
> Postgres 7.3 is not usable with RT, and to stick to 7.2, or to use
> MySQL.
>
> Apparently, there is a lot of work that will be necessary to retrofit
> RT to this change. Is there any way possible to make this a logged
> warning rather than a fatal so that there is time to transition the RT
> code? I didn't see any notice in any prior release of the upcoming
> change.
>
> This unfortunately locks me into the 7.2 line until (if) RT can be
> fixed up, and I could really use the 7.3 improvements for my other
> applications.
>
> Thanks.
>
> --
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> Vivek Khera, Ph.D. Khera Communications, Inc.
> Internet: khera@kciLink.com Rockville, MD +1-240-453-8497
> AIM: vivekkhera Y!: vivek_khera http://www.khera.org/~vivek/
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
>
--
Larry Rosenman http://www.lerctr.org/~ler
Phone: +1 972-414-9812 E-Mail: ler@lerctr.org
US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
On 18 Dec 2002, Vivek Khera wrote:
> I see in the 7.3 HISTORY file this:
>
> * An empty string ('') is no longer allowed as the input into an
> integer field. Formerly, it was silently interpreted as 0.
>
> This is causing major issues with the Request Tracker program
> <http://www.bestpractical.com/rt>.
>
> The author has gone so far as to notify all current users that
> Postgres 7.3 is not usable with RT, and to stick to 7.2, or to use
> MySQL.
This is backwards. RT isn't usable with Postgresql 7.3. There's a SQL
standard (several actually, but 92/99 are mostly it) . If RT had been
programmed to them, it wouldn't even notice this change. I know that
sounds cold hearted, but it isn't Postgresql's job to work with
noncompliant SQL.
> Apparently, there is a lot of work that will be necessary to retrofit
> RT to this change. Is there any way possible to make this a logged
> warning rather than a fatal so that there is time to transition the RT
> code? I didn't see any notice in any prior release of the upcoming
> change.
the problem is that it is not allowed by spec, and allowing it is against
spec, so it is likely that if it was implemented, it would have to be
similar to the transform_null_equals switch which is off by default.
> This unfortunately locks me into the 7.2 line until (if) RT can be
> fixed up, and I could really use the 7.3 improvements for my other
> applications.
Please be clear here. The problem is with RT, not Postgresql. I know
that doesn't comfort you personally, but it's an important point. If RT
wants to be portable, it's the kind of thing they need to fix, as well as
probably many more things in their SQL code.
Maybe now would be a good time for a code review of their SQL to see what
other possible non-sql spec problems are lurking there.
I'm downloading it now, and I'll take a look at the SQL code in it to see
what can be done.
Oh, that's bad. We tightened up the handling of '' as 0 because we
considered it too error-prone to assume they mean zero when they pass
''.
While we do like the new tightness, we don't want to make things harder
for people porting the code.
How about if I give you a patch against 7.3 that allows '' as 0, and you
ask if the author can distribute it and get his code changed for 7.4?
---------------------------------------------------------------------------
Vivek Khera wrote:
> I see in the 7.3 HISTORY file this:
>
> * An empty string ('') is no longer allowed as the input into an
> integer field. Formerly, it was silently interpreted as 0.
>
> This is causing major issues with the Request Tracker program
> <http://www.bestpractical.com/rt>.
>
> The author has gone so far as to notify all current users that
> Postgres 7.3 is not usable with RT, and to stick to 7.2, or to use
> MySQL.
>
> Apparently, there is a lot of work that will be necessary to retrofit
> RT to this change. Is there any way possible to make this a logged
> warning rather than a fatal so that there is time to transition the RT
> code? I didn't see any notice in any prior release of the upcoming
> change.
>
> This unfortunately locks me into the 7.2 line until (if) RT can be
> fixed up, and I could really use the 7.3 improvements for my other
> applications.
>
> Thanks.
>
> --
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> Vivek Khera, Ph.D. Khera Communications, Inc.
> Internet: khera@kciLink.com Rockville, MD +1-240-453-8497
> AIM: vivekkhera Y!: vivek_khera http://www.khera.org/~vivek/
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
>
--
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
Larry Rosenman wrote: > That explains why my RT started acting wierd (it's in test). > > This also breaks PHPGroupware as well. (I've reported it to them). > > Is there any way to get this to be a postgresql.conf option for transition? Oh, wow, we have two now. We did discuss the problems of backward compatibility, but didn't think it would effect many people. Folks, what do you want to do? -- 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
On Wed, 18 Dec 2002, Bruce Momjian wrote: > Larry Rosenman wrote: > > That explains why my RT started acting wierd (it's in test). > > > > This also breaks PHPGroupware as well. (I've reported it to them). > > > > Is there any way to get this to be a postgresql.conf option for transition? > > Oh, wow, we have two now. We did discuss the problems of backward > compatibility, but didn't think it would effect many people. > > Folks, what do you want to do? Make it a GUC setting like transform_null_equals that is defaulted to false and the dba has to turn on to enable it. Best of both worlds.
That would be nice, and per user/db if possible? --On Wednesday, December 18, 2002 15:51:39 -0700 "scott.marlowe" <scott.marlowe@ihs.com> wrote: > On Wed, 18 Dec 2002, Bruce Momjian wrote: > >> Larry Rosenman wrote: >> > That explains why my RT started acting wierd (it's in test). >> > >> > This also breaks PHPGroupware as well. (I've reported it to them). >> > >> > Is there any way to get this to be a postgresql.conf option for >> > transition? >> >> Oh, wow, we have two now. We did discuss the problems of backward >> compatibility, but didn't think it would effect many people. >> >> Folks, what do you want to do? > > Make it a GUC setting like transform_null_equals that is defaulted to > false and the dba has to turn on to enable it. Best of both worlds. -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
These incompatibilities have been caused by better compliance with standards, right? If
this is the case then I don't think you want to go backwards. Indeed - as a software developer, I
always test my code on pending betas just to see what's going to change before a major release
gets to the general public so my app will already be compliant. To be competitive with the major
RDBMS' this kind of progress needs to be made.
If the postgres developers want to put app developers on notice perhaps they can adopt
a deprecation policy where whenever a feature is going to be removed or changed in a manner
that breaks existing code the following steps should be taken:
1. next release announce it as deprecated and produce a warning message whenever
it's used. if it's major, possible provide a compile option --with<newfun/behaviour> so users can test
it.
2. the following release remove or make the change. if it's major, possibly provide a
compile option --with-<oldfunc/behaviour> so users can temporarily keep the old behaviour.
3. the following release - its done/gone/never to return again
Any app developer/user who can't follow this stable plan isn't interested in a better
postgres anyway.
best regards,
Ben Scherrey
PS: FWIW - it was a bit annoying for "where x = null" to break silently for my client's code. A
deprecation warning/error would have made their app debugging very quick rather than wondering
why their database broke (when it didn't).
12/18/2002 5:28:37 PM, Bruce Momjian <pgman@candle.pha.pa.us> wrote:
>Larry Rosenman wrote:
>> That explains why my RT started acting wierd (it's in test).
>>
>> This also breaks PHPGroupware as well. (I've reported it to them).
>>
>> Is there any way to get this to be a postgresql.conf option for transition?
>
>Oh, wow, we have two now. We did discuss the problems of backward
>compatibility, but didn't think it would effect many people.
>
>Folks, what do you want to do?
>
>--
> 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
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
>subscribe-nomail command to majordomo@postgresql.org so that your
>message can get through to the mailing list cleanly
>
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Folks, what do you want to do?
Nothing. We are certainly not going to add such a switch to 7.3 at this
point (it's been feature-frozen for months, remember?) and by the time
7.4 is out it'll be moot, because the non-spec-compliant applications
will have been fixed.
I agree with Scott Marlowe's take on this: the correct solution is to
fix the applications.
regards, tom lane
Bruce Momjian writes: > Folks, what do you want to do? Make them fix their code. It's one thing to have warts for allowing the market-leading GUI frontend for databases (MS Access) to work, but if every, with all due respect, random client wants to have its own break-the-spec-for-me switch we'll go mad. Explain to me why it's so hard to say 0 when you mean 0. -- Peter Eisentraut peter_e@gmx.net
Quoting Peter Eisentraut <peter_e@gmx.net>: > Bruce Momjian writes: > > > Folks, what do you want to do? > > Make them fix their code. > > It's one thing to have warts for allowing the market-leading GUI frontend > for databases (MS Access) to work, but if every, with all due respect, > random client wants to have its own break-the-spec-for-me switch we'll go > mad. > > Explain to me why it's so hard to say 0 when you mean 0. because there is legacy code out there, that was ported from MySQL, and now it used to work with PostgreSQL and it doesn't now. RT is a MAJOR app..... the PHPGroupware stuff could also be major, depending on what people are using. I guess Vivek and I are just going to have to do without these 2 apps until/ unless they can be rev'd. I really wish this had been better thought out. I know I crabbed it during BETA, but was the first one. now it's too late. This stinks from a Marketing perspective. LER > > -- > Peter Eisentraut peter_e@gmx.net > -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
Vivek,
I'll not echo what others have said so far, and indeed what i've said
myself in a previous thread about this.
An important fact to take into account is that PostgreSQL 7.3 was
available as an open beta for quite a period - This period is both for
maturing changes made during the development and to allow client
authors to test against the new version.
Shouldn't at least ONE person who uses 'Request Tracker' have
perticipated in this beta program? PostgreSQL is community project
after all, you need to be proactive.
Indeed if someone involved with 'Request Tracker' read/skimmed the
hackers list then they could have discussed the implications before
the change was made. The PostgreSQL developers are great (thanks
guys!) but they are not all-seeing and all-knowing, they cannot
foresee all possible implications of a change.
Thanks, Lee.
Vivek Khera writes:
> I see in the 7.3 HISTORY file this:
>
> * An empty string ('') is no longer allowed as the input into an
> integer field. Formerly, it was silently interpreted as 0.
>
> This is causing major issues with the Request Tracker program
> <http://www.bestpractical.com/rt>.
>
> The author has gone so far as to notify all current users that
> Postgres 7.3 is not usable with RT, and to stick to 7.2, or to use
> MySQL.
>
> Apparently, there is a lot of work that will be necessary to retrofit
> RT to this change. Is there any way possible to make this a logged
> warning rather than a fatal so that there is time to transition the RT
> code? I didn't see any notice in any prior release of the upcoming
> change.
>
> This unfortunately locks me into the 7.2 line until (if) RT can be
> fixed up, and I could really use the 7.3 improvements for my other
> applications.
> Indeed if someone involved with 'Request Tracker' read/skimmed the > hackers list then they could have discussed the implications before > the change was made. The PostgreSQL developers are great (thanks > guys!) but they are not all-seeing and all-knowing, they cannot > foresee all possible implications of a change. I agree. Because I have been monitoring the list for some time, I knew of the change and I wasn't surprised (though I was unhappy) when I received the dreaded error message and my app exited. Self-inflicted bugs never make me happy. :) Shane
>>>>> "sm" == scott marlowe <scott.marlowe> writes: sm> Please be clear here. The problem is with RT, not Postgresql. I know sm> that doesn't comfort you personally, but it's an important point. If RT sm> wants to be portable, it's the kind of thing they need to fix, as well as sm> probably many more things in their SQL code. Yes, RT is at fault, but just making the change without a transition period is harmful. All I ask is to make it easier to transition broken apps.
>>>>> "BM" == Bruce Momjian <pgman@candle.pha.pa.us> writes: BM> How about if I give you a patch against 7.3 that allows '' as 0, and you BM> ask if the author can distribute it and get his code changed for 7.4? That would be great. Actually what would be best is if the code could log a warning (with the full query) every time it happened, then it would be easy to run the app for a while and find all the places it happens. RT dynamically creates its queries so this would be the easiest way to fix it up. Then RT could be fixed up and not need any patches to PG.
On Thu, 2002-12-19 at 09:54, Vivek Khera wrote: > >>>>> "sm" == scott marlowe <scott.marlowe> writes: > > sm> Please be clear here. The problem is with RT, not Postgresql. I know > sm> that doesn't comfort you personally, but it's an important point. If RT > sm> wants to be portable, it's the kind of thing they need to fix, as well as > sm> probably many more things in their SQL code. > > Yes, RT is at fault, but just making the change without a transition > period is harmful. All I ask is to make it easier to transition > broken apps. > The problem is that adding all of these special flags for different apps is unmanageable. Even if the flag was there, don't you suspect when the time came for 7.4 to be released we'd see another volley of emails asking for the exception to stick around for "just one more release"? The bottom line is that at some point your code is going to have to be modified, a transition period really just gives an excuse for procrastination. Bruce has offered a patch against 7.3 for those who cant wait to upgrade; I think that should be a sufficient remedy. Robert Treat
On Thu, 2002-12-19 at 10:03, Vivek Khera wrote: > >>>>> "BM" == Bruce Momjian <pgman@candle.pha.pa.us> writes: > > BM> How about if I give you a patch against 7.3 that allows '' as 0, and you > BM> ask if the author can distribute it and get his code changed for 7.4? > > That would be great. Actually what would be best is if the code could > log a warning (with the full query) every time it happened, then it > would be easy to run the app for a while and find all the places it > happens. RT dynamically creates its queries so this would be the > easiest way to fix it up. Then RT could be fixed up and not need any > patches to PG. > IIRC 7.3 has this functionality, i think the parameter is log_min_error_statement in the postgresql.conf Robert Treat
>>>>> "PE" == Peter Eisentraut <peter_e@gmx.net> writes: PE> It's one thing to have warts for allowing the market-leading GUI frontend PE> for databases (MS Access) to work, but if every, with all due respect, PE> random client wants to have its own break-the-spec-for-me switch we'll go PE> mad. PE> Explain to me why it's so hard to say 0 when you mean 0. It is not hard. What is hard is when you change behavior abruptly between releases without a transition period in which you warn about an upcoming change when that deprecated feature is used. Ideally, it would have logged this error for the 7.3 release, and disallowed it for 7.4, something akin to the LIMIT transition. I guess nobody realized how some popular applications used this non-compliant feature before the change was implemented. Perhaps we learn from this experience and don't do this type of change again. Perhaps not. -- =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Vivek Khera, Ph.D. Khera Communications, Inc. Internet: khera@kciLink.com Rockville, MD +1-240-453-8497 AIM: vivekkhera Y!: vivek_khera http://www.khera.org/~vivek/
Vivek Khera <khera@kcilink.com> writes: > It is not hard. What is hard is when you change behavior abruptly > between releases without a transition period in which you warn about > an upcoming change when that deprecated feature is used. Ideally, it > would have logged this error for the 7.3 release, and disallowed it > for 7.4, something akin to the LIMIT transition. > > I guess nobody realized how some popular applications used this > non-compliant feature before the change was implemented. Perhaps we > learn from this experience and don't do this type of change again. > Perhaps not. Yeah, I can personally see a case for deprecating nonstandard features and spitting out a warning, rather than dropping them abruptly. The deprecation would last for one major release cycle. On the other hand, if the nonstandard wart/feature is standing in the way of implementing a high-quality and desired new feature (not saying this did or didn't happen in this case; I don't know), is it worth putting off adding the new feature rather than just excising the wart and putting up with the pain? -Doug
Vivek Khera wrote: > >>>>> "PE" == Peter Eisentraut <peter_e@gmx.net> writes: > > PE> It's one thing to have warts for allowing the market-leading GUI frontend > PE> for databases (MS Access) to work, but if every, with all due respect, > PE> random client wants to have its own break-the-spec-for-me switch we'll go > PE> mad. > > PE> Explain to me why it's so hard to say 0 when you mean 0. > > It is not hard. What is hard is when you change behavior abruptly > between releases without a transition period in which you warn about > an upcoming change when that deprecated feature is used. Ideally, it > would have logged this error for the 7.3 release, and disallowed it > for 7.4, something akin to the LIMIT transition. > > I guess nobody realized how some popular applications used this > non-compliant feature before the change was implemented. Perhaps we > learn from this experience and don't do this type of change again. > Perhaps not. Yes, this was really it --- we didn't realize how many apps used this. Also, we expected to hit them before final release, when we could have reverted the change. The prompting of the change was that '' -> 0 didn't make sense, and it was masking some COPY file format errors. -- 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
--On Thursday, December 19, 2002 12:01:21 -0500 Bruce Momjian <pgman@candle.pha.pa.us> wrote: > Vivek Khera wrote: >> >>>>> "PE" == Peter Eisentraut <peter_e@gmx.net> writes: >> >> PE> It's one thing to have warts for allowing the market-leading GUI >> frontend PE> for databases (MS Access) to work, but if every, with all >> due respect, PE> random client wants to have its own >> break-the-spec-for-me switch we'll go PE> mad. >> >> PE> Explain to me why it's so hard to say 0 when you mean 0. >> >> It is not hard. What is hard is when you change behavior abruptly >> between releases without a transition period in which you warn about >> an upcoming change when that deprecated feature is used. Ideally, it >> would have logged this error for the 7.3 release, and disallowed it >> for 7.4, something akin to the LIMIT transition. >> >> I guess nobody realized how some popular applications used this >> non-compliant feature before the change was implemented. Perhaps we >> learn from this experience and don't do this type of change again. >> Perhaps not. > > Yes, this was really it --- we didn't realize how many apps used this. > Also, we expected to hit them before final release, when we could have > reverted the change. IT DID! (see my post during Beta). You said, well, that's only one. > > The prompting of the change was that '' -> 0 didn't make sense, and > it was masking some COPY file format errors. > > -- > 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 > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
On Thu, 19 Dec 2002, Vivek Khera wrote: > >>>>> "sm" == scott marlowe <scott.marlowe> writes: > > sm> Please be clear here. The problem is with RT, not Postgresql. I know > sm> that doesn't comfort you personally, but it's an important point. If RT > sm> wants to be portable, it's the kind of thing they need to fix, as well as > sm> probably many more things in their SQL code. > > Yes, RT is at fault, but just making the change without a transition > period is harmful. All I ask is to make it easier to transition > broken apps. Actually, I agree with you on this. I believe that when something like this is changed it should have an accompanying GUC var that is defaulted to false that the DBA has to turn on, and that var can then be removed in the next major version. I've looked through the RT source code, and there are only about two dozen instances of updates using '' in an insert, so it looks like it wouldn't be all that hard to fix.
Larry Rosenman wrote: > > Yes, this was really it --- we didn't realize how many apps used this. > > Also, we expected to hit them before final release, when we could have > > reverted the change. > IT DID! (see my post during Beta). > > You said, well, that's only one. Yes, I do remember that. We basically weigh the value of of such changes based on the value of unmasking bugs vs. causing pain for folks. What we easily could have done was add a GUC that could be turned off if you wanted the broken behavior, and keep it for one release cycle. We thought all the apps could be fixed during the beta period --- obviously we were wrong. -- 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
Larry Rosenman <ler@lerctr.org> writes:
> --On Thursday, December 19, 2002 12:01:21 -0500 Bruce Momjian
> <pgman@candle.pha.pa.us> wrote:
>> Yes, this was really it --- we didn't realize how many apps used this.
>> Also, we expected to hit them before final release, when we could have
>> reverted the change.
> IT DID! (see my post during Beta).
> You said, well, that's only one.
It's still only two. And I still haven't heard any cogent argument why
we shouldn't regard these apps as broken --- regardless of how many
there are. I cannot see any rational reason for accepting '' as meaning
zero in an integer field. If someone were to present an example where
this actually makes sense (and is not just evidence of very sloppy
programming) I'd be more sympathetic ...
BTW, I note that we seem to have missed a couple of cases: float4,
float8, and OID still accept '' as zero in CVS tip. On the other hand,
int8 and numeric never have accepted '' AFAIR.
Another thing we should probably look at doing is allowing trailing
spaces in all these input routines. Currently, leading spaces are
ok but trailing are not:
regression=# select ' 1'::int;
int4
------
1
(1 row)
regression=# select ' 1 '::int;
ERROR: pg_atoi: error in " 1 ": can't parse " "
It appears to me that SQL expects trailing spaces to be legal; for
example, the SQL99 rules for CASTing strings to numerics say
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 )
regards, tom lane
--On Thursday, December 19, 2002 12:22:18 -0500 Tom Lane <tgl@sss.pgh.pa.us> wrote: > Larry Rosenman <ler@lerctr.org> writes: >> --On Thursday, December 19, 2002 12:01:21 -0500 Bruce Momjian >> <pgman@candle.pha.pa.us> wrote: >>> Yes, this was really it --- we didn't realize how many apps used this. >>> Also, we expected to hit them before final release, when we could have >>> reverted the change. > >> IT DID! (see my post during Beta). > >> You said, well, that's only one. > > It's still only two. And I still haven't heard any cogent argument why > we shouldn't regard these apps as broken --- regardless of how many > there are. I cannot see any rational reason for accepting '' as meaning > zero in an integer field. If someone were to present an example where > this actually makes sense (and is not just evidence of very sloppy > programming) I'd be more sympathetic ... The issue isn't whether the app is broken or not, (it is), but the fact that we ****CHANGED**** behavior with ****NO**** notice and no way to keep the app working on the current release without MAJOR changes to the app. (some apps just have it so ingrained in them). We ***SHOULD*** have noticed the user community, and had a transition plan. This should be a lesson to us to be more sensitive to applications out there, and give NOTICE about compatibility issues. [snip] I understand your point, Tom, but this does not help PostgreSQL's acceptability in the marketplace. LER -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
Larry Rosenman <ler@lerctr.org> writes:
> The issue isn't whether the app is broken or not, (it is), but the
> fact that we ****CHANGED**** behavior with ****NO**** notice
There was notice, to them as were paying attention (eg, anyone who tried
a beta release). More to the point, I can name half a dozen other
incompatible changes in 7.3 (most, but not all, forced by schema
support) that will undoubtedly break some apps. We cannot have a
transition plan and backwards-compatibility kluge for every little
compatibility issue, or our progress will grind to a standstill.
According to the start of this thread, RT's author has recommended that
RT users stick to PG 7.2 until he can fix the app. That seems a
perfectly sensible answer to me.
regards, tom lane
Tom Lane wrote: > Larry Rosenman <ler@lerctr.org> writes: > > The issue isn't whether the app is broken or not, (it is), but the > > fact that we ****CHANGED**** behavior with ****NO**** notice > > There was notice, to them as were paying attention (eg, anyone who tried > a beta release). More to the point, I can name half a dozen other > incompatible changes in 7.3 (most, but not all, forced by schema > support) that will undoubtedly break some apps. We cannot have a > transition plan and backwards-compatibility kluge for every little > compatibility issue, or our progress will grind to a standstill. That is a good point. Though we knew about the atoi '' potential incompatibility, there are certainly others, some of which we weren't even aware of. (We just got SSL fixed in 7.3.1.) So, while we can do GUC params for some of the big changes, we still aren't going to be able to hit them all. > According to the start of this thread, RT's author has recommended that > RT users stick to PG 7.2 until he can fix the app. That seems a > perfectly sensible answer to me. I have a patch I will post now. -- 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
Vivek Khera wrote:
> >>>>> "BM" == Bruce Momjian <pgman@candle.pha.pa.us> writes:
>
> BM> How about if I give you a patch against 7.3 that allows '' as 0, and you
> BM> ask if the author can distribute it and get his code changed for 7.4?
>
> That would be great. Actually what would be best is if the code could
> log a warning (with the full query) every time it happened, then it
> would be easy to run the app for a while and find all the places it
> happens. RT dynamically creates its queries so this would be the
> easiest way to fix it up. Then RT could be fixed up and not need any
> patches to PG.
OK, patch attached and tested:
test=> CREATE TABLE test(x int);
CREATE TABLE
test=> INSERT INTO test VALUES ('');
WARNING: pg_atoi: zero-length string
INSERT 140191 1
test=> SELECT x FROM test;
x
---
0
(1 row)
However, the regression tests will fail now because we explicitly test
for '' to generate an error.
I added my name, date, and purpose as a comment in the patched code.
--
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
Index: src/backend/utils/adt/numutils.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/adt/numutils.c,v
retrieving revision 1.54
diff -c -c -r1.54 numutils.c
*** src/backend/utils/adt/numutils.c 4 Sep 2002 20:31:28 -0000 1.54
--- src/backend/utils/adt/numutils.c 19 Dec 2002 17:10:56 -0000
***************
*** 70,76 ****
if (s == (char *) NULL)
elog(ERROR, "pg_atoi: NULL pointer");
else if (*s == 0)
! elog(ERROR, "pg_atoi: zero-length string");
else
l = strtol(s, &badp, 10);
--- 70,80 ----
if (s == (char *) NULL)
elog(ERROR, "pg_atoi: NULL pointer");
else if (*s == 0)
! {
! /* 7.3.X workaround for broken apps, bjm 2002-12-19 */
! elog(WARNING, "pg_atoi: zero-length string");
! l = (long) 0;
! }
else
l = strtol(s, &badp, 10);
Larry Rosenman wrote: > > The issue isn't whether the app is broken or not, (it is), but the fact > that we > ****CHANGED**** behavior with ****NO**** notice and no way to keep the > app working on > the current release without MAJOR changes to the app. (some apps just > have it so ingrained > in them). > > We ***SHOULD*** have noticed the user community, and had a transition plan. > > This should be a lesson to us to be more sensitive to applications out > there, and > give NOTICE about compatibility issues. > Isn't that what beta is for? - For app developers to test their apps against the new version to discover problems/issues before the production release. If this had been brought up during the beta period there may well have been a different outcome. In general it is difficult to determine whether any particular change will impact app developers. And this problem isn't unique to postgres. I can't remember a major Oracle upgrade that hasn't broken my applications in some way. The difference with Oracle is that I don't find out until the new release is production, whereas with postgres I ensure that I am testing with all the betas and getting any issues resolved before it goes production. --Barry
--On Thursday, December 19, 2002 09:57:15 -0800 Barry Lind <blind@xythos.com> wrote: > > Larry Rosenman wrote: > > > > The issue isn't whether the app is broken or not, (it is), but the fact > > that we > > ****CHANGED**** behavior with ****NO**** notice and no way to keep the > > app working on > > the current release without MAJOR changes to the app. (some apps just > > have it so ingrained > > in them). > > > > We ***SHOULD*** have noticed the user community, and had a transition > plan. > > > This should be a lesson to us to be more sensitive to applications out > > there, and > > give NOTICE about compatibility issues. > > > > Isn't that what beta is for? - For app developers to test their apps > against the new version to discover problems/issues before the production > release. If this had been brought up during the beta period there may > well have been a different outcome. I brought up at least ONE app that had issues during beta. and was told, "well, that's only one"....... > > In general it is difficult to determine whether any particular change > will impact app developers. And this problem isn't unique to postgres. > I can't remember a major Oracle upgrade that hasn't broken my > applications in some way. The difference with Oracle is that I don't > find out until the new release is production, whereas with postgres I > ensure that I am testing with all the betas and getting any issues > resolved before it goes production. I noticed some oddities with my TEST RT, but didn't tie them to the 7.3 change. So, there was at least notice during the beta, but I was told to go away (essentially). > > --Barry > -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
There is a similar problem with arrays (see examples below). Some comments:
sendmail was written with a philosophy of "accept as much as possible,
generate only that which is strictly standards compliant". A laudable goal
but one which has allowed people to write all sorts of crappy mail user
agents that "worked". The resulting proliferation of broken MUAs has led to
the requirement to add "compatibility" hacks not only to sendmail but to most
other mail transport programs as well. Fortunately many MTAs have
configuration options that allow you to turn these "features" on or off.
From that perspective it is preferable to get people to write
specification-compliant code. But lest anyone get too smug I would point out
that www.postgresql.org utterly fails the w3c html validator test. So much
for adherance to standards.
But what if there aren't standards...
PostgreSQL has many extensions (arrays for one) which are seemingly not fully
thought out. I can determine the current behavior by experiment and reading
the source code but I cannot find a specification for the feature to tell me
if the current release is working as the developers ultimately intend. I
don't want to code to some behavior that I've discovered but which is not
documented - down that road is only misery.
An example: I have asked on these groups several times about arrays and have
gotten no replies. Currently if I ask for an element beyond the end of an
array it will return a null. If I could find that to be the documented
specification for array behavior I could start working on my app. As it is I
can not because I may be creating code that will break with the next release
of PostgreSQL.
I am all the more concerned since with version 7.3 the interpretation of
arrays has become non-consistent. A skipped element in a text array gets a
default entry of '' while a skipped element in an int array, which used to
get a default entry of 0, now causes an error. So before we can chastize
someone for not following a specification there has to actually be one.
Here are the results from 7.3:
t=> create table foo (bar int[]);
CREATE TABLE
t=> insert into foo values ('{1,,3}');
ERROR: pg_atoi: zero-length string
t=> create table foo (bar text[]);
CREATE TABLE
t=> insert into foo values ('{"one",,"three"}');
INSERT 17047 1
t=> select * from foo;
bar
----------------
{one,"",three}
(1 row)
Cheers,
Steve
P.S. My vote for array behavior is to allow nulls within an array (as seems
from the docs to be the ultimate goal) and to continue to return a null from
any uninitialized array element. If selecting an element outside of an array
returns an error then it will be a pain to ask simple queries on variable
length arrays when you don't know in advance how many elements might exist in
each record. For example:
select count(*) from foo where bar[6] = 'snark';
-Steve
On Wednesday 18 December 2002 4:06 pm, Peter Eisentraut wrote:
> Bruce Momjian writes:
> > Folks, what do you want to do?
>
> Make them fix their code.
>
> It's one thing to have warts for allowing the market-leading GUI frontend
> for databases (MS Access) to work, but if every, with all due respect,
> random client wants to have its own break-the-spec-for-me switch we'll go
> mad.
>
> Explain to me why it's so hard to say 0 when you mean 0.
>>>>> "BM" == Bruce Momjian <pgman@candle.pha.pa.us> writes: BM> you wanted the broken behavior, and keep it for one release cycle. We BM> thought all the apps could be fixed during the beta period --- obviously BM> we were wrong. In the case of RT, the primary development is done in MySQL, and Postgres is also supported. Also, there is a major new version being done, so the current release only gets bug fixes and not that much other attention from the developers... Just a timing thing I suppose.
Steve Crawford wrote: > An example: I have asked on these groups several times about arrays and have > gotten no replies. Currently if I ask for an element beyond the end of an > array it will return a null. If I could find that to be the documented > specification for array behavior I could start working on my app. As it is I > can not because I may be creating code that will break with the next release > of PostgreSQL. > > I am all the more concerned since with version 7.3 the interpretation of > arrays has become non-consistent. A skipped element in a text array gets a > default entry of '' while a skipped element in an int array, which used to > get a default entry of 0, now causes an error. So before we can chastize > someone for not following a specification there has to actually be one. If you have an itch, then I'd suggest you scratch it ;-) I'm sure if you submitted patches to improve the current documentation, they'd be gratefully accepted. Similarly, if there are currently unspecified aspects of array handling behavior that concern you, post a proposal to HACKERS and work with the community to refine and gain acceptance of that proposal. Even if you don't actually do the coding, those first few steps (thinking through how arrays *should* behave, clearly and completely *explaining* it, and getting community *agreement*) are typically non-trivial, and a significant barrier for anyone wanting to step up and write the code. Just my 2c. Joe
On Thu, 2002-12-19 at 12:55, Larry Rosenman wrote: > > Isn't that what beta is for? - For app developers to test their apps > > against the new version to discover problems/issues before the production > > release. If this had been brought up during the beta period there may > > well have been a different outcome. > I brought up at least ONE app that had issues during beta. and was told, > "well, that's only one"....... No -- you brought up 1 app that had problems in the beta (not "at least ONE app"). And simply because a single application breaks is not sufficient reason to revert the change -- we'd need to see fairly widespread breakage. We're seeing some evidence of that now, but the point is that we didn't see it during the beta period. Admittedly, some of that is our (the developers) fault: I recall raising the issue of backward compatibility at the time the change was made, but the consensus was that it wouldn't break many applications, if any. While we should try to do better in the future, I agree with those who say that if you want to ensure your app works with a new release of PostgreSQL, you should participate in the development process. > So, there was at least notice during the beta, but I was told to go away > (essentially). No, the response to your inquiry was completely appropriate. Cheers, Neil -- Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Larry Rosenman wrote: > > > --On Thursday, December 19, 2002 09:57:15 -0800 Barry Lind > <blind@xythos.com> wrote: > >> >> Larry Rosenman wrote: >> > >> > The issue isn't whether the app is broken or not, (it is), but the >> fact >> > that we >> > ****CHANGED**** behavior with ****NO**** notice and no way to keep the >> > app working on >> > the current release without MAJOR changes to the app. (some apps just >> > have it so ingrained >> > in them). >> > >> > We ***SHOULD*** have noticed the user community, and had a transition >> plan. > >> > This should be a lesson to us to be more sensitive to applications out >> > there, and >> > give NOTICE about compatibility issues. >> > >> >> Isn't that what beta is for? - For app developers to test their apps >> against the new version to discover problems/issues before the production >> release. If this had been brought up during the beta period there may >> well have been a different outcome. > > I brought up at least ONE app that had issues during beta. and was > told, "well, that's > only one"....... > >> >> In general it is difficult to determine whether any particular change >> will impact app developers. And this problem isn't unique to postgres. >> I can't remember a major Oracle upgrade that hasn't broken my >> applications in some way. The difference with Oracle is that I don't >> find out until the new release is production, whereas with postgres I >> ensure that I am testing with all the betas and getting any issues >> resolved before it goes production. > > I noticed some oddities with my TEST RT, but didn't tie them to the 7.3 > change. > > So, there was at least notice during the beta, but I was told to go away > (essentially). > And given one report of a problem, I think that was the correct response. I don't think it is wise to add a GUC parameter for every change that impacts one person/one app. However if multiple people had reported the same problem and the impact of the change was clearly understood to negatively impact a large number of users then I am sure a GUC parameter would have been added a while ago. The point I was trying to make is that there were probably a hundred changes between 7.2 and 7.3 that could have impacted apps, some intentional changes, others not (and it wouldn't make sense to add parameters for all of them). Deciding which of these have a significant impact on existing apps is part of what the beta period is for. The more people that test their apps during the beta period the better change we won't have issues like this in the future. thanks, --Barry
On Thu, 19 Dec 2002, Barry Lind wrote: > And given one report of a problem, I think that was the correct > response. I don't think it is wise to add a GUC parameter for every > change that impacts one person/one app. However if multiple people had > reported the same problem and the impact of the change was clearly > understood to negatively impact a large number of users then I am sure a > GUC parameter would have been added a while ago. > > The point I was trying to make is that there were probably a hundred > changes between 7.2 and 7.3 that could have impacted apps, some > intentional changes, others not (and it wouldn't make sense to add > parameters for all of them). Deciding which of these have a significant > impact on existing apps is part of what the beta period is for. The > more people that test their apps during the beta period the better > change we won't have issues like this in the future. So, and this is just theorizing, what about having every change that affects parsing syntacitcally, especially changes like this, be tracked and handled with a single compatibility GUC var like revert_prev_syntax that is only around for one version, defaults to off, and handles all the new versus old syntax. That way, when there's a new release, say 7.4 from 7.3, you can just strip out all the code marked as being turned on by this one GUC and move on with a new empty spot for revert_prev_syntax. Of course, then you might have a problem where reverting to old syntax takes away a feature someone wants in one area while removing a problem in another, but, if you need to revert to the previous versions syntax, then you probably aren't too worried about whether or not you have every single shiny new feature... Just a bit of a ramble.
Steve Crawford <scrawford@pinpointresearch.com> writes:
> I am all the more concerned since with version 7.3 the interpretation of
> arrays has become non-consistent.
This is not inconsistent. You are in both cases trying to feed an empty
string to the input parser for the array's element type. You may as
well complain because "abc" is acceptable input for text and not for
integer...
regards, tom lane