Skip to content

Commit 47ccddc

Browse files
Javakky-pxvJavakky-pxv
committed
fix: Support ORDER AND SEPARATOR in GROUP_CONCAT
1 parent 95cf197 commit 47ccddc

File tree

5 files changed

+85
-22
lines changed

5 files changed

+85
-22
lines changed

‎src/Parser/ExpressionParser.php

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,12 @@ public function __construct(
127127
/**
128128
* @param array<int, Token> $tokens
129129
*
130-
* @return array{0: bool, 1: array<int, Expression>}
130+
* @return array{
131+
* 0: bool,
132+
* 1: list<Expression>,
133+
* 2: ?array<int, array{direction: 'ASC'|'DESC', expression: Expression}>,
134+
* 3: ?Expression
135+
* }
131136
*/
132137
private function getListExpression(array $tokens)
133138
{
@@ -137,6 +142,9 @@ private function getListExpression(array $tokens)
137142
$needs_comma = false;
138143
$args = [];
139144

145+
$order_by = null;
146+
$separator = null;
147+
140148
if (isset($tokens[0]) && $tokens[0]->value == "DISTINCT") {
141149
$distinct = true;
142150
$pos++;
@@ -155,21 +163,28 @@ private function getListExpression(array $tokens)
155163
}
156164
}
157165

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

173+
if ($arg->value === 'SEPARATOR') {
174+
$p = new ExpressionParser($tokens, $pos);
175+
list(, $expr) = $p->buildWithPointer();
176+
$separator = $expr;
177+
break;
178+
}
179+
165180
$p = new ExpressionParser($tokens, $pos - 1);
166181
list($pos, $expr) = $p->buildWithPointer();
167182
$args[] = $expr;
168183
$pos++;
169184
$needs_comma = true;
170185
}
171186

172-
return [$distinct, $args, $order_by ?? null];
187+
return [$distinct, $args, $order_by, $separator];
173188
}
174189

175190
/**
@@ -283,8 +298,8 @@ function ($token) {
283298

284299
$fn = new CastExpression($token, $expr, $type);
285300
} else {
286-
list($distinct, $args) = $this->getListExpression($arg_tokens);
287-
$fn = new FunctionExpression($token, $args, $distinct);
301+
list($distinct, $args, $order, $separator) = $this->getListExpression($arg_tokens);
302+
$fn = new FunctionExpression($token, $args, $distinct, $order, $separator);
288303
}
289304

290305
$this->pointer = $closing_paren_pointer;

‎src/Processor/Expression/BinaryOperatorEvaluator.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ public static function evaluate(
6666
$left,
6767
$right,
6868
],
69-
false
69+
false,
70+
null,
71+
null
7072
),
7173
$row,
7274
$result

‎src/Processor/Expression/FunctionEvaluator.php

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -938,26 +938,60 @@ private static function sqlConcat(
938938
* @param array<string, mixed> $row
939939
*/
940940
private static function sqlGroupConcat(
941-
FakePdoInterface $conn,
942-
Scope $scope,
941+
FakePdoInterface $conn,
942+
Scope $scope,
943943
FunctionExpression $expr,
944-
QueryResult $result
945-
): string {
944+
QueryResult $result
945+
): string
946+
{
946947
$args = $expr->args;
947948

948949
$items = [];
949950
foreach ($result->rows as $row) {
950951
$tmp_str = "";
952+
$orders = array_map(
953+
954+
function (array $order) use ($result, $row, $scope, $conn) { // @phpstan-ignore The type of $row is indeterminate throughout the project
955+
/** @var array{expression: Expression} $order */
956+
return Evaluator::evaluate($conn, $scope, $order["expression"], $row, $result);
957+
},
958+
$expr->order ?? []
959+
);
951960
foreach ($args as $arg) {
952-
$val = (string) Evaluator::evaluate($conn, $scope, $arg, $row, $result);
961+
$val = (string)Evaluator::evaluate($conn, $scope, $arg, $row, $result);
953962
$tmp_str .= $val;
954963
}
955964
if ($tmp_str !== "" && (!$expr->distinct || !isset($items[$tmp_str]))) {
956-
$items[$tmp_str] = $tmp_str;
965+
$items[$tmp_str] = ["val" => $tmp_str, "orders" => $orders];
966+
}
967+
}
968+
969+
usort($items, function ($a, $b) use ($expr): int {
970+
/**
971+
* @var array{val: string, orders: array<int, scalar>} $a
972+
* @var array{val: string, orders: array<int, scalar>} $b
973+
*/
974+
for ($i = 0; $i < count($expr->order ?? []); $i++) {
975+
$direction = $expr->order[$i]["direction"] ?? 'ASC';
976+
$a_val = $a["orders"][$i];
977+
$b_val = $b["orders"][$i];
978+
979+
if ($a_val < $b_val) {
980+
return ($direction === 'ASC') ? -1 : 1;
981+
} elseif ($a_val > $b_val) {
982+
return ($direction === 'ASC') ? 1 : -1;
983+
}
957984
}
985+
return 0;
986+
});
987+
988+
if (isset($expr->separator)) {
989+
$separator = (string)(Evaluator::evaluate($conn, $scope, $expr->separator, [], $result));
990+
} else {
991+
$separator = ",";
958992
}
959993

960-
return implode(",", array_values($items));
994+
return implode($separator, array_column($items, 'val'));
961995
}
962996

