WordPress.org

Make WordPress Core

Opened 12 months ago

Closed 5 months ago

Last modified 5 months ago

#24800 closed defect (bug) (fixed)

DROP TABLE error 1051 (Unknown table)

Reported by: soulseekah Owned by: 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 12 months ago.
24800.diff (933 bytes) - added by soulseekah 12 months ago.
24800.2.diff (1.6 KB) - added by nacin 12 months ago.
24800.3.diff (697 bytes) - added by jdgrimes 5 months ago.

Download all attachments as: .zip

Change History (12)

soulseekah12 months ago

comment:1 soulseekah12 months 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.

soulseekah12 months ago

nacin12 months ago

comment:2 nacin12 months 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.

comment:3 soulseekah12 months ago

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

comment:4 follow-up: nacin5 months 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.

comment:5 in reply to: ↑ 4 jdgrimes5 months 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 5 months ago by jdgrimes (previous) (diff)

jdgrimes5 months ago

comment:6 jdgrimes5 months 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.

comment:7 nacin5 months 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.

comment:8 nacin5 months 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.