Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#24800 closed defect (bug) (fixed)

DROP TABLE error 1051 (Unknown table)

Reported by: soulseekah's profile soulseekah Owned by: nacin's profile nacin
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Unit Tests Keywords: has-patch
Focuses: Cc:

Description

When dropping a table via $wpdb->query the query is rewritten to its temporary counterpart DROP TEMPORARY TABLE.

http://core.trac.wordpress.org/browser/tests/trunk/includes/testcase.php#L55

However, all CREATE TABLE queries are not rewritten to CREATE TEMPORARY TABLE, so when the time comes the database shoves an error into the face of anyone who's trying to drop a table.

This is currently only relevant to custom tests extending from the WP_UnitTestCase class.

Attachments (4)

24800.tests.diff (577 bytes) - added by soulseekah 11 years ago.
24800.diff (933 bytes) - added by soulseekah 11 years ago.
24800.2.diff (1.6 KB) - added by nacin 11 years ago.
24800.3.diff (697 bytes) - added by jdgrimes 10 years ago.

Download all attachments as: .zip

Change History (12)

#1 @soulseekah
11 years ago

Again, just to clarify: the issue is in the WP_UnitTestCase, which doesn't allow custom code to CREATE and DROP tables. This is relevant for custom plugin and theme tests making use of the provided WordPress test case class.

@soulseekah
11 years ago

@nacin
11 years ago

#2 @nacin
11 years ago

The create filter wasn't as careful as the drop filter, as it was working on only dbDelta creation queries, not any queries that go through $wpdb.

24800.2.diff (untested) makes both filters much more careful.

#3 @soulseekah
11 years ago

Related tests seem to pass fine (unrelated current failing tests reside in XMLRPC, formats and media).

#4 follow-up: @nacin
10 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 27041:

Test framework: Parse CREATE TABLE queries the same way we do DROP TABLE queries.

props soulseekah.
fixes #24800.

#5 in reply to: ↑ 4 @jdgrimes
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to nacin:

In 27041:

Test framework: Parse CREATE TABLE queries the same way we do DROP TABLE queries.

props soulseekah.
fixes #24800.

This has broken some things. I noticed that some unit tests for one of my plugins were giving strange results. In one unit test for multisite, a blog would be created using $this->factory->blog->create(). Then stuff would fly apart in the next unit test, because for some reason the changes were being committed instead of rolled-back by MySQL. I eventually traced this down to the fact that the CREATE TABLE queries for new a new blog will have a leading newline. That is causing the CREATE TABLE query not to be converted to a CREATE TEMPORARY TABLE query. And a CREATE TABLE query will automatically cause a commit. It worked before, because it was a simple str_replace(). Now it will ignore the query if there is leading whitespace.

I'll attach a patch with a possible fix.

Last edited 10 years ago by jdgrimes (previous) (diff)

@jdgrimes
10 years ago

#6 @jdgrimes
10 years ago

  • Keywords has-patch added

Also, note that the names of these functions were changed (the plural was dropped), so anybody who is hacking them (e.g., for testing plugin install/uninstall), will have to update.

#7 @nacin
10 years ago

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

In 27086:

Test suite: Trim queries before deciding whether to create temporary tables.

props jdgrimes.
fixes #24800.

#8 @nacin
10 years ago

  • Milestone changed from Awaiting Review to 3.9

I chose to trim() inside the checks because then we would not be trimming the whitespace off all queries, only CREATE/DROP TABLE ones. The second trim is not a problem as it'll only run for these kinds of queries.

Note: See TracTickets for help on using tickets.