Use the same data type for site statuses

[ocean90]

For status archived we use:

archived enum('0','1') NOT NULL default '0',

Fo the others:

public tinyint(2) NOT NULL default '1',
mature tinyint(2) NOT NULL default '0',
spam tinyint(2) NOT NULL default '0',
deleted tinyint(2) NOT NULL default '0',

Shouldn't we use the same data type for the others?

[nacin]

I'd have no problem doing this, but the way the schema is baked into the pluggable install_network(), instead of a global...

make_db_current() should be running any updates to the MS tables as well. This seems like something we definitely need to handle before release, because right now changes to the schema will be very convoluted.

[wpmuguru]

There are plugins that set some of those flags to something other than 0 or 1. If there is a consensus to change this to make them a consistent datatype then the archived flag should be changed to a tinyint.

[nacin]

Replying to wpmuguru:

There are plugins that set some of those flags to something other than 0 or 1. If there is a consensus to change this to make them a consistent datatype then the archived flag should be changed to a tinyint.

I agree. We eliminated enum fields in 2.5, this one should go too.

I'm leaving this in 3.0 because I think we need to make some wider adjustments. Any suggestions on how we can improve MS schema upgrades?

Is there a reason for install_network() to be pluggable? Or specifically, for the table definitions to be pluggable?

[wpmuguru]

I'm not sure which ticket the checkboxes for site status were added, but they need to be switched back to radio buttons. If the checkboxes are not checked and the super admin updates the site's options, the tiny int values are set to 0 (even though before editing they had a value of something other than 0 or 1).

[wpmuguru]

Replying to nacin:

Is there a reason for install_network() to be pluggable? Or specifically, for the table definitions to be pluggable?

That was pluggable in MU. If someone wants to add columns to global tables, the schemas have to be pluggable so that a dbDelta in a subsequent upgrade doesn't drop the columns.

[nacin]

Replying to wpmuguru:

Replying to nacin:

Is there a reason for install_network() to be pluggable? Or specifically, for the table definitions to be pluggable?

That was pluggable in MU. If someone wants to add columns to global tables, the schemas have to be pluggable so that a dbDelta in a subsequent upgrade doesn't drop the columns.

We didn't call it install_network() in MU though, they were simply part of the global variable WP uses. I think we should add them back into that variable, appending them conditionally. (And also appending global terms conditionally.)

We should be able to make changes to $wp_queries and have it work. Right now we have to call install_network() in an upgrade routine, which seems odd. If someone wants to add columns, they'd tap into $wp_queries like the rest, like how it was done in MU.

[nacin]

(In [14277]) Restore radios for MS site flags, as the schema supports other values. see #12832

[westi]

We should definitely avoid ENUMs for SQL schema we made that decision a long time ago to make it more db agnostic.

Need to read around in more detail to answer the other questions.

[wpmuguru]

Per the IRC discussion, the global tables were excluded from upgrades in MU beginning with 2.8 (​http://trac.mu.wordpress.org/changeset/1797/trunk/wp-admin/includes/schema.php).

So, IRT the 3.0 merge feature/enhancement, we don't need to include install_network() in the upgrade process.

I agree that we should eliminate the enum and incorporate the global tables into the upgrade process. For 3.0, we could change the network schema away from the enum because it would not affect existing MU installs, and put the incorporate the schema in the upgrade process on the list for 3.1.

2 weeks isn't much time to warn MU install owners that if they have changed their global tables they have to create a custom version of install_network to preserve their changes.

[nacin]

Works for me. Does install_network() even need to be pluggable in 3.0 then?

[ryan]

Let's be conservative with schema changes in 3.0. Postponing to 3.1, where we can discuss how to handle schema changes to global ms tables.

[hakre]

October is starting. Any feedback for 3.1 line-up? Postponing to 3.2?

[nacin]

We currently have no standard way to trigger an upgrade on global table schemas. At any real scale it isn't something we should be running.

We made a commitment to no schema changes in 3.1. Even as just a default on future sites, let's hold off until 3.2.

[ocean90]

Re-visit for 3.4?

[jeremyfelt]

Moving to 3.7 for discussion, do we have a nicer way to modify schema now?

[nacin]

Replying to jeremyfelt:

Moving to 3.7 for discussion, do we have a nicer way to modify schema now?

We do, but I'm not sure if ENUM converts to an integer field as smoothly as it does a varchar.

[pento]

Replying to nacin:

We do, but I'm not sure if ENUM converts to an integer field as smoothly as it does a varchar.

It doesn't. When an ENUM converts to an INT, it returns the ENUM index, not the CAST(... AS INT) value of the ENUM. In the case of the archived column, ENUM '0' has an index of 1, and ENUM '1' has an index of 2.

We could totally convert it to VARCHAR in 3.7, then INT in 3.8, though.

[nacin]

Replying to pento:

It doesn't. When an ENUM converts to an INT, it returns the ENUM index, not the CAST(... AS INT) value of the ENUM. In the case of the archived column, ENUM '0' has an index of 1, and ENUM '1' has an index of 2.

That's what I assumed.

We could totally convert it to VARCHAR in 3.7, then INT in 3.8, though.

Unfortunately, we can't do a two-step like that. The upgrade routine is designed to work in layers, version by version, such that you could upgrade across many major versions at once and still be OK. The schemas + dbDelta() wouldn't know, of course, that one needs to do a double switch.

12832.diff​ tries doing this in one shot, in pre_schema_upgrade(). It actually bypasses dbDelta entirely, by doing both ALTER queries at the same time.

[nacin]

In 25448:

Multisite blogs table: Convert the archived field from enum to tinyint to match the other status fields. fixes #12832.