Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This issue has been split from the parent/meta issue #2001350: [meta] Drupal cannot be installed on PostgreSQL.
Updated: Comment #N
Problem/Motivation
Toolbar menu links do not appear because of the way a orderBy() override for pgsql adds fields in front of fields expected by Entity\Query.
fvideon identified the issue in Comment #37:
Secondly there was a problem with fetching the menu entries for the admin toolbar. This issue ended up being caused by the way the orderBy override in Driver/pgsql/Select.php is adding all the orderBy attributes to the query ahead of those added by Entity\Query. When processing results, Entity\Query expects specific attributes at indices 0 and 1 which are not there. The workaround was to add a tag to the query that causes the orderBy override to skip the step of adding fields. This works as long as Entity\Query does add all the fields it is using for sorting, and as far as I've seen it seems to.
Proposed resolution
The current workaround is to add a tag to the query to disable the special features of the override in specific cases.
Crell posted in the parent issue comment #113 that adding a driver-specific tag is not acceptable in core. Whether or not there is a different fix is unknown.
Remaining tasks
- Come up with a new/better resolution or explain why there is no other resolution
- Write a patch
- Patch probably cannot be manually tested until Drupal can get through the installer due to transaction issue.
API changes
Maybe.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | interdiff.txt | 1.18 KB | mradcliffe |
| #4 | drupal-2181285-pgsql-orderby-4.patch | 660 bytes | mradcliffe |
| #2 | drupal-2181285-pgsql-orderby-hack.patch | 1.27 KB | mradcliffe |
Comments
Comment #1
mradcliffeComment #2
mradcliffeThis is the original patch from the parent issue, which did not pass code review.
Comment #3
mradcliffeAdded more information to issue summary from parent issue.
Comment #4
mradcliffeEntity Query already provides a query tag so let's just use that instead.
Comment #5
bzrudi71 CreditAttribution: bzrudi71 commentedPatch applied and toolbar is working as expected. As this patch makes use of native 'entity_query' tag and we are in the scope of PostgreSQL driver only, I can't see any side effect with the patch. In other words - let's get it in :-)
Comment #6
bzrudi71 CreditAttribution: bzrudi71 commentedComment #7
webchickHuh, was surprised to see how small this patch is!
It's not clear from the diff context, but basically what the rest of the orderBy() method is doing is some crazy-assed gymnastics in order to determine whether it's a table, or a database + table, or what have you. If we're using EFQ, we're not bothering with any of that.
I wondered if it made sense to apply this change to other drivers, but for example mysql doesn't seem to do any of these types of gymnastics: https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Database!Query!Se... its orderBy() is only 4 lines of very straight-forward code.
Therefore, I think I agree with #5, and this looks good to go from my side.
Committed and pushed to 8.x. Thanks for all of your work on these fixes!
Comment #8
mradcliffeIdeally we want to watch what's going on in EFQ to make sure that fields are being added to select that are in the order by. Postgresql requires this, but I think we can tackle that later after testbot discovers tests.