963997
/**

‎src/Query/Expression/FunctionExpression.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
<?php
2+
23
namespace Vimeo\MysqlEngine\Query\Expression;
34

45
use Vimeo\MysqlEngine\Parser\Token;
5-
use Vimeo\MysqlEngine\TokenType;
6-
use Vimeo\MysqlEngine\Processor\ProcessorException;
76

87
final class FunctionExpression extends Expression
98
{
@@ -31,12 +30,23 @@ final class FunctionExpression extends Expression
3130
* @var bool
3231
*/
3332
public $distinct;
33+
/** @var ?array<int, array{expression: Expression, direction: 'ASC'|'DESC'}> $order */
34+
public $order;
35+
/** @var ?Expression $separator */
36+
public $separator;
3437

3538
/**
3639
* @param Token $token
37-
* @param array<int, Expression> $args
40+
* @param array<int, Expression> $args
41+
* @param ?array<int, array{expression: Expression, direction: 'ASC'|'DESC'}> $order
3842
*/
39-
public function __construct(Token $token, array $args, bool $distinct)
43+
public function __construct(
44+
Token $token,
45+
array $args,
46+
bool $distinct,
47+
?array $order,
48+
?Expression $separator
49+
)
4050
{
4151
$this->token = $token;
4252
$this->args = $args;
@@ -45,8 +55,10 @@ public function __construct(Token $token, array $args, bool $distinct)
4555
$this->precedence = 0;
4656
$this->functionName = $token->value;
4757
$this->name = $token->value;
48-
$this->operator = (string) $this->type;
58+
$this->operator = $this->type;
4959
$this->start = $token->start;
60+
$this->separator = $separator;
61+
$this->order = $order;
5062
}
5163

5264
/**
@@ -57,7 +69,7 @@ public function functionName()
5769
return $this->functionName;
5870
}
5971

60-
public function hasAggregate() : bool
72+
public function hasAggregate(): bool
6173
{
6274
if ($this->functionName === 'COUNT'
6375
|| $this->functionName === 'SUM'

‎tests/EndToEndTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ public function testGroupConcat()
917917
$pdo = self::getConnectionToFullDB(false);
918918

919919
$query = $pdo->prepare(
920-
'SELECT `type`, GROUP_CONCAT(DISTINCT `profession`) as `profession_list`
920+
'SELECT `type`, GROUP_CONCAT(DISTINCT `profession` ORDER BY `name` SEPARATOR \' \') as `profession_list`
921921
FROM `video_game_characters`
922922
GROUP BY `type`'
923923
);
@@ -928,11 +928,11 @@ public function testGroupConcat()
928928
[
929929
[
930930
"type" => "hero",
931-
"profession_list" => "plumber,hedgehog,earthworm,monkey,pokemon,princess,boxer,yellow circle,dinosaur,not sure,sure"
931+
"profession_list" => "monkey sure earthworm not sure boxer plumber yellow circle pokemon princess hedgehog dinosaur"
932932
],
933933
[
934934
"type" => "villain",
935-
"profession_list" => "evil dinosaur,evil doctor,throwing shit from clouds,evil chain dude"
935+
"profession_list" => "evil dinosaur evil chain dude evil doctor throwing shit from clouds"
936936
],
937937
],
938938
$query->fetchAll(\PDO::FETCH_ASSOC)

0 commit comments

Comments
 (0)