The query used on the /node page (default home page) does not query alias for the fields in the condition. This mean that if that query is altered (say to add a revision table) then there are ambiguous fields.

In general it seems like best practice to use aliases.

CommentFileSizeAuthor
#8 1728568_add_alias_to_node_query_d8.patch774 bytese2thex
#6 1728568_add_alias_to_node_query_d8.patch774 bytese2thex
#3 table-alias-1728568-3.patch774 bytesoriol_e9g
#1 1728568_add_alias_to_node_query.patch761 bytese2thex
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

e2thex’s picture

e2thex CreditAttribution: e2thex commented
Status: Active » Needs review
FileSize
1728568_add_alias_to_node_query.patch761 bytes

The attached patch just adds the alias to the fields.

indytechcook’s picture

indytechcook CreditAttribution: indytechcook commented
Status: Needs review » Reviewed & tested by the community

This patch works and fixes the issue I had with a query alter.

oriol_e9g’s picture

oriol_e9g
Primary language Catalan
Location Barcelona
CreditAttribution: oriol_e9g commented
Version: 7.15 » 8.x-dev
Status: Reviewed & tested by the community » Needs review
FileSize
table-alias-1728568-3.patch774 bytes

Go to fix first in D8

oriol_e9g’s picture

oriol_e9g
Primary language Catalan
Location Barcelona
CreditAttribution: oriol_e9g commented
Title: /node query does not alais fields in conditions » Add table alias in node query conditions to avoid ambiguous fields

Status: Needs review » Needs work

The last submitted patch, table-alias-1728568-3.patch, failed testing.

e2thex’s picture

e2thex CreditAttribution: e2thex commented
Version: 8.x-dev » 7.15
Status: Needs work » Needs review
FileSize
1728568_add_alias_to_node_query_d8.patch774 bytes

rolled for d8

Status: Needs review » Needs work

The last submitted patch, 1728568_add_alias_to_node_query_d8.patch, failed testing.

e2thex’s picture

e2thex CreditAttribution: e2thex commented
Version: 7.15 » 8.x-dev
Status: Needs work » Needs review
FileSize
1728568_add_alias_to_node_query_d8.patch774 bytes

rolled for d8 and i change the version

e2thex’s picture

e2thex CreditAttribution: e2thex commented
Issue tags: +lsd-csi

tagging as part of lsd project

indytechcook’s picture

indytechcook CreditAttribution: indytechcook commented
Status: Needs review » Reviewed & tested by the community

Still looks good in d8.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

This seems like reasonable hardening that shouldn't hurt anything.

Committed and pushed to 8.x and 7.x. Thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +Needs backport to D7

Retroactively tagging.

Status: Fixed » Closed (fixed)

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

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Issue tags: +7.17 release notes

Adding this to the release notes and CHANGELOG.txt, since this is an alterable query so it could possibly affect some existing code which uses hook_query_alter() to examine/change it.