Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 5 years ago

#5563 closed defect (bug) (fixed)

wp_signups table upgrade problem

Reported by: Denis-de-Bernardy Owned by:
Milestone: 2.0.1 Priority: highest
Severity: major Version: 2.0
Component: New User Experience Keywords: has-patch
Cc:

Description

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

Attachments (6)

5563.patch (8.3 KB) - added by johnjamesjacoby 6 years ago.
5563.02.patch (1.6 KB) - added by r-a-y 6 years ago.
5563.03.patch (1.6 KB) - added by r-a-y 6 years ago.
03.patch tweaks the logic for bp_core_maybe_upgrade_signups() from 02.patch
5563.04.patch (1.0 KB) - added by r-a-y 6 years ago.
5563.05.patch (820 bytes) - added by r-a-y 6 years ago.
5563.06.patch (3.2 KB) - added by johnjamesjacoby 6 years ago.
Simplify

Download all attachments as: .zip

Change History (34)

#1 @r-a-y
6 years ago

  • Component changed from Core to Update/Install
  • Summary changed from Possible BP bug during WP upgrades to wp_signups table upgrade problem

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.

#2 @johnjamesjacoby
6 years ago

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)

Version 0, edited 6 years ago by johnjamesjacoby (next)

#3 @johnjamesjacoby
6 years ago

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)

#4 @r-a-y
6 years ago

  • Keywords has-patch added

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.

@r-a-y
6 years ago

#5 @johnjamesjacoby
6 years ago

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

#6 @r-a-y
6 years ago

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.

#7 @imath
6 years ago

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

#8 @r-a-y
6 years ago

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
6 years ago

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

#9 @r-a-y
6 years ago

  • Milestone changed from Awaiting Review to 2.0.1

#10 @johnjamesjacoby
6 years ago

In 8309:

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

#11 @johnjamesjacoby
6 years ago

In 8310:

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

#12 @johnjamesjacoby
6 years ago

In 8311:

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

#13 @johnjamesjacoby
6 years ago

  • Keywords 2nd-opinion added; has-patch removed
  • Priority changed from normal to highest
  • Severity changed from normal to major

Tested same as Ray above, and now appears fixed.

Ray, able to test and report back?

#14 @r-a-y
6 years ago

  • Keywords has-patch added

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.

Last edited 6 years ago by r-a-y (previous) (diff)

@r-a-y
6 years ago

#15 @johnjamesjacoby
6 years ago

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?

#16 follow-up: @r-a-y
6 years ago

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.

Last edited 6 years ago by r-a-y (previous) (diff)

#17 in reply to: ↑ 16 @johnjamesjacoby
6 years ago

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?

Last edited 6 years ago by johnjamesjacoby (previous) (diff)

#18 @r-a-y
6 years ago

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.

Last edited 6 years ago by r-a-y (previous) (diff)

@r-a-y
6 years ago

#19 @johnjamesjacoby
6 years ago

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.)

#20 @r-a-y
6 years ago

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.

Last edited 6 years ago by r-a-y (previous) (diff)

@johnjamesjacoby
6 years ago

Simplify

#21 @johnjamesjacoby
6 years ago

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?

#22 @johnjamesjacoby
6 years ago

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)

#23 @johnjamesjacoby
6 years ago

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)

#24 follow-up: @r-a-y
6 years ago

  • Keywords 2nd-opinion removed
  • Resolution set to fixed
  • Status changed from new to closed

Thoughts?

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

#25 in reply to: ↑ 24 @johnjamesjacoby
6 years ago

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!

This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.


6 years ago

#27 @justsayno1
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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!

#28 @r-a-y
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

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.

Note: See TracTickets for help on using tickets.