Forum OpenACS Q&A: PG 7.2->7.3 upgrade gotcha?

Collapse
Posted by Michael Steigman on
In the course of upgrading my home machine from Redhat 7.3 to Redhat 9, I brought a dump from 7.2 into 7.3 and, with some help from the folks on IRC, figured out that 7.3 does not look for truncated plsql names as 7.2 did (at least not by default in the packages built for RH9). I had to drop and reload all truncated functions after loading the old data.

Anyone else run into this yet?

Collapse
Posted by Jonathan Ellis on
aren't you supposed to use pg_dump from the more recent version when upgrading?
I ran into the same problem when upgrading the debian pg package from 7.2 to 7.3. Propably some parameter setting for maximum identifier size was changed on the way. A silly workaround might be to compile 7.3 by hand and set this parameter to the old value, but I'd rather rename to the full names as you did.

Can you share how you dropped and reloaded _all_ the functions - did you find some way to do that automatically? And does everything still work, e.g. views that contain fields computed by such functions?

Jonathan, I don't think so since the debian upgrade scripts explicitely keeps an old version of the pg_dump executable around to create the dump.
Collapse
Posted by Jonathan Ellis on
perhaps it's being excessively cautious...  the 7.3 pg_dump manual says "pg_dump can handle databases from previous releases of PostgreSQL, but very old versions are not supported anymore (currently prior to 7.0)."
Collapse
Posted by Michael Steigman on
Jonathon - I don't think pg_dump is the problem. There was a limit of 31 characters prior to 7.3 (someone correct me if I'm wrong) so the functions were truncated when created. I'm not an expert but I doubt that PG keeps the full name around after the functions have been created.

Til - I reloaded everything manually. I just queried for function names of 31 characters and built up a huge file of drop/create commands. That's the simple, braindead way and it *seems* to have worked. I did have to drop and recreate one view (acs_messages_latest) but that's it. It would be nice to have a little script to do this automatically.

If there are views in particular you are curious/worried about, let me know and I'll test them out. As I said, I haven't tested thoroughly.

Also, I don't think workaround you mention would help as the problem occurs when the plsql functions which have been truncated are called by their full names from other plsql functions or from xql files. There may be an option to enable PG to look for truncated names in 7.3. I did look at the 7.3 release notes which contain a few upgrade tips but that's not mentioned. There's a link to some additional upgrade tips but the link leads to a 404. I haven't looked any further yet.

Collapse
Posted by Don Baccus on
Oh, man, I didn't even think about this "gotcha" in the upgrade.  The increase in default name length is great, but it does screw the upgrade path for existing sites.

No, it does not store the extra chars when it truncates.  It is ... truncating ... at the max length just as it says it is.  So those chars are gone forever.

Since it appears you've already done 85% or more of the work necessary to put together an upgrade script by brute force, would you have time to either tackle the full job or perhaps cooperate with someone else who is willing to work on it?

This really sucks.

Collapse
Posted by Andrew Piskorski on
This is a good example of why all PL/SQL and plpgsql code should be defined, with 'create or replace', in their own SQL files, separate from the 'create table', etc. DML statements. Then all you have to do is re-source all those stock SQL files into the database, and all your stored functions and procedures are nicely redefined.

In my own work with Oracle, way back when I adopted a naming convention of "*-ph.sql" and "*-pb.sql" to indicate "package header" and "package body" files, since most Oracle PL/SQL code should be in PL/SQL packages. PostgreSQL doesn't have plpgsql packages, but at least when I tweaked the OpenACS Static Pages package, I used the same "*-pb.sql" naming convention for Postgres, to line up with Oracle.

(Note that it is also often useful to have the Oracle PL/SQL package header and body in two separate files, but this is much less important than having the PL/SQL definitions separate from the DML.)

The same goes for views and triggers, incidentally - they should be separate from the DML so you can easily recreate them just by sourcing the file. The general principle here is that stuff which you can simply "create or replace" (stored procedure code, views, triggers) should always be in a separate file from stuff which you can only "create" (tables). This makes the logistics of maintenance and upgrades substantially easier, and there's no drawback. Any more detailed break out of stuff, like PL/SQL package bodies and headers in separate, is just icing on the cake.

Maybe we could come up with some OpenACS recomended standard for this sort of thing and move all the packages towards using it.

