Skip to content

Commit

Permalink
Fixed incorrect generation of union queries (#46)
Browse files Browse the repository at this point in the history
* Fixed incorrect generation of union queries
* Changed to add SELECT * FROM before each union part.
* Changed to wrap the query before adding a final order by/limit/offset for union queries.
  * This is needed because otherwise these clauses are applied only to the last union subquery.
* Changed to don't run tests in the non-integration mode
* Added more tests
* Fixed case when offset is set without a limit clause
  • Loading branch information
AdalbertMemSQL authored Dec 1, 2022
1 parent 9e31536 commit 943a303
Show file tree
Hide file tree
Showing 2 changed files with 245 additions and 0 deletions.
100 changes: 100 additions & 0 deletions src/Query/Grammar.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,104 @@ protected function wrapJsonFieldAndPath($column)

return [$field, $path];
}

/**
* Wrap a union subquery in parentheses.
*
* @param string $sql
* @return string
*/
protected function wrapUnion($sql): string
{
return 'SELECT * FROM ('.$sql.')';
}

/**
* Compile the "union" queries attached to the main query.
*
* @param Builder $query
* @return string
*/
protected function compileUnions(Builder $query): string
{
$sql = '';

foreach ($query->unions as $union) {
$sql .= $this->compileUnion($union);
}

return ltrim($sql);
}

/**
* Compile a select query into SQL.
*
* @param Builder $query
* @return string
*/
public function compileSelect(Builder $query): string
{
$sql = parent::compileSelect($query);

if (! empty($query->unionOrders) || isset($query->unionLimit) || isset($query->unionOffset)) {
$sql = "SELECT * FROM (".$sql.") ";

if (! empty($query->unionOrders)) {
$sql .= ' '.$this->compileOrders($query, $query->unionOrders);
}

if (isset($query->unionLimit)) {
$sql .= ' '.$this->compileLimit($query, $query->unionLimit);
}

if (isset($query->unionOffset)) {
$sql .= ' '.$this->compileUnionOffset($query, $query->unionOffset);
}
}

return ltrim($sql);
}

/**
* Compile the "offset" portions of the query.
*
* @param Builder $query
* @param $offset
* @return string
*/
protected function compileOffset(Builder $query, $offset): string
{
return $this->compileOffsetWithLimit($offset, $query->limit);
}

/**
* Compile the "offset" portions of the final union query.
*
* @param Builder $query
* @param $offset
* @return string
*/
protected function compileUnionOffset(Builder $query, $offset): string
{
return $this->compileOffsetWithLimit($offset, $query->unionLimit);
}

/**
* Compile the "offset" portions of the query taking into account "limit" portion.
*
* @param $offset
* @param $limit
* @return string
*/
private function compileOffsetWithLimit($offset, $limit): string
{
// OFFSET is not valid without LIMIT
// Add a huge LIMIT clause
if (! isset($limit)) {
// 9223372036854775807 - max 64-bit integer
return ' LIMIT 9223372036854775807 OFFSET '.(int) $offset;
}

return ' OFFSET '.(int) $offset;
}
}
145 changes: 145 additions & 0 deletions tests/Hybrid/UnionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
<?php

namespace SingleStore\Laravel\Tests\Hybrid;

use Illuminate\Support\Facades\DB;
use SingleStore\Laravel\Schema\Blueprint;
use SingleStore\Laravel\Tests\BaseTest;

class UnionTest extends BaseTest
{
use HybridTestHelpers;

protected function setUp(): void
{
parent::setUp();

if ($this->runHybridIntegrations()) {
$this->createTable(function (Blueprint $table) {
$table->id();
});

DB::table('test')->insert([
['id' => 1],
['id' => 2],
['id' => 3],
['id' => 4],
['id' => 100],
]);
}
}

/** @test */
function union() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 3);
$second = DB::table('test')->where('id', '>', 5);
$res = $first->union($second)->get();

$indexes = array_map(function ($value): int {
return $value->id;
}, $res->toArray());
sort($indexes);

$this->assertEquals([1, 2, 100], $indexes);
}

/** @test */
function unionAll() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 4);
$second = DB::table('test')->where('id', '>', 2);
$res = $first->unionAll($second)->get();

$indexes = array_map(function ($value): int {
return $value->id;
}, $res->toArray());
sort($indexes);

$this->assertEquals([1, 2, 3, 3, 4, 100], $indexes);
}

/** @test */
function unionWithOrderByLimitAndOffset() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 3);
$second = DB::table('test')->where('id', '>', 5);
$res = $first->union($second)->orderBy('id')->limit(1)->offset(1)->get();

$indexes = array_map(function ($value): int {
return $value->id;
}, $res->toArray());

$this->assertEquals([2], $indexes);
}

/** @test */
function unionWithOrderBy() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 3);
$second = DB::table('test')->where('id', '>', 5);
$res = $first->union($second)->orderBy('id')->get();

$indexes = array_map(function ($value): int {
return $value->id;
}, $res->toArray());

$this->assertEquals([1, 2, 100], $indexes);
}

/** @test */
function unionWithLimit() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 3);
$second = DB::table('test')->where('id', '>', 5);
$res = $first->union($second)->limit(2)->get();

$this->assertCount(2, $res);
}

/** @test */
function unionWithOffset() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 3);
$second = DB::table('test')->where('id', '>', 5);
$res = $first->union($second)->offset(1)->get();

$this->assertCount(2, $res);
}

/** @test */
function unionWithInnerOffset() {
if (! $this->runHybridIntegrations()) {
return;
}

$first = DB::table('test')->where('id', '<', 3)->offset(1)->orderBy('id');
$second = DB::table('test')->where('id', '>', 5);
$res = $first->union($second)->get();

$indexes = array_map(function ($value): int {
return $value->id;
}, $res->toArray());
sort($indexes);

$this->assertEquals([2, 100], $indexes);
}
}

0 comments on commit 943a303

Please sign in to comment.