Geeklog-Core / geeklog

Geeklog - The Secure CMS.
https://www.geeklog.net
24 stars 19 forks source link

Combine User Table Upgrade Process errors out #1128

Closed eSilverStrike closed 2 years ago

eSilverStrike commented 2 years ago

I get the following error when upgrading a Geeklog site to 2.2.2. It has 414 users. The select statement in update_CombineUserTables222 works but the insert fails on line 262

$userAttributeDAO->create($entity);

( ! ) Fatal error: 1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ' 0, '', , , '', , , )' at line 5 in C:\wamp64\www\dsrtest\system\classes\Database\DbMysqli.php on line 517

1 | 0.0001 | 411296 | {main}( ) | ...\index.php:0 2 | 0.0004 | 434088 | Installer->run( ) | ...\index.php:57 3 | 0.0005 | 436184 | Installer->installEngine( ) | ...\installer.class.php:1168 4 | 0.0005 | 436360 | Geeklog\Install\Migrate->step4( ) | ...\installer.class.php:1044 5 | 0.0216 | 511792 | Geeklog\Install\Migrate->doDatabaseUpgrades( ) | ...\Migrate.php:710 6 | 13.1601 | 585024 | update_CombineUserTables222( ) | ...\Common.php:1607 7 | 16.3963 | 1089640 | Geeklog\DAO\UserAttributeDAO->create( ) | ...\mysql_2.2.1_to_2.2.2.php:262 8 | 16.3964 | 1090024 | DB_query( ) | ...\UserAttributeDAO.php:85 9 | 16.3964 | 1090024 | Geeklog\Database\DbMysqli->dbQuery( ) | ...\lib-database.php:233 10 | 16.3974 | 1090296 | trigger_error ( ) | ...\DbMysqli.php:517

After digging around for an hour I figured it out. Record uid 8127 didn't exist in the user table but had records in gl_usercomment and gl_userindex for some reason. Not sure how this happened but it did and causes update_CombineUserTables222 to fail as half the columns from the select statement was null for the record.

Wondering how the best way to find and delete these records.... Maybe a cleaning process should be done first on these tables to remove any orphan records or add any missing records?

I also do not want to slow this process down any more as Geeklog.net has over 7500 users and this already takes long enough (and I worry about PHP timing out)...

@mystralkk any ideas?

I am thinking of just adding

$sql = "DELETE uc FROM {$_TABLES['usercomment']} uc WHERE uid NOT IN (SELECT uid FROM {$_TABLES['users']} u WHERE uc.uid = u.uid)";

for those 4 user tables to clean up orphan records at the beginning of update_CombineUserTables222

eSilverStrike commented 2 years ago

Okay I added the delete statements... anything else we should do?

I am worried that this process may timeout during Geeklog.net because of the loop. Maybe the Select statement needs to be turned into "Insert into user_attributes" and a select statement. The problem with this is that it doesn't do any data checking that you do.

mystralkk commented 2 years ago

Okay I added the delete statements... anything else we should do?

No, just adding DELETE statement is enough.

I am worried that this process may timeout during Geeklog.net because of the loop. Maybe the Select statement needs to be turned into "Insert into user_attributes" and a select statement. The problem with this is that it doesn't do any data checking that you do.

I believe doing without data checking will just delay detecting erroneous data, but timeout must be avoided at any cost. So, changing the loop into one "INSERT INTO gl_user_attributes SELECT * FROM ..." will be better.

eSilverStrike commented 2 years ago

Okay I tested this SQL on an actual DB with almost 1000 users and it seems to work fine

  INSERT INTO gl_user_attributes 
        SELECT f.uid, 
        IFNULL(c.commentmode, 'nested'), IFNULL(c.commentorder, 'ASC'), IFNULL(c.commentlimit, 100),
        IFNULL(x.etids, '-'), IFNULL(x.noboxes, 0), IFNULL(x.maxstories, 0), 
        IFNULL(f.about, ''), IFNULL(f.location, ''), IFNULL(f.pgpkey, ''), IFNULL(f.tokens, 0), IFNULL(f.lastgranted, 0), IFNULL(f.lastlogin, 0), 
        IFNULL(p.dfid, 0), IFNULL(p.advanced_editor, 1), IFNULL(p.tzid, ''), IFNULL(p.emailfromadmin, 1), IFNULL(p.emailfromuser, 1), IFNULL(p.showonline, 1) 
        FROM gl_usercomment AS c 
        LEFT JOIN gl_userindex AS x ON c.uid = x.uid
        LEFT JOIN gl_userinfo AS f ON c.uid = f.uid
        LEFT JOIN gl_userprefs AS p ON c.uid = p.uid

@mystralkk Do you see any problems? Any nulls I have it just setup to use the defaults from the user_attributes table. If you agree I will just comment out your existing code and add in the code for this SQL statement.

Also I notice the user_attributes doesn't specify defaults for the columns etids, about, and pgpkey. I will add defaults to those columns as well in the table create (same defaults as in the above statement) unless there is a reason not to?

mystralkk commented 2 years ago

@mystralkk Do you see any problems? Any nulls I have it just setup to use the defaults from the user_attributes table. If you agree I will just comment out your existing code and add in the code for this SQL statement.

I agree.

Also I notice the user_attributes doesn't specify defaults for the columns etids, about, and pgpkey. I will add defaults to those columns as well in the table create (same defaults as in the above statement) unless there is a reason not to?

The columns etids, about, and pgpkey are of TEXT type, so you cannot assign a default value.

https://dev.mysql.com/doc/refman/5.6/en/data-type-defaults.html

eSilverStrike commented 2 years ago

Notice in Geeklog.net that 17 users are missing a user attribute record.

Because of this these user profiles are not viewing on the front end or editable in the backend. It also seems to create errors with the forum when viewing topics the user has posted in (and probably other issues if they have comments, etc...)

Looks like due to unknown errors in the past users may not have a usercomment record so the current statement would not have added a user attribute record. Need to add an additional SQL statement to add in any missing user attribute records.