Collapse
Posted by Malte Sussdorff on
Andrew, after trying to upgrade one of our sites last night, I'm all yours..
Collapse
Posted by Michael Steigman on
What I have now: a huge file containing definitions for functions with names > 31 from 4.6.1 along with drop command for the truncated functions of the same name. If anyone would like a copy of it, let me know.

I'm willing to work with someone to mold this into a general solution. What else would be needed? How should non-core packages be handled (the only non-core stuff I have installed is photo album and lars-blogger)? Also, if this were to become a general solution, it would probably make more sense to redefine the functions using definitions from a PG 7.3 compatible release, i.e., 4.6.3 (or maybe 4.6.4), and then note that the upgrage path from 7.2 to 7.3 must go through that particular release (I don't think we want to maintain this monstrosity for each release, do we?).

Having a naming convention for breaking out non-DML stuff would be great. Almost every new module I've seen does this for its package code but each seems to use its own naming convention.

Collapse
Posted by Andrew Piskorski on
Michael, your one big huge file duplicating all the plpgsql functions with long names is a good solution for people faced with this problem right now with the same version of OpenACS (4.6.1) you're using.

But no, it wouldn't make sense at all to maintain that indefinitely as part of the regular OpenACS release. (Having the same function defined redundantly in two different places is almost always a bad idea.)

The correct general solution is to establish an OpenACS wide convention like I was talking about above. Then there will be no need for duplicate copies of function definitions, you'd simply write a little script to find and source all the "sql/postgresql/*-pb.sql" files or whatever.

Converting all the packages to a particular naming convention should be pretty straightforwad, but there's definitely a fair amount of drudgery involved, and it does require touching all those source files and being careful about it. So in the interim, Michael, for OpenACS 4.6.2 or 4.6.3 users, if you have either some script to automatically generate the "one big file" solution you came up with for 4.6.1, or a similar file for those newer versions of OpenACS, that would probably be quite useful to anyone using those versions of OpenACS who needs to upgrade from PostgreSQL 7.2 to 7.3.

In fact, the changing of packages to match a new naming convention would probably need to be done on the Head (currently what will become OpenACS 5.0), and likely anyone upgrading to 5.0 will have already upgraded from PostgreSQL 7.2 to 7.3?

There is no reason for this naming convention to be any difference for core and non-core packages. If we establish an organizing and naming convention, then we can and should change all packages to use it, core or not. Naturally, the core packages would probably get done first.

Collapse
Posted by Don Baccus on
Well ... until PG 7.2 PostgreSQL didn't support CREATE OR REPLACE, so it wasn't an option during our first couple of releases.  Semantic changes in the opaque type used by trigger functions mean you can't use CREATE OR REPLACE with any of these you want to be able to upgrade across the PG 7.2->PG 7.3 boundary.

And PG 7.2 doesn't support CREATE OR REPLACE VIEW :(

(7.3 does? hope hope)

We're getting *close* but aren't quite to the point where we can CREATE OR REPLACE everything, sadly.  But we can do so for normal functions now since we no longer support PG 7.1  I've been switching over some create scripts when I've had other reasons to visit them.

If anyone wants to go on the warpath to do this for any particular package, feel free ... just keep in mind that it's not kosher to use CREATE OR REPLACE VIEW yet or to use CREATE OR REPLACE on a function that is used in a trigger.

Of course if there are any missing REPLACEs on Oracle packages these should be fixed ASAP.

Collapse
Posted by Andrew Piskorski on
Ah, important points about views, etc. in Postgres. I wasn't aware of those limitations.

As far as an OpenACS-wide file naming and organization convention, ok, here's one concrete proposal:

  • Wherever possible and feasible, database objects must be defined with "create or replace" rather than "create". See below [TODO] for info on Oracle and PostgreSQL limitations.

  • All procedural code that lives inside the database must be defined in its own files separate from any DDL ("create table", etc.) statements. These files must be named either "*-pb.sql" (for Oracle package bodies, functions, and procedures; for PostgreSQL functions and procedures) or "*-ph.sql" (for Oracle package headers).

  • Oracle package headers may be included with the package bodies in the "*-pb.sql" file, but often should be in their own "*-ph.sql" file (developer's judgment). As PostgreSQL pl/pgsql does not support packages, there should be no Postgres "*-ph.sql" files.

  • Triggers should be in their own file named "*-tr.sql".

  • Views should be in their own file named "*-vw.sql".

If anyone has counter-suggestions of comments, please chime in. :)

E.g., perhaps "*-fn.sql" should also be allowed in addition to "*-pb.sql" for Oracle and PostgreSQL functions and procedures. In that case, I think you'd add:

  • Stand-alone Oracle functions and procedures, not part of any PL/SQL package, may be in a "*-pb.sql" file, but usually should be in a "*-fn.sql" file instead.

  • PostgreSQL functions designed to emulate or be compatible with Oracle package body functions should preferably be in a "*-pb.sql" file rather than a "*-fn.sql" file.
Collapse
Posted by Andrew Piskorski on
Hm, in PostgreSQL, you can "create or replace" functions, but if you use it on a function used in a trigger the trigger will break? Ouch. Or what bad thing will happen to the trigger exactly?

Is that true for any functions called by a Postgres trigger in any way, or just for the function that defines the body of the trigger?

Collapse
Posted by Tilmann Singer on
Michael, I would appreciate if you could post the script you have so far, e.g. to file-storage
Collapse
Posted by Michael Steigman on
OK, it's now in file storage at https://openacs.org/storage/file?file_id=109882.
Collapse
Posted by Don Baccus on
It just breaks for the function that corresponds to the body of an Oracle trigger, i.e. the one called directly by the trigger.

They changed the return type and kludged CREATE to do the right thing, but when you're REPLACING you get an ambiguity error because overload resolution is done on the parameter list only.  The function return type plays no role in disambiguation of a function signature (as is typical in languages that allow function overloading)

In an upgrade script where you know the function already exists you can DELETE then CREATE the function but that doesn't work in *-create.sql scripts.

Collapse
Posted by Jun Yamog on
Hi,

I created a simple script to read the dump file. And functions that has 31 chars, I try to look for the function name on the oacs dir. Then substitute it.

You can get the script here:

https://openacs.org/storage/file?file_id=117145

This only cover functions. I did saw 2 views that needs un-truncating too, they are:

ad_template_sample_users_sequence
acs_privilege_descendant_map_view
After restoration, views that make use of the truncated functions. Must be rebuilt. It varies from 0 to a couple, will depend on what packages are installed. So log your restoration process to a log file.

Hope this helps someone.

Also since I just go over the table name, view name and function name. Did I miss anything major? This are the things that are created during restore:

CREATE CONSTRAINT
CREATE FUNCTION
CREATE INDEX
CREATE RULE
CREATE SEQUENCE
CREATE TABLE
CREATE TRIGGER
CREATE TRUSTED
CREATE UNIQUE
CREATE VIEW
Collapse
Posted by Jun Yamog on
Oh yeah my perl is rusty, please comment if there are any problems.
Collapse
Posted by Deds Castillo on
The perl script Jun made was very useful.  There's still some problems in upgrading though.  Here is the scenario:

1. We use the script to replace all create statements
2. This will only replace the creates. I've noticed some sql queries which use truncated views in the "from" section of their select statements.
3. The problem now arises because the selects will look for the shorter names but Jun's script now created longer names.

Found out the hard way when my RH8.0 got flaky and I was forced to upgrade to RH9.0.

Collapse
Posted by Dave Bauer on
I found a little glitch running this on the openacs.org database. It likely will never show up on any other data.

On line 39

