Topic: Feedback on the update process

I'm curious if any of you have any feedback on the 1.2 to 1.3 update process. Has anyone had any issues with the character set conversion? I committed a fix for the update script yesterday, so if you have had issues, please grab the latest version of the update script and try again.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Re: Feedback on the update process

Hello, sorry for my english...

I test the last release of db_update.

My database is encoded in latin1_swedish_ci

The result of conversion seems wrong. exemple : http://box-news.fr/viewtopic.php?id=33

3 (edited by K-Ray 2008-02-10 10:26)

Re: Feedback on the update process

It doesn't seem to work with sqlite.
From what I have found out the script gets stuck everytime an 'alter table' comes around.

Also the sqlite.php is missing the 'replace' piece in the query_build function.

my setup is
Lighttpd 1.4.18-1
PHP 5.2.5
SQLite 2.8.17
punbb 1.3-rev.1478

Re: Feedback on the update process

achtungbaby wrote:

Hello, sorry for my english...

I test the last release of db_update.

My database is encoded in latin1_swedish_ci

The result of conversion seems wrong. exemple : http://box-news.fr/viewtopic.php?id=33

Difficult for me to say what has gone wrong. How did you make the "copy" of the database to test on?

K-Ray wrote:

It doesn't seem to work with sqlite.
From what I have found out the script gets stuck everytime an 'alter table' comes around.

Hmm. It shouldn't run any ALTER TABLE's for SQLite at all. Do you think you could try to track down in more detail where it fails? Does it produce an error?

K-Ray wrote:

Also the sqlite.php is missing the 'replace' piece in the query_build function.

Smartys fixed that.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

5

Re: Feedback on the update process

Reporting first test : updated a little French database (so with accentuated caracters in it)

- no errors, no warnings..
- the tables have been updated (latin1_general_ci -> utf8_general_ci), except 2 : forums_perms and subscriptions (not a big deal, but why ?).
- the new fields (like first_post_id in topics) and/or new indexes have been created.
- BUT the data HAVE NOT been converted to utf8... (accents remain in latin1 format)

Looked at db_update.php to find an 'obvious' reason... not found anything  : i couldn't say why the utf8_encode() didn't work.

Re: Feedback on the update process

Mpok wrote:

- the tables have been updated (latin1_general_ci -> utf8_general_ci), except 2 : forums_perms and subscriptions (not a big deal, but why ?).

Because those tables don't have any text/varchar fields, only integers.

Mpok wrote:

- BUT the data HAVE NOT been converted to utf8... (accents remain in latin1 format)

Looked at db_update.php to find an 'obvious' reason... not found anything  : i couldn't say why the utf8_encode() didn't work.

What was the old encoding, ISO-8859-1?

"Programming is like sex: one mistake and you have to support it for the rest of your life."

7 (edited by K-Ray 2008-02-11 20:00)

Re: Feedback on the update process

Rickard wrote:

Hmm. It shouldn't run any ALTER TABLE's for SQLite at all. Do you think you could try to track down in more detail where it fails? Does it produce an error?

This is what I get:
An error was encountered
The error occurred on line 354 in D:\server\web\pun\include\dblayer\sqlite.php
Database reported: SQL logic error or missing database (Errno: 1).

Which really gives you nothing much, so I added 
define('PUN_SHOW_QUERIES', 1);
inside the db_update.php which gave me it bit more to go on:

Failed query: ALTER TABLE extensions ADD uninstall_note TEXT

The db_update.php calls the add_field function several times and that function in sqlite.php sure does call an ALTER TABLE.

8

Re: Feedback on the update process

Rickard wrote:

Because those tables don't have any text/varchar fields, only integers.

Yep, i know, that's why i said "it's not a big deal", but for consistency only, i think it can be done anyway...

Rickard wrote:

What was the old encoding, ISO-8859-1?

Yes (what else ?)
I will try to redo the update in a few days (putting some 'echo' in db_update to really see what occurs).
For now, in order to continue the 1.3 beta test, i had to completly reconstruct my datas (based on a plain text save) and to do the encoding myself : i used the utf8_encode() function and it worked, so i clearly have no idea of what was going wrong with db_update (the difference is that db_update take the data from the old tables and my 'reconstruction' script take the data from a plain text file).

Re: Feedback on the update process

Mpok wrote:
Rickard wrote:

Because those tables don't have any text/varchar fields, only integers.

Yep, i know, that's why i said "it's not a big deal", but for consistency only, i think it can be done anyway...

Agreed, for consistency we should have it (since new tables will have it).

Re: Feedback on the update process

K-Ray: it actually should be running an alter table: SQLite certainly claims to support them in the documentation I'm seeing. Of course, the field_exists function could be failing: is there any way you could check if that's the case?

Re: Feedback on the update process

My mistake. Of course it support alter table. It's quite limited though. Only rename table and add column.

Mpok: Any information you can provide that would lead us to a fix would be greatly appreciated.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

12

Re: Feedback on the update process

As far as I know ALTER TABLE is NOT supported in the 2.x versions of sqlite.

I have found this in the sqlite changes file:
2005 March 21 (3.2.0)
    * Added support for ALTER TABLE ADD COLUMN.

13

Re: Feedback on the update process

