API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

> array|bool The table field mapping for the given table or FALSE if not available.

That leads you to believe that the return is the same as what you get from $storage->getTableMapping()->getAllColumns($table). But it's not -- the array is flipped, so keys are field names and values are just incrementing integers.

CommentFileSizeAuthor
#7 interdiff_4-7.txt680 bytesleolandotan
#7 fix-return-doc-2975751-7.patch899 bytesleolandotan
#4 fix-return-doc-2975751-4.patch845 bytesmsankhala
#2 fix-return-doc-2975751-2.patch813 bytesmsankhala
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Active » Needs review
Issue tags: +Novice
FileSize
fix-return-doc-2975751-2.patch813 bytes

Here is a patch.

joachim’s picture

joachim CreditAttribution: joachim as a volunteer commented
Status: Needs review » Needs work 
+++ b/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php
@@ -410,7 +410,9 @@ protected function addJoin($type, $table, $join_condition, $langcode, $delta = N
+   *   columns name and values are just incrementing integers or FALSE if not
+   *   available.

The 'or FALSE' sounds like it's an alternative to the incrementing integer, rather than the whole thing.

I would make a new sentence here:

> incrementing integers. If the (table? mapping? it's not clear WHAT is not available) is not available, FALSE is returned.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
fix-return-doc-2975751-4.patch845 bytes

@joachim Here is updated patch. Tables::getTableMapping() simply checks if $storage instanceof SqlEntityStorageInterface and gets the table mapping so it is never clear whether table is not available or mapping is not available.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Needs review » Needs work 
+++ b/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php
@@ -410,7 +410,9 @@ protected function addJoin($type, $table, $join_condition, $langcode, $delta = N
    * @return array|bool

So, it's array OR false. Then let's be more explicit:

@return array|false

PS: Please post an interdiff each time, regardless of how small is the change.

leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: Unassigned » leolandotan

I'll work on this.

leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: leolandotan » Unassigned
Status: Needs work » Needs review
FileSize
fix-return-doc-2975751-7.patch899 bytes
interdiff_4-7.txt680 bytes

I applied the required fix from comment #5.

Hope everything is in order.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Needs review » Reviewed & tested by the community

Thank you!

alexpott’s picture

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

Committed and pushed 845da7492b to 8.6.x and aaf49c1413 to 8.5.x. Thanks!

  • alexpott committed 845da74 on 8.6.x 
    Issue #2975751 by msankhala, leolando.tan, joachim, claudiu.cristea:...

  • alexpott committed aaf49c1 on 8.5.x 
    Issue #2975751 by msankhala, leolando.tan, joachim, claudiu.cristea:...

Status: Fixed » Closed (fixed)

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