wp_signups table upgrade problem

[Denis-de-Bernardy]

Any odds that this WP ticket might be related to BP?

​https://core.trac.wordpress.org/ticket/27855

WordPress database error: [Incorrect table definition; there can be only one auto column and it must be defined as a key]
ALTER TABLE wp_signups ADD COLUMN signup_id bigint(20) NOT NULL auto_increment

[r-a-y]

This is an edge case conflict with the Gravity Forms User Registration plugin.

They also create their own wp_signups table to address issues with single-site registration. However, they did not add in the changes that were made to the DB schema in WP 3.7:
​https://core.trac.wordpress.org/changeset/25179

Looks like we'll need to run WP's upgrade script ourselves.

In particular, adding in the query additions that are in upgrade.php from that changeset to our upgrader if the wp_signups table already exists.

[johnjamesjacoby]

In 8301:

Signups: Introduce functions for upgrading gthe lobal wp_signups table if an old version exists. This happens primarily on singlesite installations where third party plugins may have previously created the sign-ups table without keeping it updated.

• Bumps the DB version number to 8300.
• Moves functions for table creation and upgrade into the admin schema file.
• Moves sign-ups table creation out of activation process, and into the database upgrade process.
• No not rely on dbDelta() for these specific database upgrades (inspired by ​[WP25179])

See #5563, ​#WP27855. (2.0 branch)

[johnjamesjacoby]

In 8302:

Signups: Introduce functions for upgrading the global wp_signups table if an old version exists. This happens primarily on singlesite installations where third party plugins may have previously created the sign-ups table without keeping it updated.

• Bumps the DB version number to 8300.
• Moves functions for table creation and upgrade into the admin schema file.
• Moves sign-ups table creation out of activation process, and into the database upgrade process.
• No not rely on dbDelta() for these specific database upgrades (inspired by ​[WP25179])

See #5563, ​#WP27855. (trunk)

[r-a-y]

Thanks for getting to this before I was able to, JJJ.

Using r8302 still caused the DB error that Denis-de-Bernardy posted.

02.patch gets rid of these errors.

• I made some changes to bp_core_maybe_upgrade_signups() to check to see if the signup_id column exists in the wp_signups table. If it doesn't exist, then we proceed with upgrading the signups table.

• We also need to upgrade the signups table before installing the signups table for cases where Gravity Forms already created the table.

I was able to test on a fresh install of WP, then activating Gravity Forms / Gravity Forms User Registration, then activating BP trunk and the DB errors are gone.

One last test I want to do is on a BP 2.0 install that still has this bug and then upgrading to trunk to see the errors are gone.

[johnjamesjacoby]

How can you upgrade the signup table if you haven't created it yet?

[r-a-y]

How can you upgrade the signup table if you haven't created it yet?

Let's say Gravity Forms created the table first with the older schema and we run bp_core_install_signups() without upgrading / altering the wp_signups table.

bp_core_install_signups() uses dbDelta() and dbDelta() doesn't like the discrepancies between these two different schemas and will cause the DB error above.

[imath]

Hi,

Just noticed after updating trunk & testing an upgrade i had this Database error :

WordPress database error: [Duplicate column name 'signup_id']
ALTER TABLE wp_signups ADD signup_id BIGINT(20) NOT NULL AUTO_INCREMENT PRIMARY KEY FIRST

