I was trying to write some migration code using UNION and an ORDER BY and found it putting the ORDER BY before the second query. Queries added via SelectQueryInterface::union() should either have their order stripped or be turned into subqueries. I think the fix is to move the UNION output in __toString() so it is called before the parts processing ORDER BY and RANGEs.

See also https://www.drupal.org/node/1145076 for history.

CommentFileSizeAuthor
#11 2772107-11.patch3.74 KBpoker10
#11 2772107-11_test-only.patch2.48 KBpoker10
#8 patch_select_union-2772107-8.diff1.36 KBcgv
#7 2772107-7.patch3.75 KBeojthebrave
#5 2772107-5.patch3.75 KBaerozeppelin
#3 core-union-order-by-2772107-3.patch1.31 KBq0rban

Comments

daffie created an issue. See original summary.

michel.settembrino’s picture

michel.settembrino commented
Issue summary: View changes

Reference to old issue added to do not loose history.

q0rban’s picture

q0rban commented
Status: Active » Needs work
Issue tags: +Needs tests
StatusFileSize
new core-union-order-by-2772107-3.patch1.31 KB

The patch in #11 on #1145076: UNION queries don't support ORDER BY clauses does not work in our testing of a UNIONed query with an order by. The patch in #7 by cbergmann does. The union being performed is on a View query, using hook_view_pre_execute(). Working patch from #7 is attached here for reference, but does not include tests. Make sure cbergmann gets credit for this if it is expanded on.

stefan.r’s picture

stefan.r commented

Hmm, why does #11 not work? It works in D8...

The tests you can probably mostly copypaste from #55.

aerozeppelin’s picture

aerozeppelin commented
Status: Needs work » Needs review
StatusFileSize
new 2772107-5.patch3.75 KB

Uploading patch from #11 and #55.

Status: Needs review » Needs work

The last submitted patch, 5: 2772107-5.patch, failed testing.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Status: Needs work » Needs review
StatusFileSize
new 2772107-7.patch3.75 KB

I just ran into this issue. And can confirm that the patch in #5 fixes the issue. And is also the same fix that was applied to Drupal 8. This version just fixes a couple of typos in the comments included with #5.

cgv’s picture

cgv commented
StatusFileSize
new patch_select_union-2772107-8.diff1.36 KB

Hi, I think patches #5 and #7 not support order by or limit per union query, so we can't set limits to individuals query and then (optionally) apply a global limit. Patch #3 tries to solve it but have a little error that I solve in this patch.

c7bamford’s picture

c7bamford commented

#7 works for me. #8 fails on queries using multiple unions.

sgdev’s picture

sgdev commented

Patch #7 works for me too.

poker10’s picture

poker10 commented
Issue tags: -Needs tests
StatusFileSize
new 2772107-11_test-only.patch2.48 KB
new 2772107-11.patch3.74 KB

The patch from #7 is working correctly, we are also using it on some projects. Also it is a complete backport of the D8 issue with tests included.

Reuploading the unchanged patch from #7 with a test only version to verify the failure. If there would not be any problems with the testbot results, this can be set as RTBC.

Regarding to comment #8: Thanks for the report, but I think we will need to create followup for this if needed. The code in D9 is the same as in the patch #7, so we cannot make any additional changes here without checking the problem in D9 first.

The last submitted patch, 11: 2772107-11_test-only.patch, failed testing. View results

poker10’s picture

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

Changing the status to RTBC as mentioned in #11.

mcdruid’s picture

mcdruid commented
Issue tags: +RTBM

D9 code for comparison:

https://git.drupalcode.org/project/drupal/-/blob/9.4.5/core/lib/Drupal/C...

...and the tests:

https://git.drupalcode.org/project/drupal/-/blob/9.4.5/core/tests/Drupal...

This looks good to me, thanks!

poker10 credited cbergmann.

poker10 credited David_Rothstein.

poker10 credited drewish.

poker10 credited johnnydarkko.

poker10’s picture

poker10 commented

  • poker10 committed cfb8a71 on 7.x 
    Issue #2772107 by poker10, cgv, eojthebrave, q0rban, daffie,...

poker10 credited aangel.

poker10 credited amcgowanca.

poker10 credited bigjim.

poker10 credited fuerst.

poker10 credited Libero82.

poker10 credited nagiek.

poker10 credited somersoft.

poker10’s picture

poker10 commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -RTBM

Thanks all who contributed!

Just an additional note to #8:

Hi, I think patches #5 and #7 not support order by or limit per union query, so we can't set limits to individuals query and then (optionally) apply a global limit. Patch #3 tries to solve it but have a little error that I solve in this patch.

As @daffie mentioned here: #1145076-41: UNION queries don't support ORDER BY clauses , we cannot easily add that support:

To conclude: lets not add ORDER BY clauses to individual select statements because our three supported databases do not like it, MySQL is ignoring it and SQLite has outlawed it completely.

I have also added credits from the parent issue.

Status: Fixed » Closed (fixed)

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