UNION queries don't support ORDER BY clauses

I'm using table sort in http://drupalcode.org/project/linkchecker.git/blob/refs/heads/7.x-1.x:/l... and this worked when developped it, but maybe i missed something... I guess this may be a major issue for me.

Drupal newb here and I'm getting the same problem. I have two select queries, both of which return 3 columns that I want to union, and then order by the first field. The system always puts ORDER BY before the second query, and I get a "General error: 1221 Incorrect usage of UNION and ORDER BY" exception.

same problem here
no solution for D7 ?

One Sollution is to wrap the select into parentheses.

The attached patch is against Drupal 7. It wraps the "master" select and the union selects into parentheses.

This didn't work for me.

#7: core-union-order-by-1145076-7.patch queued for re-testing.
What was the problem? Was it a problem applying the patch?

Wrapping the queries in parentheses fixes the fatal error, but in most cases leaving the ORDER BY as part of the individual select statements doesn't do what you want (see e.g. http://dev.mysql.com/doc/refman/5.1/en/union.html - it doesn't affect the actual order of the final result set, but rather only is useful if you need a LIMIT on the individual selects also).

I think for this issue we should support the most common case which is to order the final query (and according to some comments I've seen that's the only one officially supported by the SQL standard, although I haven't bothered to verify).

So, the attached patch goes back to @drewish's original suggestion for how to fix this, and combines it with the tests from #2. I modified the tests a bit also, because it didn't seem like they were asserting the correct expected order?

This will need to be moved to Drupal 8, but here are patches for Drupal 7 for starters to let the testbot run against them first.

Patches for Drupal 8.

Can we include the the wrapping parentheses (mentioned in #11, for using LIMIT with two select queries) in the patch?

I'm seeing a similar issue with LIMIT as reported originally by @drewish concerning ORDER BY statements, #11 fixes the issue for me in D7.

Yes, LIMIT is failing for me, too.

BTW, the patch doesn't work for me. DBTNG still putting the order by in the wrong place (with 7.15):

PDOException: SQLSTATE[HY000]: General error: 1221 Incorrect usage of UNION and ORDER BY: SELECT cb.title AS title, cb.changed_date AS changed_date FROM {a_blog} cb ORDER BY changed_date DESC UNION ALL SELECT cg.title AS title, cg.changed_date AS changed_date FROM {a_gallery} cg

#7 worked for me; the patch applied the parenthesis around the queries as expected.
#11 didn't apply the parenthesis correctly and just applies an update to a test file.

But then I came across an issue when I tried to UNION another query that was already UNION'ed:
SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ') UNION ( SELECT ' at line 9

Sorry, I don't have the same insight as @David_Rothstein but I created a new patch variant of @cbergmann's solution from #7. Still having the same issue with LIMIT--it only applies the LIMIT to the first query. This one's over my head personally, so hopefully this patch leads to the correct solution.

Sorry, the patch from #17 was bad.
Rerolled the patch here.

FYI: Workaround is to put the UNIONed queries into a subquery like this:

SELECT column1, column2 FROM (
  SELECT column1, column2 FROM table1
  UNION
  SELECT column3, column4 FROM table2
) subquery
ORDER BY column1, column2;

In db_select() speech:

$table1 = db_select('table1', 't1')
  ->fields('t1', array('column1', 'column2'));

$table2 = db_select('table2', 't2')
  ->fields('t2', array('column3', 'column4'));

$query = Database::getConnection()
  ->select($table1->union($table2))
  ->fields(NULL, array('column1', 'column2'))
  ->orderBy('column1')
  ->orderBy('column2');

Ah... This is why have not seen it yet... :-))) good to know...

We've used the patches for D7 that have been submitted on a particular client project and it has resolved the issues that we ran into with unionizing two queries from different views. Thanks for submitting these patches, it'd be great to see these patches get put into core when ready.

This would need to be rerolled against Drupal\Core\Database\Query\Select. It would also be great to have a simple union query test so this doesn't regress.