the code reads

    if ($_ =~ /CREATE FUNCTION "(.*)"/) {

and it will capture the functions inside of data such as forum postings.

I changed it to

    if ($_ =~ /^CREATE FUNCTION "(.*)"/) {

to only find CREATE FUNCTION at the beginning of a line and it seems to work.

Collapse
Posted by Jun Yamog on
Hi,

Thanks for using it.  Never thought that it will be used by others who I really know or to openacs.org.

I have put a comment about the views, corrected the regexp and put a link to this forum on the file.

Collapse
Posted by Tilmann Singer on
Jun, thanks so much for writing this, it was really useful for me recently!

It seems that it did catch almost all of the identifiers except one: content_item__get_latest_revision was not replaced with content_item__get_latest_revision in a few places, so I had to replace them manually. No idea why it missed them. There was no error in the output of dump_parser.pl. If you are interested in debugging it I can to send you the relevant portions of the dump file.

Collapse
Posted by Jun Yamog on
Hi Tils,

No thanks.  But I am glad it did help someone else.  If anybody wants to improve, its open source.  Modify it. :)

Collapse
Posted by Justin Palmer on
This doesn't answer your question, but someone might find it helpful...  Postgres 7.3 will also do some funky stuff when you try to do "+" or "-" between timestamps and integers.  You can find a fix for this which works great at:

http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&threadm=bg5cbj%241map%241%40FreeBSD.csie.NCTU.edu.tw&rnum=3&prev=/groups%3Fq%3DUnable%2Bto%2Bidentify%2Ban%2Boperator%2Bfor%2Btypes%2B%27timestamp%2Bwith%2Btime%2Bzone%27%2Band%2B%27integer%27%26hl%3Den%26lr%3D%26ie%3DUTF-8%26selm%3Dbg5cbj%25241map%25241%2540FreeBSD.csie.NCTU.edu.tw%26rnum%3D3

Collapse
Posted by Tom Ayles on

Hey Jun, cool script! Just upgraded three 4.6.3 instances and one from head from PSQL7.2.4 to 7.3.4, with hardly any trouble. I came across another couple of functions that are missed by the script though, from the surveys package. For reference, these are:

  • survey_response__initial_respon needs to be replaced survey_response__initial_response_id
  • survey_response__initial_user_i needs to be replaced by survey_response__initial_user_id

Sorry I can't offer a patch to the script!

Collapse
Posted by Tilmann Singer on
Jun's script is now also in CVS, at bin/pg_7.2to7.3_upgrade_helper.pl
Collapse
Posted by Richard Hamilton on
I am trying to upgrade a 4.6.3 instance at the moment and ran the script to do the functions.

Thank you Jun and Tilmann for doing that - now I know why I really must learn perl!

When I restore the data it was obvious that none of the views had been created and that most of these views were not truncated ones (but depended on cr_xxxx views which were). To make sure that I found them all I modified your script to do views and discovered that the regexp that you have in it is in the default greedy form - which does matter if searching for views.

So here is one possible alternative non greedy regexp for the record :

/^CREATE VIEW "([^"]*)"/

Maybe this will help someone to find other truncated views in some of the more infrequently used packages.

Richard

Collapse
Posted by Tilmann Singer on
Jun wrote the script, not me.

Do you think your change is safe for general use, if yes could you please commit it (if you have commit rights) or provide a patch?

Collapse
Posted by Richard Hamilton on
Tilmann,

Actually I think this is the reason why the two functions are missed also.

I don't have commit rights to the cvs - in fact cvs is one of the next things I need to bone up on!

