Union + Order = broken #5162

Closed
HillTravis opened this Issue Sep 23, 2013 · 8 comments

4 participants

@HillTravis

When writing a UNION query using Zend\Db\Sql\Select::combine(), the ORDER BY clause is added to the first query, not the entire query. I don't see a way to put an ORDER BY on the union-ed query. Here is a code sample:

public function fetchPage($fkADUser, array $orderBy, $page = 1, $limit = 20) {
    $inboxSelect = $this->inboxTableGateway->getSql()
        ->select()
        ->columns([
            'pkid',
        ])
        ->join(['f' => 'Fax'], 'FaxInbound.fkFax = f.pkid', [
            'PageCount',
            'CreateTime',
        ])
        ->where(['f.fkADUser = ?' => $fkADUser])
        ->where('FaxInbound.DeleteTime IS NOT NULL');

    $sentSelect = $this->sentTableGateway->getSql()
        ->select()
        ->columns([
            'pkid',
        ])
        ->join(['f' => 'Fax'], 'FaxOutbound.fkFax = f.pkid', [
            'PageCount',
            'CreateTime',
        ])
        ->where(['f.fkADUser = ?' => $fkADUser])
        ->where('FaxOutbound.DeleteTime IS NOT NULL');

    $inboxSelect->combine($sentSelect)->order($orderBy);

    return $this->inboxTableGateway->selectWith($inboxSelect);
}

And here is the resulting query (from SQL Profiler):

(
    SELECT
        [FaxInbound].[pkid] AS [pkid],
        [f].[PageCount] AS [PageCount],
        [f].[CreateTime] AS [CreateTime]
    FROM
        [FaxInbound]
        INNER JOIN [Fax] AS [f]
            ON [FaxInbound].[fkFax] = [f].[pkid]
    WHERE
        f.fkADUser = @P1
        AND FaxInbound.DeleteTime IS NOT NULL
    ORDER BY
        [CreateTime] DESC
) UNION (
    SELECT
        [FaxOutbound].[pkid] AS [pkid],
        [f].[PageCount] AS [PageCount],
        [f].[CreateTime] AS [CreateTime]
    FROM
        [FaxOutbound]
        INNER JOIN [Fax] AS [f]
            ON [FaxOutbound].[fkFax] = [f].[pkid]
    WHERE
        f.fkADUser = @P2
        AND FaxOutbound.DeleteTime IS NOT NULL
)

Notice that the ORDER BY is on the first query, and not the union-ed query. This leads to the following error message from SQL Server:

Msg 156, Level 15, State 1, Line 13
Incorrect syntax near the keyword 'ORDER'.

If the ORDER BY clause is moved outside, to the end of the query, it works correctly.

@HillTravis

It looks like changing the order of Zend\Db\Sql\Select::$specifications fixes the issue. I haven't done any major tests....I've just made sure that my other queries still work (some that do use ORDER/LIMIT/OFFSET, some that don't). I also don't have any other DB engines to test against, so I don't know if this would cause problems for those.

Original:

protected $specifications = array(
    'statementStart' => '%1$s',
    self::SELECT => array(...),
    self::JOINS  => array(...),
    self::WHERE  => 'WHERE %1$s',
    self::GROUP  => array(...),
    self::HAVING => 'HAVING %1$s',
    self::ORDER  => array(...),
    self::LIMIT  => 'LIMIT %1$s',
    self::OFFSET => 'OFFSET %1$s',
    'statementEnd' => '%1$s',
    self::COMBINE => '%1$s ( %2$s )',
);

New:

protected $specifications = array(
    'statementStart' => '%1$s',
    self::SELECT => array(...),
    self::JOINS  => array(...),
    self::WHERE  => 'WHERE %1$s',
    self::GROUP  => array(...),
    self::HAVING => 'HAVING %1$s',
    'statementEnd' => '%1$s',
    self::COMBINE => '%1$s ( %2$s )',
    self::ORDER  => array(...),
    self::LIMIT  => 'LIMIT %1$s',
    self::OFFSET => 'OFFSET %1$s',
);
@Lerry74

Hi I had the same problem how can I modify Zend\Db\Sql\Select::$specifications
thx
Lerry

@Lerry74

Found the file thx.

@samsonasik samsonasik added a commit to samsonasik/zf2 that referenced this issue Feb 22, 2014
@samsonasik samsonasik Fixes #5162 b62e1eb
@samsonasik samsonasik referenced this issue Feb 22, 2014
Closed

Fixes #5162 #5855

@samsonasik

@Travesty3 @Lerry74 I created PR #5855 for it.

@ralphschindler
Zend Framework member

I have not tested this against a real database, but my first inclination is to say that if you want to apply any operations to the union itself, then you likely need to make the union parts a subselect. Here is some pseudocode that demonstrates the idea:

$one = (new Zend\Db\Sql\Select)->from('one');
$two = (new Zend\Db\Sql\Select)->from('two');
$one->combine($two);

$oneTwo = (new Zend\Db\Sql\Select)->from(['sub' => $one])->order('id');
echo $oneTwo->getSqlString();

produces pseudo-sql something like:

SELECT "sub".* FROM (( SELECT "one".* FROM "one" ) UNION ( SELECT "two".* FROM "two" )) AS "sub" ORDER BY "id" ASC

Will this fit your requirements?

@ralphschindler
Zend Framework member

Closing as not an issue.

@ralphschindler ralphschindler added a commit that referenced this issue Mar 11, 2014
@ralphschindler ralphschindler Merge #5855 to develop
Fixed CS
Merge branch 'combineunionorder' of git://github.com/samsonasik/zf2 into hotfix/5855

* 'combineunionorder' of git://github.com/samsonasik/zf2:
  fix T_OBJECT_OPERATOR on php 5.3
  fix array syntax for php 5.3
  undo change on Select.php, and leave the test case
  Fixes #5162

Conflicts:
	tests/ZendTest/Db/Sql/SelectTest.php
8c24d96
@ralphschindler ralphschindler added a commit that referenced this issue Mar 11, 2014
@ralphschindler ralphschindler Closes #5855
Merge branch 'hotfix/5855' into develop

* hotfix/5855:
  fix T_OBJECT_OPERATOR on php 5.3
  fix array syntax for php 5.3
  undo change on Select.php, and leave the test case
  Fixes #5162
caf1cdd
@HillTravis

@ralphschindler: Sorry for taking so long to reply...I only check this stuff when I'm at work, and work has been pretty crazy. I see the issue has been closed (probably due to my lack of response), but I'm surprised that this issue would be ignored...

It looks like your suggestion would work (I also haven't tested it), but it seems to me like a hack. If I were writing this query out by hand, there's no need to wrap the whole thing in another SELECT query.

Has anyone managed to test out my proposed solution in any other SQL platforms?

@ralphschindler
Zend Framework member

I'm not sure how you would, in code, demonstrate order over the primary query with the combine, and then order over the full combined result set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment