Skip to content

[5.7] Fix Postgres grammar when using union queries #27589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 20, 2019
Merged

[5.7] Fix Postgres grammar when using union queries #27589

merged 4 commits into from
Feb 20, 2019

Conversation

edersoares
Copy link
Contributor

Currently Postgres grammar creates SQL invalid when query has union or union all, using limit and/or offset clauses in its subqueries.

Postgres syntax needs that subquery is between parentheses:

Wrong

select "comment" from "comments" where "user_id" in (1) limit 10 

union 

select "comment" from "comments" where "user_id" in (2) limit 10

Correct

(select "comment" from "comments" where "user_id" in (1) limit 10) 

union

(select "comment" from "comments" where "user_id" in (2) limit 10)

This approach is based in PR #17890.

@@ -873,7 +901,7 @@ public function testUnionAggregate()
$builder->getProcessor()->shouldReceive('processSelect')->once();
$builder->from('posts')->select('id')->union($this->getMySqlBuilder()->from('videos')->select('id'))->count();

$expected = 'select count(*) as aggregate from (select * from "posts" union select * from "videos") as "temp_table"';
$expected = 'select count(*) as aggregate from ((select * from "posts") union (select * from "videos")) as "temp_table"';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR now adds them always, even if not necessary. But I guess this is fine? 🤷‍♀️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just only a select clause is not necessary, but if a LIMIT or GROUP BY clause is added, parentheses are necessary in Postgres. I guess that this behavior is not a problem. 😄

@mfn
Copy link
Contributor

mfn commented Feb 20, 2019

PS: can confirm the problem; as soon as LIMIT is added, parenthesis are necessary 👍

@taylorotwell taylorotwell merged commit c821cbf into laravel:5.7 Feb 20, 2019
@GrahamCampbell GrahamCampbell changed the title Fix Postgres grammar when using union queries [5.7] Fix Postgres grammar when using union queries Mar 1, 2019
@jnguyen32
Copy link

Any chance this can get merged into lumen as well? Having same issue and lumen 5.7 doesn't produce the parentheses.

@driesvints
Copy link
Member

@jnguyen32 I don't see how this affects lumen? Lumen just uses the DB component from Laravel. It doesn't has custom query builder functionality.

NateWr added a commit to NateWr/pkp-lib that referenced this pull request Apr 3, 2019
Laravel's QueryBuilder doesn't do unions in postgres until v5.7.
Since we run v5.5, we need to manually piece the union sql string
together.

See: laravel/framework#27589
NateWr added a commit to NateWr/pkp-lib that referenced this pull request Apr 8, 2019
Laravel's QueryBuilder doesn't do unions in postgres until v5.7.
Since we run v5.5, we need to manually piece the union sql string
together.

See: laravel/framework#27589
NateWr added a commit to NateWr/pkp-lib that referenced this pull request Apr 17, 2019
Laravel's QueryBuilder doesn't do unions in postgres until v5.7.
Since we run v5.5, we need to manually piece the union sql string
together.

See: laravel/framework#27589
NateWr added a commit to NateWr/pkp-lib that referenced this pull request Apr 29, 2019
Laravel's QueryBuilder doesn't do unions in postgres until v5.7.
Since we run v5.5, we need to manually piece the union sql string
together.

See: laravel/framework#27589
NateWr added a commit to NateWr/pkp-lib that referenced this pull request Apr 30, 2019
Laravel's QueryBuilder doesn't do unions in postgres until v5.7.
Since we run v5.5, we need to manually piece the union sql string
together.

See: laravel/framework#27589
NateWr added a commit to NateWr/pkp-lib that referenced this pull request May 1, 2019
Laravel's QueryBuilder doesn't do unions in postgres until v5.7.
Since we run v5.5, we need to manually piece the union sql string
together.

See: laravel/framework#27589
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants