Thread: Avoiding upgrade backlash
Hackers, I'm seeing from reports on IRC that 8.3 may cause an unprecedented number of upgrade issues for users who have old, badly-written applications. Stefan K, for example, was reporting the necessity of re-writing 3000 lines of his PL/pgSQL code on an older app (about 10%). While we are right to choke down abuse of implicit casting, it might be a good idea to give users stronger warnings and instructions. I'm thinking that we need to warn everyone about: 1) They need to use 8.3's pg_dump, not the old version, to upgrade (this is always true but now doing it wrong will break a lot more users). 2) They need to check for bugs 3) If Robert gets his type-cast backport package together, the location of that. I'm thinking this warning should go (prominently) into the release notes, the community notice, and the extended web release. -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes:
> While we are right to choke down abuse of implicit casting, it might be a
> good idea to give users stronger warnings and instructions.
> I'm thinking that we need to warn everyone about:
> 1) They need to use 8.3's pg_dump, not the old version, to upgrade (this is
> always true but now doing it wrong will break a lot more users).
Uh, what makes you think that will make any difference?
> 3) If Robert gets his type-cast backport package together, the location of
> that.
The very *last* thing I want to see is encouragement of people trying to
undo that change.
regards, tom lane
Am Montag, 12. November 2007 schrieb Josh Berkus: > I'm thinking that we need to warn everyone about: > 1) They need to use 8.3's pg_dump, not the old version, to upgrade (this is > always true but now doing it wrong will break a lot more users). What difference would pg_dump make? > 2) They need to check for bugs What bugs? > 3) If Robert gets his type-cast backport package together, the location of > that. Well, if you want to undo the changes, you don't need a backport package; you can just change the cast's definition. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter, > What difference would pg_dump make? For many user's databases ... many more than any previous version after 7.3 ... using the old version's pg_dump *will not load* into 8.3. Unless you restore the casts. > Well, if you want to undo the changes, you don't need a backport package; > you can just change the cast's definition. Right. We've talked (on IRC) about putting together a package which restores the removed casts. Given that Gentoo, Red Hat and SuSE don't support having multiple PostgreSQL versions on the system in their packaging, I'm thinking the "restore casts" package is essential rather than being a nice idea. Should I be discussing this on a different list? -- Josh Berkus PostgreSQL @ Sun San Francisco
Am Dienstag, 13. November 2007 schrieb Josh Berkus: > > What difference would pg_dump make? > > For many user's databases ... many more than any previous version after > 7.3 ... using the old version's pg_dump *will not load* into 8.3. Unless > you restore the casts. An example case would be useful here as well. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Josh Berkus <josh@agliodbs.com> writes:
> Peter,
>> What difference would pg_dump make?
> For many user's databases ... many more than any previous version after
> 7.3 ... using the old version's pg_dump *will not load* into 8.3. Unless you
> restore the casts.
Please provide a concrete example.
> Should I be discussing this on a different list?
If you are looking for code changes, neither docs nor advocacy are
suitable forums.
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Tue, 13 Nov 2007 11:39:26 -0500 Tom Lane <tgl@sss.pgh.pa.us> wrote: > Josh Berkus <josh@agliodbs.com> writes: > > Peter, > >> What difference would pg_dump make? > > > For many user's databases ... many more than any previous version > > after 7.3 ... using the old version's pg_dump *will not load* into > > 8.3. Unless you restore the casts. > > Please provide a concrete example. > > > Should I be discussing this on a different list? > > If you are looking for code changes, neither docs nor advocacy are > suitable forums. Josh wasn't looking for code changes. He was insuring that we were loud about the rather significant problems that upgrading to 8.3, "may" cause. Joshua D. Drake > > regards, tom lane > > ---------------------------(end of > broadcast)--------------------------- TIP 4: Have you searched our > list archives? > > http://archives.postgresql.org > - -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240 PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFHOdWAATb/zqfZUUQRAjG4AJ9WZgxRABLBu/E6gid1t1Jg1pDxcQCfanaB MNH1nlHU6lEO+z+IJ6mxLvE= =2Tga -----END PGP SIGNATURE-----
Peter Eisentraut <peter_e@gmx.net> writes:
> Am Montag, 12. November 2007 schrieb Josh Berkus:
>> 3) If Robert gets his type-cast backport package together, the location of
>> that.
> Well, if you want to undo the changes, you don't need a backport
> package; you can just change the cast's definition.
It's actually not going to be that easy, because most of those casts
aren't even in pg_cast anymore: they have been subsumed into the
CoerceViaIO mechanism. You'd need to resurrect the individual cast
functions before you could put entries back, too.
Another little problem is that you're likely to break as much stuff as
you fix. An example in CVS HEAD:
regression=# select 42 || 'foo';
?column?
----------
42foo
(1 row)
regression=# select 42 like 'foo';
ERROR: operator does not exist: integer ~~ unknown
LINE 1: select 42 like 'foo';
^
HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
OK, let's "fix" that by making int->text implicit again:
regression=# create function inttotext(int) returns text as $$
regression$# begin return $1; end$$ language plpgsql strict immutable;
CREATE FUNCTION
regression=# create cast (int as text) with function inttotext(int)
regression-# as implicit;
CREATE CAST
Now LIKE works:
regression=# select 42 like 'foo';
?column?
----------
f
(1 row)
but || not so much:
regression=# select 42 || 'foo';
ERROR: operator is not unique: integer || unknown
LINE 1: select 42 || 'foo';
^
HINT: Could not choose a best candidate operator. You might need to add explicit type casts.
regards, tom lane
I have added additional documentation for this item to the release
notes, with examples:
Non-character values are no longer automatically cast to TEXT (Peter,
Tom)
Previously, a function or operator that took a TEXT parameter used to
automatically cast a non-TEXT value to TEXT and call the function or
operator (assuming there was no other matching function or operator).
That no longer happens and an explicit cast to TEXT is now required. For
example, these expressions now throw an error:
substr(current_date, 1, 1);
23 LIKE '%2%'
5.4 ~ '6';
but these work because of the explicit casts:
substr(current_date::text, 1, 1);
23::text LIKE '%2%
5.4::text ~ '6';
CHAR and VARCHAR still cast to TEXT automatically. Concatenation (||)
with non-TEXT types is still automatically cast, assuming one of the
parameters is textual. While this change will require additional casts
for some queries, it also eliminates some unusual behavior.
Hope it isn't too detailed.
---------------------------------------------------------------------------
Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > Am Montag, 12. November 2007 schrieb Josh Berkus:
> >> 3) If Robert gets his type-cast backport package together, the location of
> >> that.
>
> > Well, if you want to undo the changes, you don't need a backport
> > package; you can just change the cast's definition.
>
> It's actually not going to be that easy, because most of those casts
> aren't even in pg_cast anymore: they have been subsumed into the
> CoerceViaIO mechanism. You'd need to resurrect the individual cast
> functions before you could put entries back, too.
>
> Another little problem is that you're likely to break as much stuff as
> you fix. An example in CVS HEAD:
>
> regression=# select 42 || 'foo';
> ?column?
> ----------
> 42foo
> (1 row)
>
> regression=# select 42 like 'foo';
> ERROR: operator does not exist: integer ~~ unknown
> LINE 1: select 42 like 'foo';
> ^
> HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
>
> OK, let's "fix" that by making int->text implicit again:
>
> regression=# create function inttotext(int) returns text as $$
> regression$# begin return $1; end$$ language plpgsql strict immutable;
> CREATE FUNCTION
> regression=# create cast (int as text) with function inttotext(int)
> regression-# as implicit;
> CREATE CAST
>
> Now LIKE works:
>
> regression=# select 42 like 'foo';
> ?column?
> ----------
> f
> (1 row)
>
> but || not so much:
>
> regression=# select 42 || 'foo';
> ERROR: operator is not unique: integer || unknown
> LINE 1: select 42 || 'foo';
> ^
> HINT: Could not choose a best candidate operator. You might need to add explicit type casts.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce, > CHAR and VARCHAR still cast to TEXT automatically. Concatenation (||) > with non-TEXT types is still automatically cast, assuming one of the > parameters is textual. While this change will require additional casts > for some queries, it also eliminates some unusual behavior. > > Hope it isn't too detailed. Hmmm. I don't feel like it contains a clear warning, though. We need a statement like: Some users may encounter issues upgrading due to the changes in handling data type casts. If you suspect that your schema or application may not be handling data type comparisons cleanly, you are strongly urged to test for casting error messages before upgrading your production server. -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes:
> Hmmm. I don't feel like it contains a clear warning, though. We need a
> statement like:
> Some users may encounter issues upgrading due to the changes in handling
> data type casts. If you suspect that your schema or application may not
> be handling data type comparisons cleanly, you are strongly urged to test
> for casting error messages before upgrading your production server.
When have we ever suggested that people *shouldn't* test their apps
before committing to a major version update?
I think the whole thing has been significantly overblown. We have made
incompatible changes bigger than this one --- and, often, harder to find
let alone fix --- in almost every major release. This one you can at
least be pretty sure that you will get an error, and not a silent change
in behavior, if you fall foul of it.
regards, tom lane
Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: > > Hmmm. I don't feel like it contains a clear warning, though. We need a > > statement like: > > > Some users may encounter issues upgrading due to the changes in handling > > data type casts. If you suspect that your schema or application may not > > be handling data type comparisons cleanly, you are strongly urged to test > > for casting error messages before upgrading your production server. > > When have we ever suggested that people *shouldn't* test their apps > before committing to a major version update? > > I think the whole thing has been significantly overblown. We have made > incompatible changes bigger than this one --- and, often, harder to find > let alone fix --- in almost every major release. This one you can at > least be pretty sure that you will get an error, and not a silent change > in behavior, if you fall foul of it. Agreed. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > Tom Lane wrote: >> Josh Berkus <josh@agliodbs.com> writes: >>> Hmmm. I don't feel like it contains a clear warning, though. We need a >>> statement like: >>> Some users may encounter issues upgrading due to the changes in handling >>> data type casts. If you suspect that your schema or application may not >>> be handling data type comparisons cleanly, you are strongly urged to test >>> for casting error messages before upgrading your production server. >> When have we ever suggested that people *shouldn't* test their apps >> before committing to a major version update? >> >> I think the whole thing has been significantly overblown. We have made >> incompatible changes bigger than this one --- and, often, harder to find >> let alone fix --- in almost every major release. This one you can at >> least be pretty sure that you will get an error, and not a silent change >> in behavior, if you fall foul of it. > > Agreed. Honestly, I think you are underestimating this problem. I know that every single one of our customers is going to run into this and as much as I would like to say otherwise, the world has many more postgresql users than I have customers. This needs to be a dominant statement not only in the release notes but in the FAQ and frankly we should be running some up to the release announcements on -general and possibly the website about this change, how it manifests itself and explicit examples on how to resolve it. Joshua D. Drake
Joshua D. Drake wrote: > Bruce Momjian wrote: > > Tom Lane wrote: > >> Josh Berkus <josh@agliodbs.com> writes: > >>> Hmmm. I don't feel like it contains a clear warning, though. We need a > >>> statement like: > >>> Some users may encounter issues upgrading due to the changes in handling > >>> data type casts. If you suspect that your schema or application may not > >>> be handling data type comparisons cleanly, you are strongly urged to test > >>> for casting error messages before upgrading your production server. > >> When have we ever suggested that people *shouldn't* test their apps > >> before committing to a major version update? > >> > >> I think the whole thing has been significantly overblown. We have made > >> incompatible changes bigger than this one --- and, often, harder to find > >> let alone fix --- in almost every major release. This one you can at > >> least be pretty sure that you will get an error, and not a silent change > >> in behavior, if you fall foul of it. > > > > Agreed. > > Honestly, I think you are underestimating this problem. I know that > every single one of our customers is going to run into this and as much > as I would like to say otherwise, the world has many more postgresql > users than I have customers. > > This needs to be a dominant statement not only in the release notes but > in the FAQ and frankly we should be running some up to the release > announcements on -general and possibly the website about this change, > how it manifests itself and explicit examples on how to resolve it. If it is that big an issue why are we not getting more problem reports from beta testers? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > Joshua D. Drake wrote: > If it is that big an issue why are we not getting more problem reports > from beta testers? Most people don't beta test further most people don't test (you know this). My customers aren't going to be an issue obviously. What I am worried about is the backlash from those who like to do this: Ohhhh 8.3 is out. Upgrade WTH just happen! And there are far more of those than I wish to admit (consider the existence of gentoo ;)). Sincerely, Joshua D. Drake
Joshua D. Drake wrote: > Bruce Momjian wrote: > > Joshua D. Drake wrote: > > > If it is that big an issue why are we not getting more problem reports > > from beta testers? > > Most people don't beta test further most people don't test (you know this). > > My customers aren't going to be an issue obviously. What I am worried > about is the backlash from those who like to do this: > > Ohhhh 8.3 is out. > Upgrade > WTH just happen! > > And there are far more of those than I wish to admit (consider the > existence of gentoo ;)). I understand but unless we have feedback from actual users we can't know which issues need X notice and which need 10X notice. We can guess and right now I think we have enough detail. If we find we haven't covered it enough we can always revisit this, even after the final release. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
"Joshua D. Drake" <jd@commandprompt.com> writes:
> Honestly, I think you are underestimating this problem.
The reason I'm underestimating the problem is that neither you nor
Josh have provided one iota of evidence to support your position
that the sky is falling.
When I look back at the release notes for the past several major
releases, I see lots of incompatibilities that look far worse than this
one. In many cases there were fairly subtle semantic changes that would
cause a query to still run, but deliver different answers than before.
Now that is the sort of thing that would maybe justify banners on the
rooftops, but as far as I recall we have always assumed that our users
were adult enough to read the compatibility warnings in the release
notes.
Heck, we have created bigger compatibility problems in *minor* releases;
the security-related tightening of multibyte encoding validity checks
caused a lot more problems for a lot more people (and with a lot less
notice) than I believe this change will do.
What we've got here is a case where certain people who were doing fairly
squirrely things will see errors that compel them to insert explicit
casts to make their intent clear. This won't even create a backwards
compatibility problem for them: the correctly-casted code will still
work in prior releases too. To me this looks comparable to, say,
the removal of LIMIT #,# syntax in favor of LIMIT/OFFSET in 7.3.
I fail to see why this is a major problem; and merely asserting that
it is one isn't going to change my mind. Can you point to something
like a major application that will be broken and can't easily be fixed?
regards, tom lane
Tom Lane wrote: > "Joshua D. Drake" <jd@commandprompt.com> writes: >> Honestly, I think you are underestimating this problem. > > I fail to see why this is a major problem; and merely asserting that > it is one isn't going to change my mind. Can you point to something > like a major application that will be broken and can't easily be fixed? SQL-Ledger/LedgerSMB What I find interesting is that I (and Josh) are getting push back and all we are trying to do is be a little louder about the change. Joshua D. Drake > > regards, tom lane >
Joshua D. Drake wrote: > Tom Lane wrote: > > "Joshua D. Drake" <jd@commandprompt.com> writes: > >> Honestly, I think you are underestimating this problem. > > > > > I fail to see why this is a major problem; and merely asserting that > > it is one isn't going to change my mind. Can you point to something > > like a major application that will be broken and can't easily be fixed? > > SQL-Ledger/LedgerSMB > > What I find interesting is that I (and Josh) are getting push back and > all we are trying to do is be a little louder about the change. Adding extra warnings/notices is not a zero-cost action so there has to be justification to do it. Actually it is near-zero cost to use but not zero-cost to our users who have to read it multiple places. (Think dumb product warning labels.) What I have done is to move the item up to the top of our incompatibilities list in the release notes. That is near zero cost for everyone. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > Joshua D. Drake wrote: >> Tom Lane wrote: >>> "Joshua D. Drake" <jd@commandprompt.com> writes: >>>> Honestly, I think you are underestimating this problem. >>> I fail to see why this is a major problem; and merely asserting that >>> it is one isn't going to change my mind. Can you point to something >>> like a major application that will be broken and can't easily be fixed? >> SQL-Ledger/LedgerSMB >> >> What I find interesting is that I (and Josh) are getting push back and >> all we are trying to do is be a little louder about the change. > > Adding extra warnings/notices is not a zero-cost action so there has to > be justification to do it. Actually it is near-zero cost to use but not > zero-cost to our users who have to read it multiple places. (Think dumb > product warning labels.) > > What I have done is to move the item up to the top of our > incompatibilities list in the release notes. That is near zero cost for > everyone. > Thank you. Joshua D. Drake
On Sat, Nov 17, 2007 at 04:05:50PM -0500, Bruce Momjian wrote: > What I have done is to move the item up to the top of our > incompatibilities list in the release notes. That is near zero cost for > everyone. It bit me when testing my code against PG 8.3. Your docs helped me see my mistake immediately. Thanks and Regards Theo