As far as safety is concerned, I am no regexp expert at all but the [^"]* means anything NOT a " any number of times which restricts the match to a function name rather than some thing like :

CREATE VIEW "acs_func_headers" as SELECT get_func_header("

..which will obviously never be found if the view or function name happens to be >31 chars.

I think that should be robust - can't see why not.

R.

Collapse
Posted by Richard Hamilton on
How do I submit a patch for this? I don't know where to find it in the bug tracker.
Collapse
Posted by Richard Hamilton on
I have spent all day today wading through the pg_dump of an OpenACS 4.6.3 site that I want to move from PostgreSQL 7.2.1 to PostgreSQL 7.4.2 and have now corrected ALL truncated functions and views along with all references to them in the pg_dump.

I have restored the dump into PG7.4.2 and have logged the standard output which looks fine.

However standard error to the console has thrown up some things that I need to ask about.

There are a whole load of lines like this one:

NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "surveys_pk" for table "surveys"

which I assume to be harmless.

One in the middle:

ERROR:  missing data for column "content_length"
CONTEXT:  COPY cr_revisions, line 3: "498  496    \N etc...........rest of data row
100%"..."

which worries me.

And a whole load of lines like these:

WARNING:  changing return type of function on_lobs_delete from "opaque" to "trigger"

which from Don's post I suspect is also no problem.

To try to sort out the cr_revisions problem I logged back into PG7.2 abd selected all cr_revisions rows where content_length isnull. There were some, so I did an update to set content_length to 0 where content_length isnull. Query worked fine, dumped the database, ran all of the conversions again to deal with the truncations and........

same error.

Does anyone know how I can sort this out. Maybe drop the constraint that is causing the restore of this table to fail? But which one is it?

Also I see that none of my view have been recreated. Not sure why since I carefully edited all the function references in the dump file. How best to recreate them? I would like to begin with the pa_photosi and pa_photosx views.

Any ideas gratefully received.

Regards
Richard

Collapse
Posted by Richard Hamilton on
I have a nasty suspicion that the html code that I have uploaded to the database using etp contains tabs. I further have a nasty feeling that tab is the default pg_dump delimiter. If so I am completely screwed.

Do I have a choice of delimiter?

Collapse
Posted by Richard Hamilton on
For any intrepid traveller treading this treacherous path......this is how to get the data.

Since the schema restore worked:

pg_dump -avD yourdatabasename -t cr_revisions -f chooseafilename > chooseafilename.log

This outputs the data as SQL INSERTS. So to restore this you use psql in the normal way.

Collapse
Posted by Richard Hamilton on
Following the upgrade I am getting lots of these in the error logs when testing both forums and photo album:

[22/Mar/2004:23:50:49][8221.49156][-conn0-] Error: Ns_PgExec: result status: 7 message: ERROR:  operator is not unique: bit varying || "unknown"
HINT:  Could not choose a best candidate operator. You may need to add explicit type casts.
CONTEXT:  PL/pgSQL function "tree_left" line 11 at return

I am guessing that they have to do with the change in trigger return type from "opaque" to "trigger" but I have absolutely no idea how to add explicit casts to functions and I am not even sure what data type to cast.

Oh bother.

Collapse
Posted by Don Baccus on
The PG group changed the type conversion rules (again) and some expressions in postgresql.sql (in acs-kernel/sql/postgresql) were no longer valid.

It's been fixed ... pull postgresql.sql from CVS HEAD. I just changed all the "create function" statements to "create or replace function" - just ignore the table def errors you get when you run it through PSQL.

Collapse
Posted by Richard Hamilton on
Don,

Thank you for your reply.

I pulled down the postgresql.sql file from cvs with tag set to HEAD and on looking through it seemed that only some of the functions were changed to CREATE OR REPLACE. When I ran it through postgres it only created 3 functions, and tree_right wasn't one of them.

I have obviously pulled down the wrong file although I checked it and it was last edited just 5 days ago. Sorry if I have I misunderstood your instructions.

For now I will edit my version of postgresql.sql to change all function declarations to CREATE OR REPLCE and see what happens.

R.

Collapse
Posted by Richard Hamilton on
Thank you Don. I hadn't remembered that I had to drop the database and create a blank new one before applying postgrsql.sql again.

All seems to work now - fingers crossed.

One extra step that I performed was to use a text editor to search the pg_dump file and replace all occurrences of the truncated function names with the full function names kindly provided by Jun's script. That meant that as soon as I had figured out how to restore the data to my cr_revisions table, all the views were created again automatically.

Time consuming, but a relief now it is done.

Many thanks
Richard

Collapse
Posted by Richard Hamilton on
OK, I had a second site to upgrade so I decided to re-visit the database upgrade script that Jun very kindly posted and, since I am in the process of trying to learn perl, work it into a slightly more general solution.

I have re-written it to work a different way around. Instead of grepping the entire OpenACS file tree every time it finds a long function name, the script now greps the files once only and extracts all function and view definitions - storing them in a hash.

It then runs through the pg_dump file, looking for all lines starting with 'CREATE', 'DROP' and '--Name : "', and substitutes the correct name in place of the truncated name.

This runs a great deal faster and catches every truncated name, including those in the CREATE VIEW statements that were previously missed, and those where the truncated function names are referenced in definitions of other dependent views (which has been noted by someone in a different thread). In short this SHOULD render the database upgrade trivial from here on in - I hope 😊

Usage is identical to Jun's original script and every substitution is displayed as before though in a slightly different format. I suggest piping output to a file to study before you try to restore the dump.

I hope that this will help to speed things up for people and that it will prove robust accross installations with a variety of different packages installed.

I am new to perl so if anyone has any feedback for me I'd welcome it.

I tried to upload it here but the ACS chokes on the perl file handle because it thinks that it is a html tag. So if anyone wants the script (to use or to add to the CVS) just email me.

Regards
Richard
Collapse
Posted by Torben Brosten on
Richard mentions in an email that there's a revised version somewhere. Looks like it's now available here:

http://cvs.openacs.org/cvs/openacs-4/bin/pg_7.2to7.3_upgrade_helper.pl?view=markup