Smartys wrote:

K-Ray: it actually should be running an alter table: SQLite certainly claims to support them in the documentation I'm seeing. Of course, the field_exists function could be failing: is there any way you could check if that's the case?

Problem with the sqlite documentation is that it is mostly unclear about which version is applicable to the documentation.

Re: Feedback on the update process

K-Ray wrote:

As far as I know ALTER TABLE is NOT supported in the 2.x versions of sqlite.

yes if memory serves that is right. there are ways around it, but they aren't easy.

my mind is on a permanent tangent
byUsers forum

Re: Feedback on the update process

Whoops smile I think you guys are right. We added SQLite support in 1.2 and after that, I don't believe we've ever had to do an ALTER TABLE.

I other words. In order to support SQLite upgrades from 1.2, we'll have to include our own implementation of ALTER TABLE. Does anyone know of a good one? The only one I found was this one:

http://code.jenseng.com/db/

But it's huge.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Re: Feedback on the update process

it will need testing no matter what solution is decided upon.

http://www.perturb.org/display/entry/645/

has the simplest I have seen, and may be a good starting point.

my mind is on a permanent tangent
byUsers forum

Re: Feedback on the update process

Rich Pedley wrote:

it will need testing no matter what solution is decided upon.

http://www.perturb.org/display/entry/645/

has the simplest I have seen, and may be a good starting point.

Yeah, that's the procedure. However, wrapping code around it to work with any existing table is the tricky part. For example, the tables created must include the full definition of the new table (the old fields and the new field).

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Re: Feedback on the update process

yes but without alter table I don't see any alternative than doing that. Slow and cumbersome work arounds may be the only choice.

my mind is on a permanent tangent
byUsers forum

Re: Feedback on the update process

Rich Pedley wrote:

yes but without alter table I don't see any alternative than doing that.

There is always an alternative. In this case it is not supporting 1.2->1.3 upgrades for SQLite. I'm not saying this is what we should do, but it's an alternative we need to consider.

"Programming is like sex: one mistake and you have to support it for the rest of your life."

Re: Feedback on the update process

I had to use sqlite once, and hated it. I am glad I no longer have to. So it doesn't affect me.

Could sqlite support be demoted to an extension? Best of both worlds perhaps.

my mind is on a permanent tangent
byUsers forum

21

Re: Feedback on the update process

Rickard wrote:

Mpok: Any information you can provide that would lead us to a fix would be greatly appreciated.

So, i've restored my old 1.2 database and tried the update again (this time, looking carefully what occured).

Note : config = MySql 5.0, converting from ISO-8859-1 on a french server.

-> all seems ok until "stage conv_tables" (so the convert_to_utf8() function works fine).
And then, the conversion of the text columns (the 'CHANGE' ... text -> blob -> text) seems somehow to REVERSE all the work done in previous stages (conv_topics, conv_posts, etc...).
This is why when viewing data in phpMyadmin the conversion seems to not have be done (and obviously the display in the forums isn't ok, accentuated characters appear as '?').

I managed to have a correct display in the forums : replacing the double 'CHANGE...' thing in convert_table_utf8() function by a single 'MODIFY... CHARACTER SET utf8'.
Now, my data are well displayed, but i'm really not sure it is a "fix"...

I will try tomorrow another test on another server (MySql 4.1.22), cause i'm thinking the problem could be related to server configuration (default charset used by the server).

22

Re: Feedback on the update process

Rickard wrote:

I other words. In order to support SQLite upgrades from 1.2, we'll have to include our own implementation of ALTER TABLE. Does anyone know of a good one? The only one I found was this one:
http://code.jenseng.com/db/
But it's huge.

I have done a simple ADD part in about 30 lines of code. It still needs some testing though.

23

Re: Feedback on the update process

Mpok wrote:

I will try tomorrow another test on another server (MySql 4.1.22), cause i'm thinking the problem could be related to server configuration (default charset used by the server).

Got the same issue with this different server/configuration.

Another pbm (not related to the previous one) :
in stage "conv_users", u start at id=2. Ok.
But in good localizations, the word 'Guest' had been translated (username, password and email).
So the guest user needs to be converted.
I added the following code (beginning of stage "conv_users") :

        // Convert 'Guest' user
        echo 'Converting guest user …<br />'."\n";
        $result = $db->query('SELECT username, password, email FROM '.$db->prefix.'users WHERE id=1') or error(__FILE__, __LINE__);
        $cur_item = $db->fetch_assoc($result);
        if (convert_to_utf8($cur_item['username'], $old_charset) | convert_to_utf8($cur_item['password'], $old_charset) | convert_to_utf8($cur_item['email'], $old_charset))
            $db->query('UPDATE '.$db->prefix.'users SET username=\''.$db->escape($cur_item['username']).'\', password=\''.$db->escape($cur_item['password']).'\', email=\''.$db->escape($cur_item['email']).'\' WHERE id=1') or error(__FILE__, __LINE__);

Re: Feedback on the update process

The Guest user should not need to be converted because nobody should have been touching that account wink

25 (edited by yemgi 2008-02-15 11:34)

Re: Feedback on the update process

Well, in French, Guest is called Invité so you need to change the username when translating if you do it properly and unfortunately, this username has a special character