Skip to content

#64130 Fix TypeError in wpdb::load_col_info() when a query fails - #13099

Open
AKSHAT2802 wants to merge 10 commits into
WordPress:trunkfrom
AKSHAT2802:fix/64130-wpdb-load-col-info-type-check
Open

AKSHAT2802 wants to merge 10 commits into
WordPress:trunkfrom
AKSHAT2802:fix/64130-wpdb-load-col-info-type-check

Conversation

@AKSHAT2802

Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/64130

PR Description

This PR adds a guard to load_col_info() so it returns early when $this->result isn't a mysqli_result, consistent with the existing instanceof mysqli_result check already used in wpdb::flush().

A regression test (test_get_col_info_after_failed_query) is included, which runs an invalid query and then accesses col_info — this reproduces the reported TypeError on unpatched code and passes with the fix applied.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5
Used for: Investigating and confirming the bug against the current trunk, and writing/validating the regression test.

All changes were made and verified by reproducing the failure and confirming the fix in the local test environment.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

…add test for get_col_info after failed query
@github-actions

github-actions Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props akshat2802, mukesh27.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@mukeshpanchal27 mukeshpanchal27 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The fix looks right to me. I reproduced the TypeError on trunk (PHP 8.3) and confirmed this change fixes it.

Fix

  • ! ( $this->result instanceof mysqli_result ) covers every non-result state: false (failed query), true (INSERT/UPDATE/DELETE/SET/etc.) and null (no query yet, or the early return on invalid data in query()).
  • It matches the existing checks in flush() and query(), and it's stricter than the is_object() check suggested on the ticket.
  • get_col_info() then returns null, which is what callers already get when there are no columns, so valid usage is unaffected.
  • As a side note, PHPStan level 7 reports both mysqli_num_fields() and mysqli_fetch_field() here (bool|mysqli_result|null given). Those go away with this change. The project's current level 5 doesn't catch it.

Test suggestions

  1. The test only covers the failed-query (false) case. The more common trigger is probably reading col_info after a write query, where $this->result is true. Could you use a data provider (a static one, like the rest of this file) that also covers UPDATE and SET?
  2. Please add @ticket 64130.
  3. Please save and restore the previous suppression state ($suppress = $wpdb->suppress_errors( true ); … $wpdb->suppress_errors( $suppress );) instead of hardcoding false, as other tests in this file do.
  4. Nit: wptests_ is hardcoded in the table name. Any table name that doesn't exist will work.
  5. Nit: assertFalse( $wpdb->result ) checks a protected property through __get(). The col_info/get_col_info() assertions already cover the behavior, so it could be dropped.

Suggested version:

/**
 * @ticket 64130
 *
 * @dataProvider data_get_col_info_without_result_set
 *
 * @param string $query Query that does not produce a result set.
 */
public function test_get_col_info_without_result_set( $query ) {
	global $wpdb;

	$suppress = $wpdb->suppress_errors( true );
	$wpdb->query( sprintf( $query, $wpdb->options ) );
	$wpdb->suppress_errors( $suppress );

	$this->assertNull( $wpdb->get_col_info(), 'get_col_info() should return null.' );
	$this->assertNull( $wpdb->col_info, 'The col_info property should be null.' );
}

/**
 * Data provider.
 *
 * @return array<string, string[]>
 */
public static function data_get_col_info_without_result_set() {
	return array(
		'failed query' => array( 'SELECT ID FROM table_that_does_not_exist' ),
		'update query' => array( "UPDATE %s SET option_value = option_value WHERE option_name = 'blogname'" ),
		'set query'    => array( 'SET @wp_test_64130 = 1' ),
	);
}

I ran this locally. With the fix, all 3 cases pass. Without it, all 3 fail with TypeError: mysqli_num_fields(): Argument #1 ($result) must be of type mysqli_result, {false|true} given.

Comment thread tests/phpunit/tests/db.php
AKSHAT2802 and others added 2 commits October 1, 2026 15:07
refactor order

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants