Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Union + Order = broken #5162

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

Union + Order = broken #5162

HillTravis opened this issue Sep 23, 2013 · 8 comments
Labels

Comments

@HillTravis
Copy link

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
Copy link
Author

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
Copy link

Lerry74 commented Nov 8, 2013

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

@Lerry74
Copy link

Lerry74 commented Nov 9, 2013

Found the file thx.

samsonasik added a commit to samsonasik/zf2 that referenced this issue Feb 22, 2014
@samsonasik
Copy link
Contributor

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

@ralphschindler
Copy link
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
Copy link
Member

Closing as not an issue.

ralphschindler pushed a commit that referenced this issue Mar 11, 2014
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
ralphschindler pushed a commit that referenced this issue Mar 11, 2014
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
@HillTravis
Copy link
Author

@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
Copy link
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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants