WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#14508 closed task (blessed) (fixed)

Always require wp-db.php

Reported by: nacin Owned by: westi
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: Database Keywords:
Focuses: Cc:

Description

Let's change how require_wp_db() works for performance reasons. Always include wp-db.php, then include db.php if it exists, then instantiate wpdb directly in wp-settings.php as we currently do at the end of wp-db.php. This logic would all take place in wp-settings.php and we can do away with require_wp_db() in the process.

Attachments (2)

14508.diff (2.3 KB) - added by ryan 5 years ago.
14508.2.diff (3.1 KB) - added by nacin 5 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 @westi5 years ago

  • Owner set to westi
  • Status changed from new to accepted

comment:2 @ryan5 years ago

This would help hyperdb, which currently has to do some tricks to get wp-db.php included so that it can use it as a base class.

comment:3 @tomarq5 years ago

wp-db.php is not always loaded. wp-db.php is only loaded if db.php doesn't exist. I use db.php to override several setting somewhat like the wp-hive plugin does. Please don't break these types of plugins.

comment:4 follow-up: @nacin5 years ago

We're not actually setting the $wpdb variable by including wp-db.php. Most drop-ins won't be affected. We're improving performance by eliminating an open() syscall, and we're also making it easier to extend wpdb. In fact the only thing we're preventing is a db drop-in that defines its own wpdb, which is unnecessary and easily fixed.

comment:5 @Denis-de-Bernardy5 years ago

Won't this break, like... soooo many sites that use db-related cache plugins?

comment:6 in reply to: ↑ 4 @Denis-de-Bernardy5 years ago

Replying to nacin:

In fact the only thing we're preventing is a db drop-in that defines its own wpdb, which is unnecessary and easily fixed.

FWIW, there are basically three types of plugins that mess with wp-content/db.php

One goes something like:

class whatever {
}
$wpdb = new whatever; // replace wpdb

The next goes:

include wpdb
class whatever extends wpdb {
}
$wpdb = new whatever; // override wpdb

The last is included as a plugin rather than from wp-content/db.php and goes:

class whatever {
}
$wpdb = new whatever($wpdb); // extend wpdb using php's magic functions

I'd personally advise to not change the assumptions under which things work; but if you must, be sure to announce it long, long in advance on the wp.org blog, so that any potential plugin author and site owner that messes around with wpdb can update his plugin accordingly. Else, it's not Joe user's site that will be broken; it'll be larger WP site owners.

comment:7 follow-up: @ryan5 years ago

These DB drop-ins break with almost every release anyway. It's tempting to add a wpdb API revision and die if db.php doesn't declare compat with that revision.

comment:8 @ryan5 years ago

Those three situations should still work. The only thing that will break is defining class named wpdb.

@ryan5 years ago

comment:9 @Denis-de-Bernardy5 years ago

Actually, the third may break if you define the db class after everything gets defined, if you include db.php, wpdb within it, and finally instantiate the mess. Not saying it's a bad thing to do so -- far from it. Rather, be sure to announce it somehow, else a lot of sites using cache plugins or custom db plugins might break. I know mine will. ;-)

comment:10 in reply to: ↑ 7 ; follow-up: @wpmuguru5 years ago

Replying to ryan:

These DB drop-ins break with almost every release anyway. It's tempting to add a wpdb API revision and die if db.php doesn't declare compat with that revision.

I think 2.8 was the first major release in quite a while where the DB wasn't changed. Switching this around so DB plugins can inherit the default wpdb class should make it so that DB plugins won't need to be updated with every release of WordPress.

+1 from this DB plugin dev :)

comment:11 in reply to: ↑ 10 @nacin5 years ago

Replying to wpmuguru:
Switching this around so DB plugins can inherit the default wpdb class should make it so that DB plugins won't need to be updated with every release of WordPress.

That's actually doable now, though HyperDB demonstrates how it is a cheap hack. (It sets $wpdb to true, then includes wp-db.php.) The performance benefit here would be for opcode caches, by taking the include of wp-db out from behind a conditional. And, of course, it makes it a lot easier to extend wpdb, which as the hack indicates isn't really supported well at the moment.

comment:12 @ikailo5 years ago

My plugin (WP Hive) currently uses db.php because it must do some work very early in the WP lifecycle. It's pretty hacky to include this functionality here, but it's the only spot that occurs after $wpdb is instantiated and before $table_prefix is set.

Optimally, it would be nice to be able to hook in separately from db.php, since many users of WP Hive also use caching plugins, and the way it is now causes db.php collisions & overwrites.

While there's still some work to do to resolve these collisions, this patch is a step in the right direction and it's a two-line fix in my plugin to adapt. +1 from me.

comment:13 @hakre5 years ago

Oh I found it nice to have an actual drop-in that can fully replace the wpdb class. With this change, the wpdb class is always loaded - regardless it's used or not.

If you want to do everybody a favor, think about providing an interface for the wpdb class for the future. It's still some time until this is possible, so it's easy to learn until then.

I think 3.1 is a great version change to break things again, because for 3.0 plugin authors are already trained for changes related to wpdb, so they can adopt fast. If you wait with this change, there probably will be more complaints.

@nacin5 years ago

comment:14 @nacin5 years ago

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

(In [15638]) Always include wp-db.php. Prevents a conditional include and allows db dropins to cleanly extend the wpdb class. Move require_wp_db() to load.php for consistency with bootloader helpers. fixes #14508.

Note: See TracTickets for help on using tickets.