The latest Drupal 8 patch (#12) already has a test, and the patch itself still applies fine to 8.0.x - however the test needs a reroll.

I guess we need some reviewers to compare the two approaches and help decide between them.

Oops.

reroll from #12. Please review.

Is this ready but not committed ?

I think this is RTBC. The test has been uploaded separately to illustrate the current failure. I've updated the issue summary with a beta phase evaluation.

Still green. Moving to RTBC.

I tested sqlite and postgres - both pass. I wonder what happens if both queries have an order by?

@alexpott it sounds like you're suggesting more test coverage, is that right?

I was hoping to create a similar query to the one below:

(select node.title as node_title, node.created as node_created
from table_of_nodes_1
order by node_created desc
limit 1 offset 0)
union
(select node.title as node_title, node.created as node_created 
from table_of_nodes_2
order by node_created desc
limit 1 offset 0)
order by node_created desc

I agree with comment #13. Drupal wrapping the select statements in parentheses as well as allowing optional order by clauses within each select statement and outside of the whole statement would be helpful. But I also understand that drupal can't support all possible sql statements. One solution is very well-documented here. I just created a sql view and made it available to the Views Module.

I'm also on Drupal 7 so I am unable to help test patch from comment #27.

The patch in #11 for Drupal 7 does not work in our testing of a UNIONed query with an order by. The patch in #7 does. The union being performed is on a View query, using hook_view_pre_execute().

Perhaps the second parameter to SelectQuery::union() could be either a string, as it is currently, or an array with associative keys for 'type', and new optional ones for 'order' and 'range'? With the '(' and ')' surrounding the query string, this allows the call to fully define the union SQL independently of the rest of the query. Please note that

1. only the 'order' and 'range' information for the last union call are used.
2. for 'order', the associative key is the field name and value is the direction. This is the same after using SelectQuery::orderBy() to the query.
3. for 'range', it has two associative parameters 'length' and optionally 'start'. These are same as when using SelectQuery::range().

I do appreciate that

1. Adding the '(' and ')' will possibly break some sites where the queries are specially written to work without them.
2. The suggested fix is not completely OOP and UNION handling is still a little odd.

I have attached a patch file for Drupal 7.42. This patch also is used in hook_views_pre_execute() in the module views_extra_handlers to combine the results from two or more views. Example of use in a patched views_extra_handlers_views_pre_execute().

/**
 * Implements hook_views_pre_execute().
 */
function views_extra_handlers_views_pre_execute(view &$view) {
  if(isset($view->field) && !empty($view->field)){
    foreach($view->field as $field_name => $field_obj){
      $handler = $field_obj->definition['handler'];
      if($handler == "views_extra_handlers_handler_field_query_alter"){
        if(!empty($field_obj->options['veh_union']['veh_union_view_name'])){
          $query1 = &$view->build_info['query'];

          $union_view_id = $field_obj->options['veh_union']['veh_union_view_name'];
          $union_view_display_id = $field_obj->options['veh_union']['veh_union_view_display'];
          $view2 = views_get_view($union_view_id);
          $view2->set_display($union_view_display_id);
          $view2->pre_execute($view->args);
          $view2->execute();
          $query2 = $view2->build_info['query'];

          // Verify that queries are very similar.
          //dpq($query1);
          //dpq($query2);
          $union_orderby = $field_obj->options['veh_union']['veh_union_orderby'];
          if (isset($union_orderby)) {
            $union_orderby_direction = $field_obj->options['veh_union']['veh_union_orderby_direction'];
            $type = array(
              'type' => 'UNION',
              'order' => array(
                $union_orderby => (isset($union_orderby_direction) ? $union_orderby_direction : 'ASC'),
              ),
            );
            $query1 = $query1->union($query2, $type);
          }
          else {
            // Matrimony.
            $query1 = $query1->union($query2, 'UNION');
          }
        }
      }
    }
  }
}

alexpott said in comment #34:

I wonder what happens if both queries have an order by?

Lets look at the three supported databases. The manual from SQLite is pretty clear:

As the components of a compound SELECT must be simple SELECT statements, they may not contain ORDER BY or LIMIT clauses. ORDER BY and LIMIT clauses may only occur at the end of the entire compound SELECT, and then only if the final element of the compound is not a VALUES clause.

SQLite allows only to add ORDER BY at the end and is applied afer all UNION statements.

For PostgreSQL allows to add ORDER BY to an individual select statement if it enclosed in parentheses.

The manual of MySQL is stating:

However, use of ORDER BY for individual SELECT statements implies nothing about the order in which the rows appear in the final result because UNION by default produces an unordered set of rows.

So for MySQL there is no use case because it is ignoring ORDER BY for individual select statements.

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 had to do a reroll because the SelectTest has been moved. I did not make any code changes.

The test failures for the patch with the tests and the fix are all not related to the patch. So I will ignore them.

I made no code changes, except for the fact that the SelectTest file location has been changed.
In comment #41 have I explained why the the problem raised by alexpott is not relevant.
This issue is a bug and as such can be applaid to 8.1.x.
The complete patch looks good to me and for me it is RTBC.
It was already RTBC for user jhedstrom in comment #32.

Removed the Beta phase evaluation from the IS.

Back to RTBC.

Given we've moved UNION above limit can we add test coverage for limit with a union.

Added the requested test for a union with a order and a limit. The test is the same as the other added test with a limit added to it.

The new test looks good. One tiny nit, and then I think this is RTBC.

+++ b/core/tests/Drupal/KernelTests/Core/Database/SelectTest.php
@@ -352,6 +352,35 @@ function testUnionOrder() {
+   * Tests that we can UNION multiple Select queries together and set the ORDER with and a LIMIT.

Line exceeds 80 characters, so needs to be shorter.

Ooops

Sorry @daffie I should have been more clear--it's actually this standard I was referring to:

All summaries (first lines of docblocks) must be under 80 characters...

Meaning this first line should be a single sentence that is under 80 characters. Further detail can be provided after that if needed.

I couldn't find a phpcs standard for that though over in #2571965: [meta] Fix PHP coding standards in core, stage 1, so I don't know if it's enforced or not, but would be good to fix here regardless.

+++ b/core/tests/Drupal/KernelTests/Core/Database/SelectTest.php
@@ -352,7 +352,8 @@ function testUnionOrder() {
+   * Tests that we can UNION multiple Select queries together and set the ORDER
+   * with and a LIMIT.

Something like 'Tests UNION of multiple SELECT queries together with ORDER and LIMIT'. I'm going to mark this RTBC as that could be fixed on commit (or if @daffie gets back to this first).

@jhedstrom: Now I get it. Sorry, for being a bit slow. :) And thank you for your help!

Back to RTBC. Thanks @daffie!

Committed and pushed 6d3fd71023d38616f4fdf7ae094a46b3ee0f7635 to 8.2.x and 62598ec to 8.1.x. Thanks!

Can someone open a new issue for the D7 backport and mark this issue as fixed?

@alexpott: Why do you want to have this issue closed and opening a new one for D7. It is must more easy just to set this one back to D7. They get all the info about the problem direct from this issue.

@daffie - this is the new policy. It exists for lots and lots of reasons - see https://www.drupal.org/core/backport-policy and #1145076: UNION queries don't support ORDER BY clauses

Created #2772107: UNION queries don't support ORDER BY clauses for D7.

@alexpott: Thank you for the explanation.

When has this policy changed? In past we always moved cases to D7 after it was fixed in D8. This way we are loosing track of the issue that moved from D7 to D8 to get fixed there first.

Try it, work for me
https://www.drupal.org/node/2205391#comment-8522061

try this for union in ORDER BY

$query = db_select('node', 'n')
->fields('n', array('nid', 'title'))
->condition('nid', 92);
$query2 = db_select('comment', 'c')
->fields('c', array('cid', 'subject'))
->condition('cid', 5);
$query2->orderBy('title', 'DESC'); // this
$query->union($query2);

$query2 = db_select('comment', 'c')
->fields('c', array('cid', 'subject'))
->condition('cid', 5);
$query2->orderBy('title', 'DESC');

This will not work because the comment table does not have a column named "title".
Second the union operator return an unordered list. Whatever you put into an union operator you get an unordered list. You can however order the unordered list after the union operator. So moving $query2->orderBy('title', 'DESC'); to the bottom and changing it to $query instead of $query2 will work much better.