Forum OpenACS Development: Minor Error in Survey Package after upgrading

After an upgrade (5.5 to 5.8) I have found an error in view-text-responses in Survey Package:
Database operation "select" failed
(exception ERROR, "ERROR:  record "v_rec" has no field "return"
LINE 1: SELECT v_rec.return
QUERY:  SELECT v_rec.return
CONTEXT:  PL/pgSQL function acs_object__get_attribute(integer,character varying) line 21 at assignment

    while executing
"ns_pg_bind select nsdb0 {
	  clob_answer as response,
    ("uplevel" body line 1)
    invoked from within
"uplevel $ulevel [list ns_pg_bind $type $db $sql]"
Posted by Gustaf Neumann on
does the following work for you without an error?
select object_id, acs_object__get_attribute(object_id, 'creation_user') from acs_objects order by object_id desc limit 10;
An error:
openacs=# select object_id, acs_object__get_attribute(object_id, 'creation_user') from acs_objects order by object_id desc limit 10;
select object_id, acs_object__get_attribute(object_id, 'creation_user') from acs_objects order by object_id desc limit 10;
ERROR:  record "v_rec" has no field "return"
LINE 1: SELECT v_rec.return
QUERY:  SELECT v_rec.return
CONTEXT:  PL/pgSQL function acs_object__get_attribute(integer,character varying) line 21 at assignment
Posted by Gustaf Neumann on
yet another upgrade problem? What do you see, when you do the following (this time, please with the "+")
\df+ acs_object__get_attribute
Well, perhaps something in my upgrading failed (I had to "force" psql one script in kernal). Perhaps some file didn't want to upgrade
openacs=# \x
Expanded display is on.
openacs=# \df+ acs_object__get_attribute
\df+ acs_object__get_attribute
List of functions
-[ RECORD 1 ]-------+----------------------------------------------------------------------------------------------------------------------------------------------
Schema              | public
Name                | acs_object__get_attribute
Result data type    | text
Argument data types | integer, character varying
Type                | normal
Security            | invoker
Volatility          | stable
Owner               | openacs
Language            | plpgsql
Source code         | 
                    | declare
                    |   object_id_in           alias for $1;  
                    |   attribute_name_in      alias for $2;  
                    |   v_table_name           varchar(200);  
                    |   v_column               varchar(200);  
                    |   v_key_sql              text; 
                    |   v_return               text; 
                    |   v_storage              text;
                    |   v_rec                  record;
                    | begin
                    |    v_storage := acs_object__get_attribute_storage(object_id_in, attribute_name_in);
                    |    v_column     := acs_object__get_attr_storage_column(v_storage);
                    |    v_table_name := acs_object__get_attr_storage_table(v_storage);
                    |    v_key_sql    := acs_object__get_attr_storage_sql(v_storage);
                    |    for v_rec in execute 'select ' || quote_ident(v_column) || '::text as return from ' || quote_ident(v_table_name) || ' where ' || v_key_sql
                    |       LOOP
                    |         v_return := v_rec.return;
                    |         exit;
                    |    end loop;
                    |    if not FOUND then 
                    |        return null;
                    |    end if;
                    |    return v_return;
                    | end;
Description         | 

Posted by Gustaf Neumann on
Your installation is missing the upgrade script: acs-kernel/sql/postgresql/upgrade/upgrade-5.7.0d1-5.7.0d2.sql

The problem at hand is a pg8/pg9 incompatibility. pg9 is closer to the standard and does not allow "return" as output column name. The error message from pg is suboptimal.

In case you have still the error.log from the upgrade, i would check, what upgrade scripts were executed and where the upgrade stopped. Then run the missing upgrade scripts manually.

If you don't have the log file anymore, go through the upgrade scripts and check in the database, if these were executed (i.e. pick e.g. an SQL function from the file and check, whether you have it in the db).

Hi Gustaf

Thanks!, it worked 😊 (I run the upgrade function manually).

I've lost error.log (at least I didn't find it) so I'll check this scripts to see if db has all functions in kernel upgrade scripts.

Hi Gustaf

I've been checking some upgrade scripts and I'm a bit confused. It seems that what I find in DB it's differect form upgrade script BUT I don't find big errors (only a few).

So, I'm not completely sure what upgrade scripts has been failed. Without that error.log, how can I know that?

Can I do a "manual upgrade" of each file? I suppose BD might be messed up, right?

Posted by Gustaf Neumann on
i think, the upgrade process of a package stops when the first upgrade script fails. Therefore, one has to run the later scripts, which were not executed.

In general it is desirable to have scripts that can be run multiple times without doing harm. i hope, that all upgrade scripts of the kernel have this property. so, i would recommend to make a backup first before running the missing upgrade scripts.

Take care to run the upgrade scripts in the right order.

Posted by Gustaf Neumann on
it sounds like a good idea to add a table to the database containing the names of every successfully executed upgrade script with timestamps. Then recoveries from broken upgrades would be much easier and more transparent
It might be. Whatever mechanism that allow user to check what function has be upgraded.

Another Idea could be stop (or prevent) upgrade but I think that's why error.log exists 😊.

My problem now (as a whatever regular user at this point) is that I'm not completely sure about what upgrade script succeed. So I'll try to do it "manual" in my dev server and check if there is some problem.

