Problem/Motivation

\Drupal\Core\Entity\Query\Sql\Tables::addNextBaseTable() doesn't use square brackets syntax.
As per #3189880: Use square brackets syntax in sql queries \Drupal\Core\Entity\Query\Sql\Tables::addNextBaseTable() should use

Steps to reproduce

Just run EFQ with ER field condition.
condition('uid.entity.name', 'foo', 'CONTAINS')

Proposed resolution

Add square brackets.

Remaining tasks

  • Review.
  • Commit
  • Patch.
  • Rejoice

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

Not required.

CommentFileSizeAuthor
#3 3208222-applied_patch.png35.21 KBabhijith s
#2 3208222-2.patch822 bytesjibran

Comments

jibran created an issue. See original summary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Active » Needs review
StatusFileSize
new 3208222-2.patch822 bytes
abhijith s’s picture

abhijith s commented
StatusFileSize
new 3208222-applied_patch.png35.21 KB

Applied patch #2 and the square brackets are added in it.

after

RTBC +1

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Makes sense. I wonder if this could be tested? Is it possible to extract the underlying SQL statement from an entity query?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Is it possible to extract the underlying SQL statement from an entity query?

Yes, it is possible.

    $query = $this->container->get('entity_type.manager')
      ->getStorage('some_entity_type')
      ->getQuery();
    $reflection = new \ReflectionObject($query);
    $property = $reflection->getProperty('sqlQuery');
    $property->setAccessible(TRUE);
    $sql = (string) $property->getValue($query);

  • catch committed dcad943 on 9.2.x 
    Issue #3208222 by jibran: Tables::addNextBaseTable() doesn't use square...

  • catch committed 07cd30b on 9.1.x 
    Issue #3208222 by jibran: Tables::addNextBaseTable() doesn't use square...
catch’s picture

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

Committed dcad943 and pushed to 9.2.x. Thanks!

No need for test coverage of whether the query has square brackets or not, if anything it would be a phpcs rule but hard to make one that covers every possibility.

@Abhijith S there's no need to provide a screenshot of the patch applying, since DrupalCI confirms that for us.

Status: Fixed » Closed (fixed)

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