Call to a member function getColumns() on boolean

Please check https://www.drupal.org/project/drupal/issues/3352880 🐛 Logic Error in Tables.php Active . I think it's related but different.

This is caused by 🐛 entity.last_installed_schema.repository has a race condition Needs review and I recommend closing this as a duplicate of that.

Please review.

Removing credit for #50 as there was no interdiff but from the pervious file size something was lost.
There was no explanation for what was changed or why. If just a reroll that doesn't receive credit anymore either.
Also CC failure, expected to check patch before uploading.
#49 mentions things are getting mixed so maybe the issue summary should be updated too.

I experienced this issue when I (config sync) imported a Feed type from one site to another and then tried to add a Feed of that type in my new site. By going to the Feed type, editing and saving it, I was then able to add the new Feed without the error. Definitely need the patch, but this is a helpful workaround if you are stuck and don't want to apply a patch.

Patch for 11.x.

Hey, wanted to add a tweak on patch from comment #53. The elseif on line 191 of /core/lib/Drupal/Core/Entity/Query/Sql/Tables.php didn't have a condition and was throwing warnings occasionally. Here is that patch with a condition for the elseif. This patch applies and works for us in Drupal 10.2.6 and PHP 8.1.27.

I checked the comments in this issue and all suggestions have been addressed. The scope creep mentioned in #2893747-49: Call to a member function getColumns() on boolean → has been addressed. Missing tests reported in #2893747-51: Call to a member function getColumns() on boolean → have been restored.

Creating merge request. Will do another code review.

Patch needs to be rerolled now that 🐛 Improve comments in \Drupal\Core\Entity\Query\Sql\Tables Fixed is merged.

Addressed my remarks and refactored the code so both PHPUnit and PHPStan are satisfied.

Created a "test-only" MR that shows that the test correctly detects the error (ref https://git.drupalcode.org/issue/drupal-2893747/-/jobs/2772907):

Drupal\KernelTests\Core\Entity\EntityQueryRelationshipTest::testInvalidFieldName with data set "0" ('non_existent_field_name.entit....value')
    Failed asserting that exception of type "Error" matches expected exception "Drupal\Core\Entity\Query\QueryException".
    Message was: "Call to a member function getColumns() on false" at
    /builds/issue/drupal-2893747/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php:251

Also backported the fix to Drupal 10.4.

LGTM :)

@hazn, thanks for the confidence in the MR. When setting an issue to RTBC it is good practice to comment on what you did to review, even adding bits of the change you particularly or sections you had questions about.

I'm triaging RTBC issues → . The Issue Summary should have a proposed resolution so the implementation can be confirmed. And there are 3 MRs and no indication of which one is to be reviewed. I'm also tagging for a title update as the title is vague. Maybe it should, at least, include the subsystem?

Sorry, but back to Needs work.

pfrenssen → changed the visibility of the branch 2893747-10.4.x to hidden.

pfrenssen → changed the visibility of the branch 2893747-test-only to hidden.

Thanks @quietone for the suggestions. I updated the issue summary and have hidden the irrelevant branches. Back to Needs Review.

Looks ready to go, thanks for all the updates @pfrenssen!

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Bot found a PHPStan regression, thank you bot! Fixed, back to NR.

Rebased and fixed the return type. Also, switched to using PHP 8 null-safe operator in Tables.php to reduce diff.

Feedback appears to be addressed I believe.