Posted by Jim Lynch on

dunno if this helps, and I haven't upgraded anything, but: I ran that query with no errors, gave me a few rows.

Posted by Jim Lynch on
One thing to be aware of, is when lib/*.tcl files are added or changed, a restart of the service is one of the things that will read those files into the openacs instance. problem can come up there: how does one know whether the new procs or the old are needed for the upgrade... because it could be dufferent for different situations
At least in my instalation (version control with git and some custom code) it was easy to check file system source code (I mean, files). I'm having some problems but it is a "merge" problem

But with db functions is hard. Besides of I'm not a pgsql developer, I don't know what versions are installed (I have to check versus upgrade scripts)

I'll run upgrade scripts (psql -f) and report here what I've seen.

It could be a "particular problem" with my installation (but perhaps useful for some old installations that wants to upgrade). 5.8 has been a big improvement for me (performance, cleanup, upgrade postgres and so on)

Posted by Gustaf Neumann on
The upgrade script can change the database in arbitrary ways (adding/deleting tuples/attributes/tables, ... and functions), so there is not much we can actually do to determine automatically, how much was executed. Keeping track of the names of the upgrade scripts could just help in the future, but of course not for not for the past (i.e. your case)...

The problem is especially bad, when one upgrades from really old versions, and an early upgrade script fails.

Yes, I suppose there is no easy solution but it might help to upgrade old versions (but it depends of every customer).

Ideally, I think the best approach would be abort the upgrade based in:
1. Some error (catching error from psql)
2. Some checking (versions or some audits as table approach you've mentioned)

I don't, only some ideas. Any way, perhaps I'm the only one with this problem and it doesn't worth 😊

Posted by Patrick Giagnocavo on
It would take some work, but I would think you could wrap each SQL statement with something that stuffed the SQL statement into a table with a timestamp, and a result code . Then you would have a record of all SQL run during the upgrade process and whether it threw an error or not.
Yes, as Gustaf said before perhaps this is the better approach. It could be useful in upgrading older versions
I've run "manually" (I mean psql -f packages/acs-kernel/sql/postgresql/upgrade/upgrade-5.7*). And I did it carefully IN ORDER. I see only 2 errors (it seems not important to my installation):

psql:upgrade-5.7.0d3-5.7.0d4.sql:118: ERROR:  update or delete on table "acs_datatypes" violates foreign key constraint "dtype_wdgt_tmpl_dtype_fk" on table "dtype_db_datatypes"
DETAIL:  Key (datatype)=(money) is still referenced from table "dtype_db_datatypes".

invalid command \
psql:upgrade-5.7.0d8-5.7.0d9.sql:76: ERROR:  syntax error at or near "return"
LINE 46:         return v_funcdef;

But (in my dev server) everything seems to be fine. I'm going to do it in my production server and keep an eye on logs.

It seems to be quite safe to upgrade scripts "manual" whenever upgrade in order. It is the first time I do this and I was a bit feary 😊

I had this package (survey) running OK. Thanks so much

Posted by Gustaf Neumann on
The first error is harmless, the second one is not essential either, but comes from to changes in postgres 9. You can fix this by loading the definition of the failing function "get_func_definition" from packages/acs-kernel/sql/postgresql/postgresql.sql

connect to the db with psql, and paste the following command, and this is fixed as well.

-- procedure get_func_definition/2
CREATE OR REPLACE FUNCTION get_func_definition(
   fname varchar,
   args oidvector
) RETURNS text AS $$
        nargs           integer default 0;
        v_pos           integer;
        v_funcdef       text default '';
        v_args          varchar;
        v_one_arg       varchar;
        v_one_type      varchar;
        v_nargs         integer;
        v_src           text;
        v_rettype       varchar;
        select proargtypes, pronargs, number_src(prosrc), 
               (select typname from pg_type where oid = p.prorettype::integer)
          into v_args, v_nargs, v_src, v_rettype
          from pg_proc p 
         where proname = fname::name
           and proargtypes = args;

         v_funcdef := v_funcdef || '
create or replace function ' || fname || '(';

         v_pos := position(' ' in v_args);

         while nargs < v_nargs loop
             nargs := nargs + 1;
             if nargs = v_nargs then 
                 v_one_arg := v_args;
                 v_args    := '';
                 v_one_arg := substr(v_args, 1, v_pos - 1);
                 v_args    := substr(v_args, v_pos + 1);
                 v_pos     := position(' ' in v_args);            
             end if;
             select case when nargs = 1 
                           then typname 
                           else ',' || typname 
                         end into v_one_type 
               from pg_type 
              where oid = v_one_arg::integer;
             v_funcdef := v_funcdef || v_one_type;
         end loop;
         v_funcdef := v_funcdef || ') returns ' || v_rettype || E' as ''\n' || v_src || ''' language ''plpgsql'';';

        return v_funcdef;

$$ LANGUAGE plpgsql stable strict;

glad we have a happy end!
I've done it without errors.

Thanks 😉

Posted by Gustaf Neumann on
lib/*.tcl files are sourced on every request; these are not relevant for upgrades. It is true that the developer has to take care about the tcl functions called from the various callbacks (stemming from tcl/*-procs.tcl), but that is a different story than the discussed problem set (upgrading the database and the database functions in connection with failed/halfdone upgrades).