upgrade.php script clobbers changes to log_online table

[Porksifal]

Hi!

I've made some fixes to the SMF 2.0 release series for my own purposes, a number of which relate to SMF being completely broken for IPv6 users out of the box. To complement the code changes, two database changes have been necessary in the log_online table:

• First, the "session" column needs to be made longer than 32 characters (I just went nuts and made it 100, but 41 should be the theoretical minimum value required).
  
• Second, the "ip" column needs to be made into a type big enough to store an IPv6 address (I'm just using PostgreSQL's "inet" type; I have no idea what you'd use here for MySQL).
  
  In any case, I understand the development focus is on SMF 2.1 and am happy to continue maintaining IPv6 compatibility myself, but the upgrade.php script for SMF 2.0.7 breaks these changes by dropping and recreating the log_online table. There was no warning that this would be the case; in fact, the documentation explicitly says to use upgrade.php from a web browser instead of trying to fiddle with the SQL scripts directly.
  
  I don't know what the appropriate fix would be, not being familiar with your policy on what upgrade.php should do, but at least providing a warning that local database changes may not be preserved would be appreciated.
  
  As an aside, all of our customisations to SMF 2.0 are available on GitHub (some of which are not relevant to any other installation, but some of which might be useful):
  
  hxxp:github.com/theflatearthsociety/forum.tfes.org [nonactive]
  
  I'm not going to be publishing any of these changes as mods, as we have our own git-based deployment method that is simpler to audit and easier to revert than the mod installation tool, so anyone who is interested in IPv6 support or other enhancements we've made is welcome to trawl that repo and fish out any tasty bits.
  
  If you'd like any further information about our particular setup, please let me know.

[Illori]

there is a mod for ipv6 already http://custom.simplemachines.org/mods/index.php?mod=3051 and similar code changes have been made in 2.1 to support ipv6 out of the box.

[Porksifal]

there is a mod for ipv6 already http://custom.simplemachines.org/mods/index.php?mod=3051 and similar code changes have been made in 2.1 to support ipv6 out of the box.

At a glance, this mod doesn't solve the issues with the log_online table this report is in relation to. I'm guessing they're PostgreSQL-specific, in that case; presumably that mod has only been tested with MySQL.

[margarett]

Most likely, yes.

If you are already on 2.0.7 you don't need to worry anymore. Until 2.1 (that already supports IPV6) you shouldn't need to touch your DB anymore, as the incremental updates (patches) usually don't touch the DB. If a new 2.0.x version is released, instead of the large upgrade, you should patch your forum:
http://wiki.simplemachines.org/smf/Patching

[Porksifal]

If you are already on 2.0.7 you don't need to worry anymore. Until 2.1 (that already supports IPV6) you shouldn't need to touch your DB anymore, as the incremental updates (patches) usually don't touch the DB. If a new 2.0.x version is released, instead of the large upgrade, you should patch your forum:
http://wiki.simplemachines.org/smf/Patching

We deploy code directly from our git repository, which should be the same operation as patching. This forum was originally installed on 2.0.2 or 2.0.3 (I forget the specific version), and was initially patched manually with no database changes, but I found that caused compatibility problems due to changes to the database schema. Since then, I've been manually running upgrade.php to update the database after each patch.

Looking at upgrade.php in the patch tarball, it does specifically run upgrade_2-0_postgresql.sql if the forum version is less than 3.0, which implies that this is needed even for patchlevel upgrades. That file contains the following, which is what causes the problem:

Code Select Expand


---# Converting "log_online".
ALTER TABLE {$db_prefix}log_online DROP CONSTRAINT {$db_prefix}log_online_log_time;
ALTER TABLE {$db_prefix}log_online DROP CONSTRAINT {$db_prefix}log_online_id_member;
DROP TABLE {$db_prefix}log_online;
CREATE TABLE {$db_prefix}log_online (
  session varchar(32) NOT NULL default '',
  log_time int NOT NULL default '0',
  id_member int NOT NULL default '0',
  id_spider smallint NOT NULL default '0',
  ip int NOT NULL default '0',
  url text NOT NULL,
  PRIMARY KEY (session)
);
CREATE INDEX {$db_prefix}log_online_log_time ON {$db_prefix}log_online (log_time);
CREATE INDEX {$db_prefix}log_online_id_member ON {$db_prefix}log_online (id_member);
---#


It seems strange that this would be shipped with the patch update but not required.

[margarett]

I don't see that...

This is the patch:
http://custom.simplemachines.org/mods/downloads/smf_patch_2.0.7.tar.gz
There is no *.sql file inside, so it can't run it.

This is the large upgrade:
http://download.simplemachines.org/index.php?thanks;filename=smf_2-0-7_upgrade.zip
And this, yes, contains instructions to run that file.

Patch updates are really like MODs. The database schema isn't changed since 2.0 RC-something The only thing that the patch upgrade *might* do (and the latest ones don't do it) is to change the version number in smf_settings table.

[Porksifal]

This is the patch:
http://custom.simplemachines.org/mods/downloads/smf_patch_2.0.7.tar.gz
There is no *.sql file inside, so it can't run it.

Sorry, you're right. I have no idea what I was looking at yesterday.

Patch updates are really like MODs. The database schema isn't changed since 2.0 RC-something The only thing that the patch upgrade *might* do (and the latest ones don't do it) is to change the version number in smf_settings table.

OK, I've reviewed my chat logs where I was discussing this with a friend a few months ago and my memory was slightly faulty.

We saw an error with a notice that the code was at version 2.0.6, with the database at version 2.0.2, and it suggested that running upgrade.php might help. It didn't, but since then I've been running upgrade.php on every upgrade, on the assumption that since the forum gave me that warning, there are differences between the schemas (otherwise running upgrade.php would never help, and the warning would be pointless).

The particular error actually turned out to be a MySQLism that breaks smiley management with PostgreSQL, which was fixed here in SortSmileyTable:

hxxp:github.com/theflatearthsociety/forum.tfes.org/commit/deccbeb38744d07e9c12a4c28136f92c14177271 [nonactive]

Sorry for not providing more detail upfront. It seems like the upgrade.php issue isn't relevant, although the forum was somewhat misleading in suggesting that I should run it. There's also the issue fixed by the patch I linked above, which I would consider a bug since SMF claims to support PostgreSQL.

[margarett]

Yes, that annoying message suggesting upgrade.php is really misleading.

[butchs]

there is a mod for ipv6 already http://custom.simplemachines.org/mods/index.php?mod=3051 and similar code changes have been made in 2.1 to support ipv6 out of the box.

I have been looking into this.  The mod is cool and all but it appears to be doing it the hard way...  Using variants of "inet_ntop" and "inet_pton"  converted to hex before saving to the DB gives the impression that the address can be saved in a single VARBINARY(39).  Here are some cool links:

http://www.highonphp.com/tag/inet_pton
http://stackoverflow.com/questions/12095835/quick-way-of-expanding-ipv6-addresses-with-php
http://php.net/manual/en/function.ip2long.php

Personally I prefer to see SMF go this way.  A single VARBINARY hex unpacked binary string for both IPv4 and IPv6.  I believe it is possible, as I am working on that direction in the next release of FF...