WordPress database error: [Can't DROP 'domain'; check that column/key exists]
ALTER TABLE wp_signups DROP INDEX domain

[r-a-y]

Hi imath, that's what I encountered myself.

02.patch should fix this.

If you have a backup, revert to it, then apply 02.patch and redo the upgrade and see if that fixes the issue.

[r-a-y]

03.patch tweaks the logic for bp_core_maybe_upgrade_signups() from 02.patch

[johnjamesjacoby]

In 8309:

Simplify signups table install & upgrade routines. See #5563. Props r-a-y. (2.0 branch)

[johnjamesjacoby]

In 8310:

Simplify signups table install & upgrade routines. See #5563. Props r-a-y. (trunk)

[johnjamesjacoby]

In 8311:

Missed a spot in the 2.0 branch's signups table upgrade routine to 2.0.1. See #5563. (2.0 branch)

[johnjamesjacoby]

Tested same as Ray above, and now appears fixed.

Ray, able to test and report back?

[r-a-y]

Almost there! Got these errors when upgrading from an existing BP 2.0 install with the wp_signups problem:

WordPress database error Duplicate column name 'signup_id' for query ALTER TABLE wp_signups ADD signup_id BIGINT(20) NOT NULL AUTO_INCREMENT PRIMARY KEY FIRST

WordPress database error Can't DROP 'domain'; check that column/key exists for query ALTER TABLE wp_signups DROP INDEX domain

04.patch should fix this.

[johnjamesjacoby]

I don't understand how that patch fixes those database errors. Looks to me like those errors weren't because of that code at all. Care to elaborate, or clear your error log and try again?

[r-a-y]

I don't understand how that patch fixes those database errors. Looks to me like those errors weren't because of that code at all. Care to elaborate, or clear your error log and try again?

$wpdb->signups is not defined in those exists() functions so the check will return false. What happens then is the upgrade or install scripts will attempt to do their thang causing the DB errors.

[johnjamesjacoby]

Replying to r-a-y:

$wpdb->signups is not defined in those exists() functions so the check will return false. What happens then is the upgrade or install scripts will attempt to do their thang causing the DB errors.

These two empty checks are inverse. If $wpdb->signups is already defined in bp_core_signups_table_exists() then we know for sure the table exists so return true (without the additional database query). If it's not defined in bp_core_signups_id_column_exists() then we know the column does not exist so return false (without the additional database query).

Seems like that's the intended sequence? Why would we want to remove those checks, and accidentally check for a column in a table that might not exist?

Would it be easier to suppress the errors inside bp_core_upgrade_signups() also? Does the existing code even work for you at all in terms of updating the signups table from the old version to the new?

I'm still fuzzy on how bp_core_upgrade_signups() is being ran if the column already exists; that's the only way those error messages would occur. Is bp_core_signups_id_column_exists() broken?

[r-a-y]

If $wpdb->signups is already defined in bp_core_signups_table_exists() then we know for sure the table exists so return true (without the additional database query).

The problem is $wpdb->signups is not defined in bp_core_signups_table_exists(). It doesn't exist on single-site for me where I've done my testing.

However, a check is done for $wpdb->signups in bp_core_signups_id_column_exists(). This is where the function is failing since $wpdb->signups doesn't exist.

05.patch is a more, accurate fix for this issue.

[johnjamesjacoby]

I still don't think this solves the problem correctly. We should not be forcing $wpdb->signups since all we're doing is checking if the signups table is already installed, not necessarily installing it (at least not at this point.)

[r-a-y]

To be clear, $wpdb->signups is only defined if it doesn't exist. 05.patch defines it after $wpdb->signups doesn't exist.

See line 464 where bp_core_signups_table_exists() bails if $wpdb->signups exists:
​https://buddypress.trac.wordpress.org/browser/trunk/bp-core/bp-core-update.php#L462

If the issue is with defining $wpdb->signups, we can code around this with a direct $wpdb->base_prefix . 'signups' in both exists() functions.

[johnjamesjacoby]

Simplify

[johnjamesjacoby]

06 removes the helper functions, since they would need to return a few different values depending on the current state of the signups table. Instead, let's suppress errors before and after, and always query for the existence of the table, and of the column to check for updates.

Thoughts?

[johnjamesjacoby]

In 8312:

Remove helper functions for assessing the state of the wp_signups table, as they are no longer used in multiple places. Instead, suppress errors and always query for the existence of the table and signup_id column before attempting to upgrade or install. See #5563. Props r-a-y. (2.0 branch)

[johnjamesjacoby]

In 8313:

Remove helper functions for assessing the state of the wp_signups table, as they are no longer used in multiple places. Instead, suppress errors and always query for the existence of the table and signup_id column before attempting to upgrade or install. See #5563. Props r-a-y. (trunk)

[r-a-y]

Thoughts?

I always prefer simplification :) Just tested and we're good to go! Marking this one as fixed.

[johnjamesjacoby]

Replying to r-a-y:

I always prefer simplification :) Just tested and we're good to go! Marking this one as fixed.

Thanks for your help!

[justsayno1]

Hi,

I am trying to appy this patch. I have tried through ssh on the server and I have also tried also looking through the target file (I assume bp-core-update.php) and trying to do it manually. However the line numbers do not seem to correspond to the original file and some of the non-changed content is different. I have the most up to date version of BP.

I admittedly have never applied a patch but some guidance from you guys would be great becasue I can't seem to figure it out. When I used patch on it I got an error:

2 out of 2 hunks FAILED -- saving rejects to file bp-core-update.php.rej

Thanks!

[r-a-y]

justsayno1 - You need to apply the changesets marked with 2.0-branch: r8309, r8311, r8312.

Or you can download the latest 2.0-branch of BuddyPress:
​https://buddypress.trac.wordpress.org/changeset/8325/branches/2.0?old_path=%2F&format=zip

Please do not reopen tickets unless you have verified that the problem still persists.