Problem/Motivation

A lot of PHP 8.2 tests currently fail like this:

Exception: Deprecated function: Creation of dynamic property Drupal\Core\Database\Query\Condition::$sqlQuery is deprecated
Drupal\Core\Entity\Query\Sql\Condition->compile()() (Line: 44)

Steps to reproduce

What's happening is that this is a trick to pass the query object to the compile method of its child condition (entity query condition object, not SQL condition).

Proposed resolution

There is already a better pattern used in the same place. Set a documented, protected property of itself on the child object, which works because it's the same class.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 interdiff_2-10.txt1.01 KBravi.shankar
#10 3311562-10.patch3.38 KBravi.shankar
#2 entity-query-condition-sqlQuery-3311562-2.patch3.41 KBberdir

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new entity-query-condition-sqlQuery-3311562-2.patch3.41 KB
Anonymous’s picture

Anonymous (not verified) commented

I have checked with drupal 10.1.x and the patch applies successfully.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

It may need a test but PHP 8.2 already used to catch it via existing coverage

See #3295821-114: Ignore: patch testing issue for PHP 8.2 attributes

  • catch committed b94cecd on 10.0.x 
    Issue #3311562 by Berdir, andypost: Set sqlQuery in Entity\Query\Sql...
  • catch committed e9afcbc on 10.1.x
    Issue #3311562 by Berdir, andypost: Set sqlQuery in Entity\Query\Sql...
  • catch committed 075c691 on 9.5.x
    Issue #3311562 by Berdir, andypost: Set sqlQuery in Entity\Query\Sql...
catch’s picture

catch
he/him
Primary language English
commented
Version: 10.0.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Given existing test coverage is catching the PHP 8.2 issue and tests are passing, I don't think we need to add explicit new test coverage here.

Committed/pushed to 10.1.x and cherry-picked back through to 9.5.x, thanks!

mfb’s picture

mfb
they or he
commented

FYI, this cherry-pick broke 9.5.x branch on PHP 7.3 - the property type declaration requires PHP 7.4

  • catch committed b083e40 on 9.5.x 
    Revert "Issue #3311562 by Berdir, andypost: Set sqlQuery in Entity\Query...
catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

Reverted on 9.5.x, we could probably recommit just with the type hint removed.

ravi.shankar’s picture

ravi.shankar commented
Status: Needs work » Needs review
StatusFileSize
new 3311562-10.patch3.38 KB
new interdiff_2-10.txt1.01 KB

Removed type hint from $sqlQuery property.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Queued for 7.3

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

passed!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 3098069 and pushed to 9.5.x. Thanks!

  • alexpott committed 3098069 on 9.5.x 
    Issue #3311562 by ravi.shankar, Berdir, andypost, catch: Set sqlQuery in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.