Skip to content

feat: Improve GROUP_CONCAT Behavior – Consider GROUP BY and Support DISTINCT, ORDER BY, and SEPARATOR Options #57

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions src/Parser/ExpressionParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,12 @@ public function __construct(
/**
* @param array<int, Token> $tokens
*
* @return array{0: bool, 1: array<int, Expression>}
* @return array{
* 0: bool,
* 1: list<Expression>,
* 2: ?array<int, array{direction: 'ASC'|'DESC', expression: Expression}>,
* 3: ?Expression
* }
*/
private function getListExpression(array $tokens)
{
Expand All @@ -137,10 +142,17 @@ private function getListExpression(array $tokens)
$needs_comma = false;
$args = [];

$order_by = null;
$separator = null;

if (isset($tokens[0]) && $tokens[0]->value == "DISTINCT") {
$distinct = true;
$pos++;
}

while ($pos < $token_count) {
$arg = $tokens[$pos];


if ($arg->value === ',') {
if ($needs_comma) {
$needs_comma = false;
Expand All @@ -151,14 +163,28 @@ private function getListExpression(array $tokens)
}
}

if ($arg->value === 'ORDER') {
$p = new OrderByParser($pos, $tokens);
[$pos, $order_by] = $p->parse();
$pos++; // ORDER BY の次の式の先頭に position を合わせる
continue;
}

if ($arg->value === 'SEPARATOR') {
$p = new ExpressionParser($tokens, $pos);
list(, $expr) = $p->buildWithPointer();
$separator = $expr;
break;
}

$p = new ExpressionParser($tokens, $pos - 1);
list($pos, $expr) = $p->buildWithPointer();
$args[] = $expr;
$pos++;
$needs_comma = true;
}

return [$distinct, $args];
return [$distinct, $args, $order_by, $separator];
}

/**
Expand Down Expand Up @@ -272,8 +298,8 @@ function ($token) {

$fn = new CastExpression($token, $expr, $type);
} else {
list($distinct, $args) = $this->getListExpression($arg_tokens);
$fn = new FunctionExpression($token, $args, $distinct);
list($distinct, $args, $order, $separator) = $this->getListExpression($arg_tokens);
$fn = new FunctionExpression($token, $args, $distinct, $order, $separator);
}

$this->pointer = $closing_paren_pointer;
Expand Down
4 changes: 3 additions & 1 deletion src/Processor/Expression/BinaryOperatorEvaluator.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ public static function evaluate(
$left,
$right,
],
false
false,
null,
null
),
$row,
$result
Expand Down
64 changes: 53 additions & 11 deletions src/Processor/Expression/FunctionEvaluator.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
namespace Vimeo\MysqlEngine\Processor\Expression;

use Closure;
use Vimeo\MysqlEngine\FakePdoInterface;
use Vimeo\MysqlEngine\Processor\ProcessorException;
use Vimeo\MysqlEngine\Processor\QueryResult;
Expand Down Expand Up @@ -67,7 +68,7 @@ public static function evaluate(
case 'CONCAT':
return self::sqlConcat($conn, $scope, $expr, $row, $result);
case 'GROUP_CONCAT':
return self::sqlGroupConcat($conn, $scope, $expr, $row, $result);
return self::sqlGroupConcat($conn, $scope, $expr, $result);
case 'FIELD':
return self::sqlColumn($conn, $scope, $expr, $row, $result);
case 'BINARY':
Expand Down Expand Up @@ -938,21 +939,62 @@ private static function sqlConcat(
* @param array<string, mixed> $row
*/
private static function sqlGroupConcat(
FakePdoInterface $conn,
Scope $scope,
FakePdoInterface $conn,
Scope $scope,
FunctionExpression $expr,
array $row,
QueryResult $result
): string {
QueryResult $result
): string
{
$args = $expr->args;

$final_concat = "";
foreach ($args as $arg) {
$val = (string) Evaluator::evaluate($conn, $scope, $arg, $row, $result);
$final_concat .= $val;
$items = [];
foreach ($result->rows as $row) {
$tmp_str = "";
/** @var Closure(array{direction: "ASC"|"DESC", expression: Expression}): ?scalar $func */
/** @psalm-suppress MissingClosureReturnType */
$func = function (array $order) use ($result, $row, $scope, $conn) {
/** @var array{expression: Expression} $order */
return Evaluator::evaluate($conn, $scope, $order["expression"], $row, $result);
};
$orders = array_map(
$func,
$expr->order ?? []
);
foreach ($args as $arg) {
$val = (string)Evaluator::evaluate($conn, $scope, $arg, $row, $result);
$tmp_str .= $val;
}
if ($tmp_str !== "" && (!$expr->distinct || !isset($items[$tmp_str]))) {
$items[$tmp_str] = ["val" => $tmp_str, "orders" => $orders];
}
}

return $final_concat;
usort($items, function ($a, $b) use ($expr): int {
/**
* @var array{val: string, orders: array<int, scalar>} $a
* @var array{val: string, orders: array<int, scalar>} $b
*/
for ($i = 0; $i < count($expr->order ?? []); $i++) {
$direction = $expr->order[$i]["direction"] ?? 'ASC';
$a_val = $a["orders"][$i];
$b_val = $b["orders"][$i];

if ($a_val < $b_val) {
return ($direction === 'ASC') ? -1 : 1;
} elseif ($a_val > $b_val) {
return ($direction === 'ASC') ? 1 : -1;
}
}
return 0;
});

if (isset($expr->separator)) {
$separator = (string)(Evaluator::evaluate($conn, $scope, $expr->separator, [], $result));
} else {
$separator = ",";
}

return implode($separator, array_column($items, 'val'));
}

/**
Expand Down
24 changes: 18 additions & 6 deletions src/Query/Expression/FunctionExpression.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
<?php

namespace Vimeo\MysqlEngine\Query\Expression;

use Vimeo\MysqlEngine\Parser\Token;
use Vimeo\MysqlEngine\TokenType;
use Vimeo\MysqlEngine\Processor\ProcessorException;

final class FunctionExpression extends Expression
{
Expand Down Expand Up @@ -31,12 +30,23 @@ final class FunctionExpression extends Expression
* @var bool
*/
public $distinct;
/** @var ?array<int, array{expression: Expression, direction: 'ASC'|'DESC'}> $order */
public $order;
/** @var ?Expression $separator */
public $separator;

/**
* @param Token $token
* @param array<int, Expression> $args
* @param array<int, Expression> $args
* @param ?array<int, array{expression: Expression, direction: 'ASC'|'DESC'}> $order
*/
public function __construct(Token $token, array $args, bool $distinct)
public function __construct(
Token $token,
array $args,
bool $distinct,
?array $order,
?Expression $separator
)
{
$this->token = $token;
$this->args = $args;
Expand All @@ -45,8 +55,10 @@ public function __construct(Token $token, array $args, bool $distinct)
$this->precedence = 0;
$this->functionName = $token->value;
$this->name = $token->value;
$this->operator = (string) $this->type;
$this->operator = $this->type;
$this->start = $token->start;
$this->separator = $separator;
$this->order = $order;
}

/**
Expand All @@ -57,7 +69,7 @@ public function functionName()
return $this->functionName;
}

public function hasAggregate() : bool
public function hasAggregate(): bool
{
if ($this->functionName === 'COUNT'
|| $this->functionName === 'SUM'
Expand Down
28 changes: 28 additions & 0 deletions tests/EndToEndTest.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

namespace Vimeo\MysqlEngine\Tests;

use PDOException;
Expand Down Expand Up @@ -911,6 +912,33 @@ public function testInsertWithNullAndEmpty()
);
}

public function testGroupConcat()
{
$pdo = self::getConnectionToFullDB(false);

$query = $pdo->prepare(
'SELECT `type`, GROUP_CONCAT(DISTINCT `profession` ORDER BY `name` SEPARATOR \' \') as `profession_list`
FROM `video_game_characters`
GROUP BY `type`'
);

$query->execute();

$this->assertSame(
[
[
"type" => "hero",
"profession_list" => "monkey sure earthworm not sure boxer plumber yellow circle pokemon princess hedgehog dinosaur"
],
[
"type" => "villain",
"profession_list" => "evil dinosaur evil chain dude evil doctor throwing shit from clouds"
],
],
$query->fetchAll(\PDO::FETCH_ASSOC)
);
}

public function testOrderBySecondDimensionAliased()
{
$pdo = self::getConnectionToFullDB(false);
Expand Down
Loading