Skip to content

Commit aacc247

Browse files
mvorisekmvorisek
authored
Fix HasMany concat with non-physical field (#1085)
1 parent 24e5a96 commit aacc247

14 files changed

Lines changed: 88 additions & 77 deletions

File tree

‎src/Field/SqlExpressionField.php‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,32 +26,15 @@ class SqlExpressionField extends Field
2626
/** @var string Specifies how to aggregate this. */
2727
public $aggregate;
2828

29-
/** @var string Aggregation by concatenation. */
30-
public $concat;
29+
/** @var string */
30+
public $concatSeparator;
3131

3232
/** @var Reference\HasMany|null When defining as aggregate, this will point to relation object. */
3333
public $aggregateRelation;
3434

3535
/** @var string Specifies which field to use. */
3636
public $field;
3737

38-
protected function init(): void
39-
{
40-
$this->_init();
41-
42-
if ($this->concat) {
43-
$this->onHookToOwnerEntity(Model::HOOK_AFTER_SAVE, \Closure::fromCallable([$this, 'afterSave']));
44-
}
45-
}
46-
47-
/**
48-
* Possibly that user will attempt to insert values here. If that is the case, then
49-
* we would need to inject it into related hasMany relationship.
50-
*/
51-
public function afterSave(Model $entity): void
52-
{
53-
}
54-
5538
/**
5639
* Should this field use alias?
5740
* Expression fields always need alias.

‎src/Model.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -863,9 +863,7 @@ public function setId($value)
863863
*/
864864
public function getModelCaption(): string
865865
{
866-
return $this->caption ?? $this->readableCaption(
867-
(new \ReflectionClass(static::class))->isAnonymous() ? get_parent_class(static::class) : static::class
868-
);
866+
return $this->caption ?? $this->readableCaption(get_debug_type($this));
869867
}
870868

871869
/**

‎src/Persistence/Sql.php‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -415,20 +415,25 @@ public function action(Model $model, string $type, array $args = [])
415415
[$fx, $field] = $args;
416416
$field = is_string($field) ? $model->getField($field) : $field;
417417

418-
if ($type === 'fx') {
419-
$expr = $fx . '([])';
418+
$query = $this->action($model, 'select', [[]]);
419+
420+
if ($fx === 'concat') {
421+
$expr = $query->groupConcat($field, $args['concatSeparator']);
420422
} else {
421-
$expr = 'coalesce(' . $fx . '([]), 0)';
423+
$expr = $query->expr(
424+
$type === 'fx'
425+
? $fx . '([])'
426+
: 'coalesce(' . $fx . '([]), 0)',
427+
[$field]
428+
);
422429
}
423430

424-
$query = $this->action($model, 'select', [[]]);
425-
426431
if (isset($args['alias'])) {
427-
$query->reset('field')->field($query->expr($expr, [$field]), $args['alias']);
432+
$query->reset('field')->field($expr, $args['alias']);
428433
} elseif ($field instanceof SqlExpressionField) {
429-
$query->reset('field')->field($query->expr($expr, [$field]), $fx . '_' . $field->shortName);
434+
$query->reset('field')->field($expr, $fx . '_' . $field->shortName);
430435
} else {
431-
$query->reset('field')->field($query->expr($expr, [$field]));
436+
$query->reset('field')->field($expr);
432437
}
433438
$this->fixMssqlOracleMissingFieldsInGroup($model, $query);
434439

‎src/Persistence/Sql/Mssql/Query.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ protected function _renderLimit(): ?string
5151
. ' fetch next ' . $cnt . ' rows only';
5252
}
5353

54-
public function groupConcat($field, string $delimiter = ',')
54+
public function groupConcat($field, string $separator = ',')
5555
{
56-
return $this->expr('string_agg({}, ' . $this->escapeStringLiteral($delimiter) . ')', [$field]);
56+
return $this->expr('string_agg({}, ' . $this->escapeStringLiteral($separator) . ')', [$field]);
5757
}
5858

5959
public function exists()

‎src/Persistence/Sql/Mysql/Query.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ class Query extends BaseQuery
1717

1818
protected string $templateUpdate = 'update [table][join] set [set] [where]';
1919

20-
public function groupConcat($field, string $delimiter = ',')
20+
public function groupConcat($field, string $separator = ',')
2121
{
22-
return $this->expr('group_concat({} separator ' . $this->escapeStringLiteral($delimiter) . ')', [$field]);
22+
return $this->expr('group_concat({} separator ' . $this->escapeStringLiteral($separator) . ')', [$field]);
2323
}
2424
}

‎src/Persistence/Sql/Oracle/Query.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ protected function _renderLimit(): ?string
7979
. ' fetch next ' . $cnt . ' rows only';
8080
}
8181

82-
public function groupConcat($field, string $delimiter = ',')
82+
public function groupConcat($field, string $separator = ',')
8383
{
84-
return $this->expr('listagg({field}, []) within group (order by {field})', ['field' => $field, $delimiter]);
84+
return $this->expr('listagg({field}, []) within group (order by {field})', ['field' => $field, $separator]);
8585
}
8686

8787
public function exists()

‎src/Persistence/Sql/Postgresql/Query.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ protected function _renderLimit(): ?string
3838
. ' offset ' . (int) $this->args['limit']['shift'];
3939
}
4040

41-
public function groupConcat($field, string $delimiter = ','): BaseExpression
41+
public function groupConcat($field, string $separator = ','): BaseExpression
4242
{
43-
return $this->expr('string_agg({}, [])', [$field, $delimiter]);
43+
return $this->expr('string_agg({}, [])', [$field, $separator]);
4444
}
4545
}

‎src/Persistence/Sql/Query.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1036,7 +1036,7 @@ public function andExpr()
10361036
*
10371037
* @return Expression
10381038
*/
1039-
public function groupConcat($field, string $delimiter = ',')
1039+
public function groupConcat($field, string $separator = ',')
10401040
{
10411041
throw new Exception('groupConcat() is SQL-dependent, so use a correct class');
10421042
}

‎src/Persistence/Sql/Sqlite/Query.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ class Query extends BaseQuery
1313

1414
protected string $templateTruncate = 'delete [from] [tableNoalias]';
1515

16-
public function groupConcat($field, string $delimiter = ',')
16+
public function groupConcat($field, string $separator = ',')
1717
{
18-
return $this->expr('group_concat({}, [])', [$field, $delimiter]);
18+
return $this->expr('group_concat({}, [])', [$field, $separator]);
1919
}
2020
}

‎src/Reference/HasMany.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ public function addField(string $fieldName, array $defaults = []): Field
125125
$field = $alias ?? $fieldName;
126126

127127
if (isset($defaults['concat'])) {
128-
$defaults['aggregate'] = $this->getOurModel(null)->dsql()->groupConcat($field, $defaults['concat']); // TODO better to concat by NUL byte or very unlikely string
128+
$defaults['aggregate'] = 'concat';
129+
$defaults['concatSeparator'] = $defaults['concat'];
130+
unset($defaults['concat']);
129131
}
130132

131133
if (isset($defaults['expr'])) {
@@ -152,7 +154,12 @@ public function addField(string $fieldName, array $defaults = []): Field
152154
};
153155
} else {
154156
$fx = function () use ($defaults, $field) {
155-
return $this->refLink(null)->action('fx', [$defaults['aggregate'], $field]);
157+
$args = [$defaults['aggregate'], $field];
158+
if ($defaults['aggregate'] === 'concat') {
159+
$args['concatSeparator'] = $defaults['concatSeparator'];
160+
}
161+
162+
return $this->refLink(null)->action('fx', $args);
156163
};
157164
}
158165

0 commit comments

Comments
 (0)