From bbdf2e9977cc7ad6324a6c090daa60135117dcf4 Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Sun, 21 Jun 2026 22:00:24 +0200 Subject: [PATCH 1/8] feat(flow-php/etl): configurable storage for aggregating functions GroupBy aggregation can move state out of the PHP heap instead of holding every result in memory. A new AggregationStorage abstraction offers three backends: memory (default, unchanged behavior), filesystem spill+merge, and PSR-16 key-value (APCu/Redis). - MergeableAggregatingFunction + merge() on all built-in aggregators - AggregationStorage: Memory / External (filesystem spill+merge) / Kv (PSR-16) - KV storage fails loudly on eviction / write failure instead of returning a wrong aggregate - config: AggregationStorageStrategy + FLOW_AGGREGATION_MAX_MEMORY wired into ConfigBuilder - APCu enabled in the nix dev shell (for the PSR-16/APCu storage test) --- .nix/php/lib/php.ini.dist | 5 +- .nix/pkgs/flow-php/package.nix | 1 + src/core/etl/src/Flow/ETL/Config.php | 2 + .../Config/Aggregation/AggregationConfig.php | 21 ++ .../Aggregation/AggregationConfigBuilder.php | 79 +++++++ .../etl/src/Flow/ETL/Config/ConfigBuilder.php | 35 ++++ .../etl/src/Flow/ETL/Function/Average.php | 12 +- .../etl/src/Flow/ETL/Function/Collect.php | 12 +- .../src/Flow/ETL/Function/CollectUnique.php | 16 +- src/core/etl/src/Flow/ETL/Function/Count.php | 11 +- src/core/etl/src/Flow/ETL/Function/First.php | 11 +- src/core/etl/src/Flow/ETL/Function/Last.php | 13 +- src/core/etl/src/Flow/ETL/Function/Max.php | 21 +- .../Function/MergeableAggregatingFunction.php | 10 + src/core/etl/src/Flow/ETL/Function/Min.php | 21 +- .../src/Flow/ETL/Function/StringAggregate.php | 13 +- src/core/etl/src/Flow/ETL/Function/Sum.php | 12 +- src/core/etl/src/Flow/ETL/GroupBy.php | 58 ++++-- .../GroupBy/AggregationStorageStrategy.php | 17 ++ .../GroupBy/Storage/AggregationStorage.php | 29 +++ .../FilesystemGroupBucketsCache.php | 58 ++++++ .../Storage/ExternalAggregationStorage.php | 121 +++++++++++ .../Flow/ETL/GroupBy/Storage/GroupBucket.php | 15 ++ .../ETL/GroupBy/Storage/GroupBucketsCache.php | 23 +++ .../Flow/ETL/GroupBy/Storage/GroupEntry.php | 19 ++ .../GroupBy/Storage/KvAggregationStorage.php | 109 ++++++++++ .../Storage/MemoryAggregationStorage.php | 49 +++++ .../Flow/ETL/Processor/GroupByProcessor.php | 32 +++ .../DataFrame/GroupByStorageTest.php | 131 ++++++++++++ .../ExternalAggregationStorageTest.php | 67 ++++++ .../AggregationConfigBuilderTest.php | 71 +++++++ .../Function/AggregatingFunctionMergeTest.php | 194 ++++++++++++++++++ .../Storage/KvAggregationStorageTest.php | 93 +++++++++ 33 files changed, 1348 insertions(+), 33 deletions(-) create mode 100644 src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php create mode 100644 src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php create mode 100644 src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/AggregationStorageStrategy.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/AggregationStorage.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php diff --git a/.nix/php/lib/php.ini.dist b/.nix/php/lib/php.ini.dist index 488665798b..ba9900777c 100644 --- a/.nix/php/lib/php.ini.dist +++ b/.nix/php/lib/php.ini.dist @@ -9,4 +9,7 @@ file_uploads = On max_file_uploads = 20 short_open_tag = off opcache.enable=1 -opcache.enable_cli=0 \ No newline at end of file +opcache.enable_cli=0 +apc.enabled=1 +apc.enable_cli=1 +apc.shm_size=128M \ No newline at end of file diff --git a/.nix/pkgs/flow-php/package.nix b/.nix/pkgs/flow-php/package.nix index 1d6bbd7406..384e33db14 100644 --- a/.nix/pkgs/flow-php/package.nix +++ b/.nix/pkgs/flow-php/package.nix @@ -22,6 +22,7 @@ let with all; enabled ++ [ + apcu bcmath dom mbstring diff --git a/src/core/etl/src/Flow/ETL/Config.php b/src/core/etl/src/Flow/ETL/Config.php index 00ebf0a0c6..4cf96bd43b 100644 --- a/src/core/etl/src/Flow/ETL/Config.php +++ b/src/core/etl/src/Flow/ETL/Config.php @@ -4,6 +4,7 @@ namespace Flow\ETL; +use Flow\ETL\Config\Aggregation\AggregationConfig; use Flow\ETL\Config\Cache\CacheConfig; use Flow\ETL\Config\ConfigBuilder; use Flow\ETL\Config\Sort\SortConfig; @@ -36,6 +37,7 @@ public function __construct( public SortConfig $sort, private ?Analyze $analyze, public TelemetryConfig $telemetry, + public AggregationConfig $aggregation, ) {} public static function builder(): ConfigBuilder diff --git a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php new file mode 100644 index 0000000000..f10975f2d0 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php @@ -0,0 +1,21 @@ +memoryLimit === null) { + $aggregationMemory = getenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); + + if (is_string($aggregationMemory)) { + $this->memoryLimit = Unit::fromString($aggregationMemory); + } else { + $memoryLimit = ini_get('memory_limit'); + + if ($memoryLimit === false || $memoryLimit === '-1') { + $this->memoryLimit = Unit::fromBytes(PHP_INT_MAX); + } else { + $this->memoryLimit = Unit::fromString( + $memoryLimit, + )->percentage(self::DEFAULT_AGGREGATION_MEMORY_PERCENTAGE); + } + } + } + + return new AggregationConfig($this->strategy, $this->memoryLimit, $this->storage, $this->filesystemProtocol); + } + + public function filesystemProtocol(string $protocol): self + { + $this->filesystemProtocol = $protocol; + + return $this; + } + + public function memoryLimit(Unit $memoryLimit): self + { + $this->memoryLimit = $memoryLimit; + + return $this; + } + + public function storage(AggregationStorage $storage): self + { + $this->storage = $storage; + + return $this; + } + + public function strategy(AggregationStorageStrategy $strategy): self + { + $this->strategy = $strategy; + + return $this; + } +} diff --git a/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php b/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php index 02606bbd83..80152a7237 100644 --- a/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php +++ b/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php @@ -8,12 +8,15 @@ use Flow\ETL\Analyze; use Flow\ETL\Cache; use Flow\ETL\Config; +use Flow\ETL\Config\Aggregation\AggregationConfigBuilder; use Flow\ETL\Config\Cache\CacheConfigBuilder; use Flow\ETL\Config\Sort\SortConfigBuilder; use Flow\ETL\Config\Telemetry\TelemetryConfig; use Flow\ETL\Config\Telemetry\TelemetryOptions; use Flow\ETL\Dataset\Memory\Unit; use Flow\ETL\Filesystem\FilesystemStreams; +use Flow\ETL\GroupBy\AggregationStorageStrategy; +use Flow\ETL\GroupBy\Storage\AggregationStorage; use Flow\ETL\NativePHPRandomValueGenerator; use Flow\ETL\Pipeline\Optimizer; use Flow\ETL\Pipeline\Optimizer\BatchSizeOptimization; @@ -34,6 +37,8 @@ final class ConfigBuilder { + public readonly AggregationConfigBuilder $aggregation; + public readonly CacheConfigBuilder $cache; public readonly SortConfigBuilder $sort; @@ -69,6 +74,7 @@ public function __construct() $this->putInputIntoRows = false; $this->optimizer = null; $this->clock = null; + $this->aggregation = new AggregationConfigBuilder(); $this->cache = new CacheConfigBuilder(); $this->sort = new SortConfigBuilder(); $this->randomValueGenerator = new NativePHPRandomValueGenerator(); @@ -79,6 +85,34 @@ public function __construct() : PackageVersion::get('flow-php/etl'); } + public function aggregationFilesystem(string $protocol): self + { + $this->aggregation->filesystemProtocol($protocol); + + return $this; + } + + public function aggregationMemoryLimit(Unit $unit): self + { + $this->aggregation->memoryLimit($unit); + + return $this; + } + + public function aggregationStorage(AggregationStorageStrategy $strategy): self + { + $this->aggregation->strategy($strategy); + + return $this; + } + + public function aggregationStore(AggregationStorage $storage): self + { + $this->aggregation->storage($storage); + + return $this; + } + public function analyze(Analyze $analyze): self { $this->analyze = $analyze; @@ -111,6 +145,7 @@ public function build(EntryFactory $entryFactory = new EntryFactory()): Config $this->sort->build(), $this->analyze, $this->telemetryConfig ?? TelemetryConfig::default($this->getClock()), + $this->aggregation->build(), ); } diff --git a/src/core/etl/src/Flow/ETL/Function/Average.php b/src/core/etl/src/Flow/ETL/Function/Average.php index d404adea22..1676783917 100644 --- a/src/core/etl/src/Flow/ETL/Function/Average.php +++ b/src/core/etl/src/Flow/ETL/Function/Average.php @@ -21,7 +21,7 @@ use function is_int; use function is_numeric; -final class Average implements AggregatingFunction, WindowFunction +final class Average implements AggregatingFunction, MergeableAggregatingFunction, WindowFunction { private int $count; @@ -82,6 +82,16 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return (new Calculator())->divide($sum, $count, $this->scale, $this->rounding); } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + $this->sum += $other->sum; + $this->count += $other->count; + } + public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/Function/Collect.php b/src/core/etl/src/Flow/ETL/Function/Collect.php index 12209f5710..78d4ad51be 100644 --- a/src/core/etl/src/Flow/ETL/Function/Collect.php +++ b/src/core/etl/src/Flow/ETL/Function/Collect.php @@ -11,10 +11,11 @@ use Flow\ETL\Row\EntryFactory; use Flow\ETL\Row\Reference; +use function array_merge; use function current; use function Flow\ETL\DSL\to_entry; -final class Collect implements AggregatingFunction +final class Collect implements AggregatingFunction, MergeableAggregatingFunction { /** * @var array @@ -41,6 +42,15 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + $this->collection = array_merge($this->collection, $other->collection); + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/CollectUnique.php b/src/core/etl/src/Flow/ETL/Function/CollectUnique.php index 3f87b0ff09..fd86c2deaf 100644 --- a/src/core/etl/src/Flow/ETL/Function/CollectUnique.php +++ b/src/core/etl/src/Flow/ETL/Function/CollectUnique.php @@ -15,7 +15,7 @@ use function Flow\ETL\DSL\to_entry; use function in_array; -final class CollectUnique implements AggregatingFunction +final class CollectUnique implements AggregatingFunction, MergeableAggregatingFunction { /** * @var array @@ -49,6 +49,20 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + /** @var mixed $value */ + foreach ($other->collection as $value) { + if (!in_array($value, $this->collection, true)) { + $this->collection[] = $value; + } + } + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Count.php b/src/core/etl/src/Flow/ETL/Function/Count.php index 52e585ef3c..3087659836 100644 --- a/src/core/etl/src/Flow/ETL/Function/Count.php +++ b/src/core/etl/src/Flow/ETL/Function/Count.php @@ -16,7 +16,7 @@ use function Flow\ETL\DSL\int_entry; -final class Count implements AggregatingFunction, WindowFunction +final class Count implements AggregatingFunction, MergeableAggregatingFunction, WindowFunction { private int $count; @@ -80,6 +80,15 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return $count; } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + $this->count += $other->count; + } + public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/Function/First.php b/src/core/etl/src/Flow/ETL/Function/First.php index 36126f0176..9e7c39b750 100644 --- a/src/core/etl/src/Flow/ETL/Function/First.php +++ b/src/core/etl/src/Flow/ETL/Function/First.php @@ -13,7 +13,7 @@ use function Flow\ETL\DSL\string_entry; -final class First implements AggregatingFunction +final class First implements AggregatingFunction, MergeableAggregatingFunction { /** * @var null|Entry @@ -37,6 +37,15 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + $this->first ??= $other->first; + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Last.php b/src/core/etl/src/Flow/ETL/Function/Last.php index d5c1e76f76..8df306d96d 100644 --- a/src/core/etl/src/Flow/ETL/Function/Last.php +++ b/src/core/etl/src/Flow/ETL/Function/Last.php @@ -13,7 +13,7 @@ use function Flow\ETL\DSL\string_entry; -final class Last implements AggregatingFunction +final class Last implements AggregatingFunction, MergeableAggregatingFunction { /** * @var null|Entry @@ -35,6 +35,17 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + if ($other->last !== null) { + $this->last = $other->last; + } + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Max.php b/src/core/etl/src/Flow/ETL/Function/Max.php index 6794783b8c..9f5eda78a7 100644 --- a/src/core/etl/src/Flow/ETL/Function/Max.php +++ b/src/core/etl/src/Flow/ETL/Function/Max.php @@ -18,7 +18,7 @@ use function is_numeric; use function max; -final class Max implements AggregatingFunction +final class Max implements AggregatingFunction, MergeableAggregatingFunction { private float|DateTimeInterface|null $max; @@ -52,6 +52,25 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + if ($other->max === null) { + return; + } + + if ($this->max === null) { + $this->max = $other->max; + + return; + } + + $this->max = max($this->max, $other->max); + } + /** * @return Entry|Entry|Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php b/src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php new file mode 100644 index 0000000000..4065cd7cf7 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php @@ -0,0 +1,10 @@ +min === null) { + return; + } + + if ($this->min === null) { + $this->min = $other->min; + + return; + } + + $this->min = min($this->min, $other->min); + } + /** * @return Entry|Entry|Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/StringAggregate.php b/src/core/etl/src/Flow/ETL/Function/StringAggregate.php index 13f1e468a3..c68fa25544 100644 --- a/src/core/etl/src/Flow/ETL/Function/StringAggregate.php +++ b/src/core/etl/src/Flow/ETL/Function/StringAggregate.php @@ -4,6 +4,7 @@ namespace Flow\ETL\Function; +use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\FlowContext; use Flow\ETL\Row; use Flow\ETL\Row\Entry; @@ -11,6 +12,7 @@ use Flow\ETL\Row\Reference; use Flow\ETL\Row\SortOrder; +use function array_merge; use function count; use function Flow\ETL\DSL\str_entry; use function implode; @@ -18,7 +20,7 @@ use function rsort; use function sort; -final class StringAggregate implements AggregatingFunction +final class StringAggregate implements AggregatingFunction, MergeableAggregatingFunction { /** * @var array @@ -40,6 +42,15 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + $this->values = array_merge($this->values, $other->values); + } + /** * @return Row\Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Sum.php b/src/core/etl/src/Flow/ETL/Function/Sum.php index 77415475c3..e117a9f302 100644 --- a/src/core/etl/src/Flow/ETL/Function/Sum.php +++ b/src/core/etl/src/Flow/ETL/Function/Sum.php @@ -19,7 +19,7 @@ use function Flow\ETL\DSL\int_entry; use function is_numeric; -final class Sum implements AggregatingFunction, WindowFunction +final class Sum implements AggregatingFunction, MergeableAggregatingFunction, WindowFunction { private float|int $sum; @@ -68,6 +68,16 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return $sum; } + public function merge(MergeableAggregatingFunction $other): void + { + if (!$other instanceof self) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); + } + + // @mago-ignore analysis:possibly-invalid-argument + $this->sum = (new Calculator())->add($this->sum, $other->sum); + } + public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/GroupBy.php b/src/core/etl/src/Flow/ETL/GroupBy.php index d606797cd4..3329121704 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy.php +++ b/src/core/etl/src/Flow/ETL/GroupBy.php @@ -9,6 +9,9 @@ use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\Exception\RuntimeException; use Flow\ETL\Function\AggregatingFunction; +use Flow\ETL\Function\MergeableAggregatingFunction; +use Flow\ETL\GroupBy\Storage\AggregationStorage; +use Flow\ETL\GroupBy\Storage\MemoryAggregationStorage; use Flow\ETL\Hash\NativePHPHash; use Flow\ETL\Row\Reference; use Flow\ETL\Row\References; @@ -36,11 +39,6 @@ final class GroupBy */ private array $aggregations; - /** - * @var array, aggregators: array}> - */ - private array $groupedTable; - private ?Reference $pivot; /** @@ -55,14 +53,16 @@ final class GroupBy private readonly References $refs; + private AggregationStorage $storage; + public function __construct(string|Reference ...$entries) { $this->refs = References::init(...array_unique($entries)); $this->aggregations = []; - $this->groupedTable = []; $this->pivotedTable = []; $this->pivotColumns = []; $this->pivot = null; + $this->storage = new MemoryAggregationStorage(); } public function aggregate(AggregatingFunction ...$aggregator): void @@ -80,6 +80,17 @@ public function aggregate(AggregatingFunction ...$aggregator): void $this->aggregations = $aggregator; } + public function allAggregationsMergeable(): bool + { + foreach ($this->aggregations as $aggregation) { + if (!$aggregation instanceof MergeableAggregatingFunction) { + return false; + } + } + + return true; + } + public function group(Rows $rows, FlowContext $context): void { $pivot = $this->pivot; @@ -146,26 +157,22 @@ public function group(Rows $rows, FlowContext $context): void $valuesHash = $this->hash($values); - if (!array_key_exists($valuesHash, $this->groupedTable)) { - $aggregators = []; - - foreach ($this->aggregations as $aggregator) { - $aggregators[] = clone $aggregator; - } - - $this->groupedTable[$valuesHash] = [ - 'values' => $values, - 'aggregators' => $aggregators, - ]; - } + $entry = $this->storage->entry($valuesHash, $values, $this->aggregations); - foreach ($this->groupedTable[$valuesHash]['aggregators'] as $aggregator) { + foreach ($entry->aggregators as $aggregator) { $aggregator->aggregate($row, $context); } + + $this->storage->save($valuesHash, $entry); } } } + public function isPivot(): bool + { + return $this->pivot !== null; + } + public function pivot(Reference $ref): void { $this->pivot = $ref; @@ -199,15 +206,17 @@ public function result(FlowContext $context): Rows return array_to_rows($rows, $context->entryFactory()); } - foreach ($this->groupedTable as $group) { + $this->storage->flush(); + + foreach ($this->storage->all() as $group) { $entries = []; /** @var mixed $value */ - foreach ($group['values'] ?? [] as $entry => $value) { + foreach ($group->values as $entry => $value) { $entries[] = $context->entryFactory()->create($entry, $value); } - foreach ($group['aggregators'] as $aggregator) { + foreach ($group->aggregators as $aggregator) { $entries[] = $aggregator->result($context->entryFactory()); } @@ -219,6 +228,11 @@ public function result(FlowContext $context): Rows return new Rows(...$rows); } + public function useStorage(AggregationStorage $storage): void + { + $this->storage = $storage; + } + /** * @param array $values */ diff --git a/src/core/etl/src/Flow/ETL/GroupBy/AggregationStorageStrategy.php b/src/core/etl/src/Flow/ETL/GroupBy/AggregationStorageStrategy.php new file mode 100644 index 0000000000..18b30633c3 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/AggregationStorageStrategy.php @@ -0,0 +1,17 @@ + + */ + public function all(): Generator; + + /** + * @param array $values + * @param array $prototypes + */ + public function entry(string $hash, array $values, array $prototypes): GroupEntry; + + public function flush(): void; + + public function save(string $hash, GroupEntry $entry): void; +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php new file mode 100644 index 0000000000..425ed8633f --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php @@ -0,0 +1,58 @@ +cacheDir = ($cacheDir ?? $this->filesystem->getSystemTmpDir())->suffix('/flow-php-group-by/'); + } + + public function get(string $bucketId): array + { + $path = $this->keyPath($bucketId); + + if (!$this->filesystem->status($path)) { + return []; + } + + $stream = $this->filesystem->readFrom($path); + $content = $stream->content(); + $stream->close(); + + return $this->serializer->unserialize($content, [GroupBucket::class])->entries; + } + + public function remove(string $bucketId): void + { + $this->filesystem->rm($this->keyPath($bucketId)->parentDirectory()); + } + + public function set(string $bucketId, array $entries): void + { + $stream = $this->filesystem->writeTo($this->keyPath($bucketId)); + $stream->append($this->serializer->serialize(new GroupBucket($entries))); + $stream->close(); + } + + private function keyPath(string $key): Path + { + return $this->cacheDir->suffix(NativePHPHash::xxh128($key) . '/' . $key . '.php.cache'); + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php new file mode 100644 index 0000000000..7885259db3 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php @@ -0,0 +1,121 @@ + + */ + private array $hot = []; + + /** + * @var list + */ + private array $spilledBuckets = []; + + public function __construct( + private readonly GroupBucketsCache $cache, + private readonly Unit $memoryLimit, + ) { + $this->consumption = new Consumption(false); + } + + /** + * @return Generator + */ + public function all(): Generator + { + if ($this->spilledBuckets === []) { + foreach ($this->hot as $entry) { + yield $entry; + } + + return; + } + + /** @var array $merged */ + $merged = []; + + foreach ($this->spilledBuckets as $bucketId) { + foreach ($this->cache->get($bucketId) as $hash => $entry) { + if (!array_key_exists($hash, $merged)) { + $merged[$hash] = $entry; + + continue; + } + + foreach ($merged[$hash]->aggregators as $index => $aggregator) { + $partial = $entry->aggregators[$index]; + + if ( + $aggregator instanceof MergeableAggregatingFunction + && $partial instanceof MergeableAggregatingFunction + ) { + $aggregator->merge($partial); + } + } + } + + $this->cache->remove($bucketId); + } + + foreach ($merged as $entry) { + yield $entry; + } + } + + public function entry(string $hash, array $values, array $prototypes): GroupEntry + { + if (!array_key_exists($hash, $this->hot)) { + $aggregators = []; + + foreach ($prototypes as $prototype) { + $aggregators[] = clone $prototype; + } + + $this->hot[$hash] = new GroupEntry($values, $aggregators); + } + + return $this->hot[$hash]; + } + + public function flush(): void + { + if ($this->spilledBuckets !== [] && $this->hot !== []) { + $this->spill(); + } + } + + public function save(string $hash, GroupEntry $entry): void + { + if ($this->consumption->currentDiff()->isGreaterThan($this->memoryLimit)) { + $this->spill(); + } + } + + private function spill(): void + { + $bucketId = bin2hex(random_bytes(16)); + $this->cache->set($bucketId, $this->hot); + $this->spilledBuckets[] = $bucketId; + $this->hot = []; + $this->consumption = new Consumption(false); + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php new file mode 100644 index 0000000000..38860554e8 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php @@ -0,0 +1,15 @@ + $entries + */ + public function __construct( + public array $entries, + ) {} +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php new file mode 100644 index 0000000000..025e523968 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php @@ -0,0 +1,23 @@ + + */ + public function get(string $bucketId): array; + + public function remove(string $bucketId): void; + + /** + * @param array $entries + */ + public function set(string $bucketId, array $entries): void; +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php new file mode 100644 index 0000000000..40f90004cf --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php @@ -0,0 +1,19 @@ + $values + * @param array $aggregators + */ + public function __construct( + public array $values, + public array $aggregators, + ) {} +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php new file mode 100644 index 0000000000..d15fa2c2f2 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php @@ -0,0 +1,109 @@ + + */ + private array $hashes = []; + + public function __construct( + private readonly CacheInterface $cache, + private readonly Serializer $serializer = new NativePHPSerializer(), + private readonly string $namespace = 'flow_group_by_', + ) {} + + /** + * @return Generator + */ + public function all(): Generator + { + foreach (array_keys($this->hashes) as $hash) { + $entry = $this->read($hash); + + if ($entry === null) { + throw $this->evicted($hash); + } + + yield $entry; + + $this->cache->delete($this->namespace . $hash); + } + } + + public function entry(string $hash, array $values, array $prototypes): GroupEntry + { + $existing = $this->read($hash); + + if ($existing !== null) { + return $existing; + } + + if (isset($this->hashes[$hash])) { + throw $this->evicted($hash); + } + + $aggregators = []; + + foreach ($prototypes as $prototype) { + $aggregators[] = clone $prototype; + } + + $this->hashes[$hash] = true; + + return new GroupEntry($values, $aggregators); + } + + public function flush(): void {} + + public function save(string $hash, GroupEntry $entry): void + { + $this->hashes[$hash] = true; + + if (!$this->cache->set($this->namespace . $hash, $this->serializer->serialize($entry))) { + throw new RuntimeException( + 'Failed to write aggregation group to the KV store (store full?). Increase its capacity ' + . '(e.g. apc.shm_size, Redis maxmemory) or use AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL.', + ); + } + } + + private function evicted(string $hash): RuntimeException + { + return new RuntimeException( + 'Aggregation group "' + . $hash + . '" was evicted from the KV store mid-aggregation; the result ' + . 'would be incorrect. Increase the store capacity (e.g. apc.shm_size, Redis maxmemory) or use ' + . 'AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL.', + ); + } + + private function read(string $hash): ?GroupEntry + { + // @mago-ignore analysis:mixed-assignment + $serialized = $this->cache->get($this->namespace . $hash); + + if (!is_string($serialized)) { + return null; + } + + return $this->serializer->unserialize($serialized, [GroupEntry::class]); + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php new file mode 100644 index 0000000000..d87dc6f2af --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php @@ -0,0 +1,49 @@ + + */ + private array $groups = []; + + /** + * @return Generator + */ + public function all(): Generator + { + foreach ($this->groups as $group) { + yield $group; + } + } + + public function entry(string $hash, array $values, array $prototypes): GroupEntry + { + if (!array_key_exists($hash, $this->groups)) { + $aggregators = []; + + foreach ($prototypes as $prototype) { + $aggregators[] = clone $prototype; + } + + $this->groups[$hash] = new GroupEntry($values, $aggregators); + } + + return $this->groups[$hash]; + } + + public function flush(): void {} + + public function save(string $hash, GroupEntry $entry): void {} +} diff --git a/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php b/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php index 46f0050ad6..bc0ed3433b 100644 --- a/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php +++ b/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php @@ -6,6 +6,10 @@ use Flow\ETL\FlowContext; use Flow\ETL\GroupBy; +use Flow\ETL\GroupBy\Storage\AggregationStorage; +use Flow\ETL\GroupBy\Storage\BucketsCache\FilesystemGroupBucketsCache; +use Flow\ETL\GroupBy\Storage\ExternalAggregationStorage; +use Flow\ETL\GroupBy\Storage\MemoryAggregationStorage; use Flow\ETL\Processor; use Generator; @@ -22,10 +26,38 @@ public function __construct( public function process(Generator $rows, FlowContext $context): Generator { + $this->groupBy->useStorage($this->createStorage($context)); + foreach ($rows as $batch) { $this->groupBy->group($batch, $context); } yield $this->groupBy->result($context); } + + private function createStorage(FlowContext $context): AggregationStorage + { + $config = $context->config->aggregation; + + if ($config->storage !== null) { + return $config->storage; + } + + if ( + $config->strategy->useMemory() + || $this->groupBy->isPivot() + || !$this->groupBy->allAggregationsMergeable() + ) { + return new MemoryAggregationStorage(); + } + + return new ExternalAggregationStorage( + new FilesystemGroupBucketsCache( + $context->filesystem($config->filesystemProtocol), + $context->config->serializer(), + $context->config->cache->localFilesystemCacheDir->suffix('/flow-php-group-by/'), + ), + $config->memoryLimit, + ); + } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php new file mode 100644 index 0000000000..3a19f8f7a1 --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php @@ -0,0 +1,131 @@ + + */ + private static ?array $orders = null; + + public function test_apcu_kv_storage_produces_identical_results(): void + { + if (!ApcuAdapter::isSupported()) { + self::markTestSkipped( + 'APCu (with apc.enable_cli=1) is required to evaluate APCu as an aggregation storage backend.', + ); + } + + $cache = new Psr16Cache(new ApcuAdapter('flow_group_by_test')); + $cache->clear(); + + $expected = $this->aggregateBySeller(config_builder()); + $actual = $this->aggregateBySeller(config_builder()->aggregationStore(new KvAggregationStorage($cache))); + + self::assertEqualsCanonicalizing($expected, $actual); + } + + public function test_high_cardinality_group_by_spills_and_stays_correct(): void + { + $memory = $this->aggregateByEmail(config_builder()); + + $external = $this->aggregateByEmail( + config_builder() + ->aggregationStorage(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL) + ->aggregationMemoryLimit(Unit::fromKb(256)), + ); + + self::assertEqualsCanonicalizing($memory, $external); + } + + public function test_kv_storage_produces_identical_results(): void + { + $expected = $this->aggregateBySeller(config_builder()); + + $actual = $this->aggregateBySeller(config_builder()->aggregationStore(new KvAggregationStorage(new Psr16Cache( + new ArrayAdapter(), + )))); + + self::assertEqualsCanonicalizing($expected, $actual); + } + + public function test_seller_aggregation_is_identical_between_memory_and_external_spill_storage(): void + { + $memory = $this->aggregateBySeller(config_builder()); + + $external = $this->aggregateBySeller( + config_builder() + ->aggregationStorage(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL) + ->aggregationMemoryLimit(Unit::fromKb(256)), + ); + + self::assertEqualsCanonicalizing($memory, $external); + self::assertCount(5, $memory); + } + + private function aggregateByEmail(ConfigBuilder $config): array + { + return df($config)->read(from_array($this->orders()))->groupBy('email')->aggregate(count())->fetch()->toArray(); + } + + private function aggregateBySeller(ConfigBuilder $config): array + { + return df($config) + ->read(from_array($this->orders())) + ->groupBy('seller_id') + ->aggregate(count(), sum('discount'), collect('customer')) + ->fetch() + ->toArray(); + } + + /** + * @return list + */ + private function orders(): array + { + if (self::$orders !== null) { + return self::$orders; + } + + $orders = []; + + foreach ((new FakeRandomOrdersExtractor(self::ORDERS))->rawData() as $order) { + /** @var mixed $discount */ + $discount = $order['discount']; + + $orders[] = [ + 'seller_id' => (string) $order['seller_id'], + 'email' => (string) $order['email'], + 'customer' => (string) $order['customer'], + 'discount' => is_numeric($discount) ? (float) $discount : null, + ]; + } + + self::$orders = $orders; + + return $orders; + } +} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php new file mode 100644 index 0000000000..aea7423f04 --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php @@ -0,0 +1,67 @@ +cacheDir->suffix('/external-aggregation-storage/'); + $this->fs()->rm($cacheDir); + + $context = flow_context(); + $storage = new ExternalAggregationStorage( + new FilesystemGroupBucketsCache($this->fs(), $this->serializer(), $cacheDir), + Unit::fromBytes(0), + ); + + $input = [ + ['type' => 'a', 'v' => 1], + ['type' => 'b', 'v' => 2], + ['type' => 'a', 'v' => 3], + ['type' => 'a', 'v' => 4], + ['type' => 'b', 'v' => 5], + ]; + + foreach ($input as $record) { + $entry = $storage->entry($record['type'], ['type' => $record['type']], [sum(ref('v'))]); + + foreach ($entry->aggregators as $aggregator) { + $aggregator->aggregate(row(str_entry('type', $record['type']), int_entry('v', $record['v'])), $context); + } + + $storage->save($record['type'], $entry); + } + + self::assertNotNull($this->fs()->status($cacheDir)); + + $results = []; + + foreach ($storage->all() as $group) { + /** @var int $sum */ + $sum = $group->aggregators[0]->result($context->entryFactory())->value(); + $results[(string) $group->values['type']] = $sum; + } + + ksort($results); + + self::assertSame(['a' => 8, 'b' => 7], $results); + + $this->fs()->rm($cacheDir); + } +} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php new file mode 100644 index 0000000000..112e62b03e --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php @@ -0,0 +1,71 @@ +memoryLimit(Unit::fromMb(16)) + ->build(); + + self::assertSame(Unit::fromMb(16)->inBytes(), $config->memoryLimit->inBytes()); + } + + public function test_default_strategy_is_memory(): void + { + $config = (new AggregationConfigBuilder())->build(); + + self::assertSame(AggregationStorageStrategy::MEMORY, $config->strategy); + self::assertNull($config->storage); + self::assertGreaterThan(0, $config->memoryLimit->inBytes()); + } + + public function test_injecting_a_custom_storage(): void + { + $storage = new KvAggregationStorage(new Psr16Cache(new ArrayAdapter())); + + $config = (new AggregationConfigBuilder()) + ->storage($storage) + ->build(); + + self::assertSame($storage, $config->storage); + } + + public function test_memory_limit_is_read_from_environment_variable(): void + { + putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV . '=10M'); + + try { + $config = (new AggregationConfigBuilder())->build(); + + self::assertSame(Unit::fromMb(10)->inBytes(), $config->memoryLimit->inBytes()); + } finally { + putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); + } + } + + public function test_selecting_the_external_strategy(): void + { + $config = (new AggregationConfigBuilder()) + ->strategy(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL) + ->build(); + + self::assertSame(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL, $config->strategy); + self::assertFalse($config->strategy->useMemory()); + } +} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php new file mode 100644 index 0000000000..8f6d059be3 --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php @@ -0,0 +1,194 @@ +aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = average(ref('v')); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertEquals(2, $left->result($context->entryFactory())->value()); + } + + public function test_collect_merge_appends_values(): void + { + $context = flow_context(); + + $left = collect(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = collect(ref('v')); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); + } + + public function test_collect_unique_merge_unions_values(): void + { + $context = flow_context(); + + $left = collect_unique(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = collect_unique(ref('v')); + $right->aggregate(row(int_entry('v', 2)), $context); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); + } + + public function test_count_merge_adds_counts(): void + { + $context = flow_context(); + + $left = count(); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = count(); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame(3, $left->result($context->entryFactory())->value()); + } + + public function test_first_merge_keeps_earlier_value(): void + { + $context = flow_context(); + + $left = first(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + + $right = first(ref('v')); + $right->aggregate(row(int_entry('v', 9)), $context); + + $left->merge($right); + + self::assertSame(1, $left->result($context->entryFactory())->value()); + } + + public function test_last_merge_keeps_later_value(): void + { + $context = flow_context(); + + $left = last(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + + $right = last(ref('v')); + $right->aggregate(row(int_entry('v', 9)), $context); + + $left->merge($right); + + self::assertSame(9, $left->result($context->entryFactory())->value()); + } + + public function test_max_merge_keeps_greatest(): void + { + $context = flow_context(); + + $left = max(ref('v')); + $left->aggregate(row(int_entry('v', 5)), $context); + $left->aggregate(row(int_entry('v', 3)), $context); + + $right = max(ref('v')); + $right->aggregate(row(int_entry('v', 9)), $context); + + $left->merge($right); + + self::assertSame(9, $left->result($context->entryFactory())->value()); + } + + public function test_merge_with_different_function_type_throws(): void + { + $this->expectException(InvalidArgumentException::class); + + sum(ref('v'))->merge(count()); + } + + public function test_min_merge_keeps_smallest(): void + { + $context = flow_context(); + + $left = min(ref('v')); + $left->aggregate(row(int_entry('v', 5)), $context); + $left->aggregate(row(int_entry('v', 3)), $context); + + $right = min(ref('v')); + $right->aggregate(row(int_entry('v', 1)), $context); + + $left->merge($right); + + self::assertSame(1, $left->result($context->entryFactory())->value()); + } + + public function test_string_aggregate_merge_concatenates_values(): void + { + $context = flow_context(); + + $left = new StringAggregate(ref('v'), ',', SortOrder::ASC); + $left->aggregate(row(str_entry('v', 'b')), $context); + $left->aggregate(row(str_entry('v', 'a')), $context); + + $right = new StringAggregate(ref('v'), ',', SortOrder::ASC); + $right->aggregate(row(str_entry('v', 'c')), $context); + + $left->merge($right); + + self::assertSame('a,b,c', $left->result($context->entryFactory())->value()); + } + + public function test_sum_merge_adds_partial_sums(): void + { + $context = flow_context(); + + $left = sum(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = sum(ref('v')); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame(6, $left->result($context->entryFactory())->value()); + } +} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php new file mode 100644 index 0000000000..4621ff2f9f --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php @@ -0,0 +1,93 @@ +entry($group, ['type' => $group], [sum(ref('v'))]); + $entry->aggregators[0]->aggregate(row(str_entry('type', $group), int_entry('v', 1)), $context); + $storage->save($group, $entry); + } + + $cache->delete('flow_group_by_a'); + + $this->expectException(RuntimeException::class); + + iterator_to_array($storage->all()); + } + + public function test_detects_eviction_on_subsequent_access(): void + { + $cache = new Psr16Cache(new ArrayAdapter()); + $storage = new KvAggregationStorage($cache); + $context = flow_context(); + + $entry = $storage->entry('a', ['type' => 'a'], [sum(ref('v'))]); + $entry->aggregators[0]->aggregate(row(str_entry('type', 'a'), int_entry('v', 1)), $context); + $storage->save('a', $entry); + + $cache->delete('flow_group_by_a'); + + $this->expectException(RuntimeException::class); + + $storage->entry('a', ['type' => 'a'], [sum(ref('v'))]); + } + + public function test_round_trip_produces_correct_aggregates(): void + { + $cache = new Psr16Cache(new ArrayAdapter()); + $storage = new KvAggregationStorage($cache); + $context = flow_context(); + + $input = [ + ['type' => 'a', 'v' => 1], + ['type' => 'b', 'v' => 2], + ['type' => 'a', 'v' => 3], + ]; + + foreach ($input as $record) { + $entry = $storage->entry($record['type'], ['type' => $record['type']], [sum(ref('v'))]); + $entry->aggregators[0]->aggregate( + row(str_entry('type', $record['type']), int_entry('v', $record['v'])), + $context, + ); + $storage->save($record['type'], $entry); + } + + $results = []; + + foreach ($storage->all() as $group) { + /** @var int $value */ + $value = $group->aggregators[0]->result($context->entryFactory())->value(); + $results[(string) $group->values['type']] = $value; + } + + ksort($results); + + self::assertSame(['a' => 4, 'b' => 2], $results); + } +} From 95dee7f03ca43acb78441237d0f7b95d4b20c3fa Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Mon, 6 Jul 2026 22:10:29 +0200 Subject: [PATCH 2/8] refactor(flow-php/etl): sort-based aggregation instead of pluggable storage Reworks the aggregation-storage approach following review. External aggregation is now sort-then-fold: sort rows by the group-by columns (reusing ExternalSort, which spills and merges in bounded memory) and fold adjacent equal keys. This makes the whole abstraction simpler and drops the concepts the review found confusing. - remove the AggregationStorage layer (Memory/External/Kv storage, GroupEntry, GroupBucket(sCache)) and the AggregationStorageStrategy enum - remove MergeableAggregatingFunction and merge() from aggregators - sort-fold never splits a group, so no merge contract is needed - add AggregatingAlgorithm with MemoryAggregation (default, hash map) and ExternalAggregation (sort + fold), selected via AggregationAlgorithms (mirrors SortAlgorithms) - GroupBy is now a spec (references/aggregations/hash/keyValues); GroupByProcessor routes pivot and global aggregation to memory - config: aggregationAlgorithm() replaces aggregationStorage()/aggregationStore(); output is streamed in batches instead of one Rows - external tags rows with an input ordinal and sorts by (group keys, ordinal), so first()/last()/collect() return the same result as the in-memory path even when a group spills across sort runs --- .../Config/Aggregation/AggregationConfig.php | 10 +- .../Aggregation/AggregationConfigBuilder.php | 62 +--- .../etl/src/Flow/ETL/Config/ConfigBuilder.php | 25 +- .../etl/src/Flow/ETL/Function/Average.php | 12 +- .../etl/src/Flow/ETL/Function/Collect.php | 12 +- .../src/Flow/ETL/Function/CollectUnique.php | 16 +- src/core/etl/src/Flow/ETL/Function/Count.php | 11 +- src/core/etl/src/Flow/ETL/Function/First.php | 11 +- src/core/etl/src/Flow/ETL/Function/Last.php | 13 +- src/core/etl/src/Flow/ETL/Function/Max.php | 21 +- .../Function/MergeableAggregatingFunction.php | 10 - src/core/etl/src/Flow/ETL/Function/Min.php | 21 +- .../src/Flow/ETL/Function/StringAggregate.php | 13 +- src/core/etl/src/Flow/ETL/Function/Sum.php | 12 +- src/core/etl/src/Flow/ETL/GroupBy.php | 308 ++++++++++-------- .../Flow/ETL/GroupBy/AggregatingAlgorithm.php | 25 ++ .../ETL/GroupBy/AggregationAlgorithms.php | 25 ++ .../GroupBy/AggregationStorageStrategy.php | 17 - .../Flow/ETL/GroupBy/ExternalAggregation.php | 110 +++++++ .../Flow/ETL/GroupBy/MemoryAggregation.php | 62 ++++ .../GroupBy/Storage/AggregationStorage.php | 29 -- .../FilesystemGroupBucketsCache.php | 58 ---- .../Storage/ExternalAggregationStorage.php | 121 ------- .../Flow/ETL/GroupBy/Storage/GroupBucket.php | 15 - .../ETL/GroupBy/Storage/GroupBucketsCache.php | 23 -- .../Flow/ETL/GroupBy/Storage/GroupEntry.php | 19 -- .../GroupBy/Storage/KvAggregationStorage.php | 109 ------- .../Storage/MemoryAggregationStorage.php | 49 --- .../Flow/ETL/Processor/GroupByProcessor.php | 50 +-- ...ageTest.php => GroupByAggregationTest.php} | 73 ++--- .../ExternalAggregationStorageTest.php | 67 ---- .../AggregationConfigBuilderTest.php | 60 +--- .../Function/AggregatingFunctionMergeTest.php | 194 ----------- .../Storage/KvAggregationStorageTest.php | 93 ------ .../tests/Flow/ETL/Tests/Unit/GroupByTest.php | 90 ++--- .../Unit/Processor/GroupByProcessorTest.php | 9 +- 36 files changed, 533 insertions(+), 1322 deletions(-) delete mode 100644 src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/AggregationStorageStrategy.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/ExternalAggregation.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/AggregationStorage.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php rename src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/{GroupByStorageTest.php => GroupByAggregationTest.php} (55%) delete mode 100644 src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php delete mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php delete mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php diff --git a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php index f10975f2d0..1109d60a1c 100644 --- a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php +++ b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php @@ -4,18 +4,12 @@ namespace Flow\ETL\Config\Aggregation; -use Flow\ETL\Dataset\Memory\Unit; -use Flow\ETL\GroupBy\AggregationStorageStrategy; -use Flow\ETL\GroupBy\Storage\AggregationStorage; +use Flow\ETL\GroupBy\AggregationAlgorithms; final readonly class AggregationConfig { - public const string AGGREGATION_MAX_MEMORY_ENV = 'FLOW_AGGREGATION_MAX_MEMORY'; - public function __construct( - public AggregationStorageStrategy $strategy, - public Unit $memoryLimit, - public ?AggregationStorage $storage = null, + public AggregationAlgorithms $algorithm, public string $filesystemProtocol = 'file', ) {} } diff --git a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php index c5f7076a4b..35277349c6 100644 --- a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php +++ b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php @@ -4,75 +4,29 @@ namespace Flow\ETL\Config\Aggregation; -use Flow\ETL\Dataset\Memory\Unit; -use Flow\ETL\GroupBy\AggregationStorageStrategy; -use Flow\ETL\GroupBy\Storage\AggregationStorage; - -use function getenv; -use function ini_get; -use function is_string; - -use const PHP_INT_MAX; +use Flow\ETL\GroupBy\AggregationAlgorithms; final class AggregationConfigBuilder { - public const int DEFAULT_AGGREGATION_MEMORY_PERCENTAGE = 70; + private AggregationAlgorithms $algorithm = AggregationAlgorithms::MEMORY_AGGREGATION; private string $filesystemProtocol = 'file'; - private ?Unit $memoryLimit = null; - - private ?AggregationStorage $storage = null; - - private AggregationStorageStrategy $strategy = AggregationStorageStrategy::MEMORY; - - public function build(): AggregationConfig - { - if ($this->memoryLimit === null) { - $aggregationMemory = getenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); - - if (is_string($aggregationMemory)) { - $this->memoryLimit = Unit::fromString($aggregationMemory); - } else { - $memoryLimit = ini_get('memory_limit'); - - if ($memoryLimit === false || $memoryLimit === '-1') { - $this->memoryLimit = Unit::fromBytes(PHP_INT_MAX); - } else { - $this->memoryLimit = Unit::fromString( - $memoryLimit, - )->percentage(self::DEFAULT_AGGREGATION_MEMORY_PERCENTAGE); - } - } - } - - return new AggregationConfig($this->strategy, $this->memoryLimit, $this->storage, $this->filesystemProtocol); - } - - public function filesystemProtocol(string $protocol): self - { - $this->filesystemProtocol = $protocol; - - return $this; - } - - public function memoryLimit(Unit $memoryLimit): self + public function algorithm(AggregationAlgorithms $algorithm): self { - $this->memoryLimit = $memoryLimit; + $this->algorithm = $algorithm; return $this; } - public function storage(AggregationStorage $storage): self + public function build(): AggregationConfig { - $this->storage = $storage; - - return $this; + return new AggregationConfig($this->algorithm, $this->filesystemProtocol); } - public function strategy(AggregationStorageStrategy $strategy): self + public function filesystemProtocol(string $protocol): self { - $this->strategy = $strategy; + $this->filesystemProtocol = $protocol; return $this; } diff --git a/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php b/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php index 80152a7237..f98012c89b 100644 --- a/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php +++ b/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php @@ -15,8 +15,7 @@ use Flow\ETL\Config\Telemetry\TelemetryOptions; use Flow\ETL\Dataset\Memory\Unit; use Flow\ETL\Filesystem\FilesystemStreams; -use Flow\ETL\GroupBy\AggregationStorageStrategy; -use Flow\ETL\GroupBy\Storage\AggregationStorage; +use Flow\ETL\GroupBy\AggregationAlgorithms; use Flow\ETL\NativePHPRandomValueGenerator; use Flow\ETL\Pipeline\Optimizer; use Flow\ETL\Pipeline\Optimizer\BatchSizeOptimization; @@ -85,30 +84,16 @@ public function __construct() : PackageVersion::get('flow-php/etl'); } - public function aggregationFilesystem(string $protocol): self - { - $this->aggregation->filesystemProtocol($protocol); - - return $this; - } - - public function aggregationMemoryLimit(Unit $unit): self + public function aggregationAlgorithm(AggregationAlgorithms $algorithm): self { - $this->aggregation->memoryLimit($unit); + $this->aggregation->algorithm($algorithm); return $this; } - public function aggregationStorage(AggregationStorageStrategy $strategy): self - { - $this->aggregation->strategy($strategy); - - return $this; - } - - public function aggregationStore(AggregationStorage $storage): self + public function aggregationFilesystem(string $protocol): self { - $this->aggregation->storage($storage); + $this->aggregation->filesystemProtocol($protocol); return $this; } diff --git a/src/core/etl/src/Flow/ETL/Function/Average.php b/src/core/etl/src/Flow/ETL/Function/Average.php index 1676783917..d404adea22 100644 --- a/src/core/etl/src/Flow/ETL/Function/Average.php +++ b/src/core/etl/src/Flow/ETL/Function/Average.php @@ -21,7 +21,7 @@ use function is_int; use function is_numeric; -final class Average implements AggregatingFunction, MergeableAggregatingFunction, WindowFunction +final class Average implements AggregatingFunction, WindowFunction { private int $count; @@ -82,16 +82,6 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return (new Calculator())->divide($sum, $count, $this->scale, $this->rounding); } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - $this->sum += $other->sum; - $this->count += $other->count; - } - public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/Function/Collect.php b/src/core/etl/src/Flow/ETL/Function/Collect.php index 78d4ad51be..12209f5710 100644 --- a/src/core/etl/src/Flow/ETL/Function/Collect.php +++ b/src/core/etl/src/Flow/ETL/Function/Collect.php @@ -11,11 +11,10 @@ use Flow\ETL\Row\EntryFactory; use Flow\ETL\Row\Reference; -use function array_merge; use function current; use function Flow\ETL\DSL\to_entry; -final class Collect implements AggregatingFunction, MergeableAggregatingFunction +final class Collect implements AggregatingFunction { /** * @var array @@ -42,15 +41,6 @@ public function aggregate(Row $row, FlowContext $context): void } } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - $this->collection = array_merge($this->collection, $other->collection); - } - /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/CollectUnique.php b/src/core/etl/src/Flow/ETL/Function/CollectUnique.php index fd86c2deaf..3f87b0ff09 100644 --- a/src/core/etl/src/Flow/ETL/Function/CollectUnique.php +++ b/src/core/etl/src/Flow/ETL/Function/CollectUnique.php @@ -15,7 +15,7 @@ use function Flow\ETL\DSL\to_entry; use function in_array; -final class CollectUnique implements AggregatingFunction, MergeableAggregatingFunction +final class CollectUnique implements AggregatingFunction { /** * @var array @@ -49,20 +49,6 @@ public function aggregate(Row $row, FlowContext $context): void } } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - /** @var mixed $value */ - foreach ($other->collection as $value) { - if (!in_array($value, $this->collection, true)) { - $this->collection[] = $value; - } - } - } - /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Count.php b/src/core/etl/src/Flow/ETL/Function/Count.php index 3087659836..52e585ef3c 100644 --- a/src/core/etl/src/Flow/ETL/Function/Count.php +++ b/src/core/etl/src/Flow/ETL/Function/Count.php @@ -16,7 +16,7 @@ use function Flow\ETL\DSL\int_entry; -final class Count implements AggregatingFunction, MergeableAggregatingFunction, WindowFunction +final class Count implements AggregatingFunction, WindowFunction { private int $count; @@ -80,15 +80,6 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return $count; } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - $this->count += $other->count; - } - public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/Function/First.php b/src/core/etl/src/Flow/ETL/Function/First.php index 9e7c39b750..36126f0176 100644 --- a/src/core/etl/src/Flow/ETL/Function/First.php +++ b/src/core/etl/src/Flow/ETL/Function/First.php @@ -13,7 +13,7 @@ use function Flow\ETL\DSL\string_entry; -final class First implements AggregatingFunction, MergeableAggregatingFunction +final class First implements AggregatingFunction { /** * @var null|Entry @@ -37,15 +37,6 @@ public function aggregate(Row $row, FlowContext $context): void } } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - $this->first ??= $other->first; - } - /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Last.php b/src/core/etl/src/Flow/ETL/Function/Last.php index 8df306d96d..d5c1e76f76 100644 --- a/src/core/etl/src/Flow/ETL/Function/Last.php +++ b/src/core/etl/src/Flow/ETL/Function/Last.php @@ -13,7 +13,7 @@ use function Flow\ETL\DSL\string_entry; -final class Last implements AggregatingFunction, MergeableAggregatingFunction +final class Last implements AggregatingFunction { /** * @var null|Entry @@ -35,17 +35,6 @@ public function aggregate(Row $row, FlowContext $context): void } } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - if ($other->last !== null) { - $this->last = $other->last; - } - } - /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Max.php b/src/core/etl/src/Flow/ETL/Function/Max.php index 9f5eda78a7..6794783b8c 100644 --- a/src/core/etl/src/Flow/ETL/Function/Max.php +++ b/src/core/etl/src/Flow/ETL/Function/Max.php @@ -18,7 +18,7 @@ use function is_numeric; use function max; -final class Max implements AggregatingFunction, MergeableAggregatingFunction +final class Max implements AggregatingFunction { private float|DateTimeInterface|null $max; @@ -52,25 +52,6 @@ public function aggregate(Row $row, FlowContext $context): void } } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - if ($other->max === null) { - return; - } - - if ($this->max === null) { - $this->max = $other->max; - - return; - } - - $this->max = max($this->max, $other->max); - } - /** * @return Entry|Entry|Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php b/src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php deleted file mode 100644 index 4065cd7cf7..0000000000 --- a/src/core/etl/src/Flow/ETL/Function/MergeableAggregatingFunction.php +++ /dev/null @@ -1,10 +0,0 @@ -min === null) { - return; - } - - if ($this->min === null) { - $this->min = $other->min; - - return; - } - - $this->min = min($this->min, $other->min); - } - /** * @return Entry|Entry|Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/StringAggregate.php b/src/core/etl/src/Flow/ETL/Function/StringAggregate.php index c68fa25544..13f1e468a3 100644 --- a/src/core/etl/src/Flow/ETL/Function/StringAggregate.php +++ b/src/core/etl/src/Flow/ETL/Function/StringAggregate.php @@ -4,7 +4,6 @@ namespace Flow\ETL\Function; -use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\FlowContext; use Flow\ETL\Row; use Flow\ETL\Row\Entry; @@ -12,7 +11,6 @@ use Flow\ETL\Row\Reference; use Flow\ETL\Row\SortOrder; -use function array_merge; use function count; use function Flow\ETL\DSL\str_entry; use function implode; @@ -20,7 +18,7 @@ use function rsort; use function sort; -final class StringAggregate implements AggregatingFunction, MergeableAggregatingFunction +final class StringAggregate implements AggregatingFunction { /** * @var array @@ -42,15 +40,6 @@ public function aggregate(Row $row, FlowContext $context): void } } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - $this->values = array_merge($this->values, $other->values); - } - /** * @return Row\Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Sum.php b/src/core/etl/src/Flow/ETL/Function/Sum.php index e117a9f302..77415475c3 100644 --- a/src/core/etl/src/Flow/ETL/Function/Sum.php +++ b/src/core/etl/src/Flow/ETL/Function/Sum.php @@ -19,7 +19,7 @@ use function Flow\ETL\DSL\int_entry; use function is_numeric; -final class Sum implements AggregatingFunction, MergeableAggregatingFunction, WindowFunction +final class Sum implements AggregatingFunction, WindowFunction { private float|int $sum; @@ -68,16 +68,6 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return $sum; } - public function merge(MergeableAggregatingFunction $other): void - { - if (!$other instanceof self) { - throw new InvalidArgumentException('Cannot merge ' . self::class . ' with ' . $other::class); - } - - // @mago-ignore analysis:possibly-invalid-argument - $this->sum = (new Calculator())->add($this->sum, $other->sum); - } - public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/GroupBy.php b/src/core/etl/src/Flow/ETL/GroupBy.php index 3329121704..0ce6f8ca6f 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy.php +++ b/src/core/etl/src/Flow/ETL/GroupBy.php @@ -9,16 +9,16 @@ use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\Exception\RuntimeException; use Flow\ETL\Function\AggregatingFunction; -use Flow\ETL\Function\MergeableAggregatingFunction; -use Flow\ETL\GroupBy\Storage\AggregationStorage; -use Flow\ETL\GroupBy\Storage\MemoryAggregationStorage; use Flow\ETL\Hash\NativePHPHash; +use Flow\ETL\Row\EntryFactory; use Flow\ETL\Row\Reference; use Flow\ETL\Row\References; +use Generator; use Stringable; use function array_filter; use function array_key_exists; +use function array_map; use function array_unique; use function array_values; use function count; @@ -32,37 +32,29 @@ use function is_scalar; use function serialize; +/** + * Specification of a group-by: the key columns and the aggregations to run. + * + * The actual accumulation of non-pivot groups is done by the aggregating algorithms + * (see {@see \Flow\ETL\GroupBy\AggregatingAlgorithm}); this class only exposes the spec + * and small helpers they need. Pivot is a memory-only cross-tab and is executed here. + */ final class GroupBy { + private const int RESULT_BATCH_SIZE = 1000; + /** * @var array */ - private array $aggregations; + private array $aggregations = []; - private ?Reference $pivot; - - /** - * @var array|bool|float|int|object|string> - */ - private array $pivotColumns; - - /** - * @var array|bool|float|int|object|string>> - */ - private array $pivotedTable; + private ?Reference $pivot = null; private readonly References $refs; - private AggregationStorage $storage; - public function __construct(string|Reference ...$entries) { $this->refs = References::init(...array_unique($entries)); - $this->aggregations = []; - $this->pivotedTable = []; - $this->pivotColumns = []; - $this->pivot = null; - $this->storage = new MemoryAggregationStorage(); } public function aggregate(AggregatingFunction ...$aggregator): void @@ -80,33 +72,142 @@ public function aggregate(AggregatingFunction ...$aggregator): void $this->aggregations = $aggregator; } - public function allAggregationsMergeable(): bool + /** + * Build the output row for one finalized group. + * + * @param array $keyValues + * @param array $aggregators + */ + public function aggregatedRow(array $keyValues, array $aggregators, EntryFactory $entryFactory): Row + { + $entries = []; + + /** @var mixed $value */ + foreach ($keyValues as $name => $value) { + $entries[] = $entryFactory->create($name, $value); + } + + foreach ($aggregators as $aggregator) { + $entries[] = $aggregator->result($entryFactory); + } + + return Row::create(...$entries); + } + + /** + * @return array + */ + public function aggregations(): array + { + return $this->aggregations; + } + + /** + * A stable string key for the given group-by values (used to bucket equal groups). + * + * @param array $values + */ + public function hash(array $values): string + { + /** @var array $stringValues */ + $stringValues = []; + + /** @var mixed $value */ + foreach ($values as $value) { + if ($value === null) { + $stringValues[] = 'null'; + } elseif (is_scalar($value)) { + $stringValues[] = (string) $value; + } else { + if ($value instanceof Stringable) { + $stringValues[] = $value->__toString(); + } elseif ($value instanceof DateTimeInterface) { + $stringValues[] = $value->format(DateTimeImmutable::ATOM); + } elseif (is_array($value)) { + $stringValues[] = $this->hash($value); + } else { + $stringValues[] = serialize($value); + } + } + } + + return NativePHPHash::xxh128(implode('', $stringValues)); + } + + public function isPivot(): bool { - foreach ($this->aggregations as $aggregation) { - if (!$aggregation instanceof MergeableAggregatingFunction) { - return false; + return $this->pivot !== null; + } + + /** + * The group-by key values for a single row (missing columns become null). + * + * @return array + */ + public function keyValues(Row $row): array + { + $values = []; + + foreach ($this->refs as $ref) { + try { + $values[$ref->name()] = $row->valueOf($ref); + } catch (InvalidArgumentException) { + $values[$ref->name()] = null; } } - return true; + return $values; + } + + /** + * A fresh set of aggregator instances for a new group. + * + * @return array + */ + public function newAggregators(): array + { + return array_map( + static fn(AggregatingFunction $aggregation): AggregatingFunction => clone $aggregation, + $this->aggregations, + ); + } + + public function pivot(Reference $ref): void + { + $this->pivot = $ref; } - public function group(Rows $rows, FlowContext $context): void + /** + * Execute the pivot cross-tab. Pivot is memory-only (it must know every distinct pivot + * value to build the columns), so it always aggregates in memory and streams the result. + * + * @param Generator $rows + * + * @return Generator + */ + public function pivotResult(Generator $rows, FlowContext $context): Generator { $pivot = $this->pivot; - if ($pivot !== null) { - foreach ($rows as $row) { + if ($pivot === null) { + throw new RuntimeException('pivotResult() called without a pivot reference'); + } + + /** @var array|bool|float|int|object|string> $pivotColumns */ + $pivotColumns = []; + /** @var array|bool|float|int|object|string>> $pivotedTable */ + $pivotedTable = []; + + foreach ($rows as $batch) { + foreach ($batch as $row) { try { - $this->pivotColumns[] = $row->valueOf($pivot); + $pivotColumns[] = $row->valueOf($pivot); } catch (InvalidArgumentException) { - $this->pivotColumns[] = null; + $pivotColumns[] = null; } } - $this->pivotColumns = array_values(array_filter(array_unique($this->pivotColumns))); - - foreach ($rows as $row) { + foreach ($batch as $row) { $values = []; foreach ($this->refs as $ref) { @@ -114,15 +215,14 @@ public function group(Rows $rows, FlowContext $context): void } $indexValue = $this->hash($values); - $pivotValue = $row->valueOf($pivot); - if (!array_key_exists($indexValue, $this->pivotedTable)) { - $this->pivotedTable[$indexValue] = []; + if (!array_key_exists($indexValue, $pivotedTable)) { + $pivotedTable[$indexValue] = []; } foreach ($this->refs as $ref) { - $this->pivotedTable[$indexValue][$ref->name()] = $row->valueOf($ref); + $pivotedTable[$indexValue][$ref->name()] = $row->valueOf($ref); } if ($pivotValue === null) { @@ -131,135 +231,55 @@ public function group(Rows $rows, FlowContext $context): void $pivotValue = type_union(type_string(), type_integer())->assert($pivotValue); - if (!array_key_exists($pivotValue, $this->pivotedTable[$indexValue])) { + if (!array_key_exists($pivotValue, $pivotedTable[$indexValue])) { // @mago-ignore analysis:invalid-property-assignment-value,possibly-invalid-clone - $this->pivotedTable[$indexValue][$pivotValue] = clone current($this->aggregations); + $pivotedTable[$indexValue][$pivotValue] = clone current($this->aggregations); } - $aggregator = $this->pivotedTable[$indexValue][$pivotValue]; + $aggregator = $pivotedTable[$indexValue][$pivotValue]; if ($aggregator instanceof AggregatingFunction) { $aggregator->aggregate($row, $context); } } - } else { - foreach ($rows as $row) { - /** @var array $values */ - $values = []; - - foreach ($this->refs as $ref) { - try { - $values[$ref->name()] = $row->valueOf($ref); - } catch (InvalidArgumentException) { - $values[$ref->name()] = null; - } - } - - $valuesHash = $this->hash($values); - - $entry = $this->storage->entry($valuesHash, $values, $this->aggregations); - - foreach ($entry->aggregators as $aggregator) { - $aggregator->aggregate($row, $context); - } - - $this->storage->save($valuesHash, $entry); - } } - } - public function isPivot(): bool - { - return $this->pivot !== null; - } + $pivotColumns = array_values(array_filter(array_unique($pivotColumns))); - public function pivot(Reference $ref): void - { - $this->pivot = $ref; - } - - public function result(FlowContext $context): Rows - { - $rows = []; - - if ($this->pivot) { - foreach ($this->pivotedTable as $index => $columns) { - $row = [$this->refs->first()->name() => $index]; + $buffer = []; - foreach ($columns as $rowIndex => $values) { - $row[$rowIndex] = $values instanceof AggregatingFunction - ? $values->result($context->entryFactory())->value() - : $values; - } - - foreach ($this->pivotColumns as $column) { - $column = type_union(type_string(), type_integer())->assert($column); - - if (!array_key_exists($column, $row)) { - $row[$column] = null; - } - } + foreach ($pivotedTable as $index => $columns) { + $row = [$this->refs->first()->name() => $index]; - $rows[] = $row; + foreach ($columns as $rowIndex => $value) { + $row[$rowIndex] = $value instanceof AggregatingFunction + ? $value->result($context->entryFactory())->value() + : $value; } - return array_to_rows($rows, $context->entryFactory()); - } - - $this->storage->flush(); - - foreach ($this->storage->all() as $group) { - $entries = []; + foreach ($pivotColumns as $column) { + $column = type_union(type_string(), type_integer())->assert($column); - /** @var mixed $value */ - foreach ($group->values as $entry => $value) { - $entries[] = $context->entryFactory()->create($entry, $value); + if (!array_key_exists($column, $row)) { + $row[$column] = null; + } } - foreach ($group->aggregators as $aggregator) { - $entries[] = $aggregator->result($context->entryFactory()); - } + $buffer[] = $row; - if (count($entries)) { - $rows[] = Row::create(...$entries); + if (count($buffer) >= self::RESULT_BATCH_SIZE) { + yield array_to_rows($buffer, $context->entryFactory()); + $buffer = []; } } - return new Rows(...$rows); - } - - public function useStorage(AggregationStorage $storage): void - { - $this->storage = $storage; + if ($buffer !== []) { + yield array_to_rows($buffer, $context->entryFactory()); + } } - /** - * @param array $values - */ - private function hash(array $values): string + public function references(): References { - /** @var array $stringValues */ - $stringValues = []; - - /** @var mixed $value */ - foreach ($values as $value) { - if ($value === null) { - $stringValues[] = 'null'; - } elseif (is_scalar($value)) { - $stringValues[] = (string) $value; - } else { - if ($value instanceof Stringable) { - $stringValues[] = $value->__toString(); - } elseif ($value instanceof DateTimeInterface) { - $stringValues[] = $value->format(DateTimeImmutable::ATOM); - } elseif (is_array($value)) { - $stringValues[] = $this->hash($value); - } else { - $stringValues[] = serialize($value); - } - } - } - - return NativePHPHash::xxh128(implode('', $stringValues)); + return $this->refs; } } diff --git a/src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php b/src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php new file mode 100644 index 0000000000..8e4f54e301 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php @@ -0,0 +1,25 @@ + $rows + * + * @return Generator + */ + public function aggregate(Generator $rows, FlowContext $context, GroupBy $groupBy): Generator; +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php b/src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php new file mode 100644 index 0000000000..5ef1902306 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php @@ -0,0 +1,25 @@ +references()->all(); + $sortRefs[] = ref(self::INPUT_ORDINAL); + + $sorted = $this->sort->sortGenerator($this->withInputOrdinal($rows), $context, References::init(...$sortRefs)); + + $currentHash = null; + /** @var array $currentKey */ + $currentKey = []; + /** @var array $aggregators */ + $aggregators = []; + $buffer = []; + + foreach ($sorted as $batch) { + foreach ($batch as $row) { + $keyValues = $groupBy->keyValues($row); + $hash = $groupBy->hash($keyValues); + + if ($hash !== $currentHash) { + if ($currentHash !== null) { + $buffer[] = $groupBy->aggregatedRow($currentKey, $aggregators, $context->entryFactory()); + + if (count($buffer) >= self::BATCH_SIZE) { + yield new Rows(...$buffer); + $buffer = []; + } + } + + $currentHash = $hash; + $currentKey = $keyValues; + $aggregators = $groupBy->newAggregators(); + } + + foreach ($aggregators as $aggregator) { + $aggregator->aggregate($row, $context); + } + } + } + + if ($currentHash !== null) { + $buffer[] = $groupBy->aggregatedRow($currentKey, $aggregators, $context->entryFactory()); + } + + if ($buffer !== []) { + yield new Rows(...$buffer); + } + } + + /** + * @param Generator $rows + * + * @return Generator + */ + private function withInputOrdinal(Generator $rows): Generator + { + $ordinal = 0; + + foreach ($rows as $batch) { + $tagged = []; + + foreach ($batch as $row) { + $tagged[] = $row->add(int_entry(self::INPUT_ORDINAL, $ordinal)); + $ordinal++; + } + + yield new Rows(...$tagged); + } + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php b/src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php new file mode 100644 index 0000000000..daac717d81 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php @@ -0,0 +1,62 @@ +, aggregators: array}> $groups */ + $groups = []; + + foreach ($rows as $batch) { + foreach ($batch as $row) { + $keyValues = $groupBy->keyValues($row); + $hash = $groupBy->hash($keyValues); + + if (!array_key_exists($hash, $groups)) { + $groups[$hash] = ['keyValues' => $keyValues, 'aggregators' => $groupBy->newAggregators()]; + } + + foreach ($groups[$hash]['aggregators'] as $aggregator) { + $aggregator->aggregate($row, $context); + } + } + } + + $buffer = []; + + foreach ($groups as $group) { + $buffer[] = $groupBy->aggregatedRow($group['keyValues'], $group['aggregators'], $context->entryFactory()); + + if (count($buffer) >= self::BATCH_SIZE) { + yield new Rows(...$buffer); + $buffer = []; + } + } + + if ($buffer !== []) { + yield new Rows(...$buffer); + } + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/AggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/AggregationStorage.php deleted file mode 100644 index 2121450c3c..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/AggregationStorage.php +++ /dev/null @@ -1,29 +0,0 @@ - - */ - public function all(): Generator; - - /** - * @param array $values - * @param array $prototypes - */ - public function entry(string $hash, array $values, array $prototypes): GroupEntry; - - public function flush(): void; - - public function save(string $hash, GroupEntry $entry): void; -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php deleted file mode 100644 index 425ed8633f..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/BucketsCache/FilesystemGroupBucketsCache.php +++ /dev/null @@ -1,58 +0,0 @@ -cacheDir = ($cacheDir ?? $this->filesystem->getSystemTmpDir())->suffix('/flow-php-group-by/'); - } - - public function get(string $bucketId): array - { - $path = $this->keyPath($bucketId); - - if (!$this->filesystem->status($path)) { - return []; - } - - $stream = $this->filesystem->readFrom($path); - $content = $stream->content(); - $stream->close(); - - return $this->serializer->unserialize($content, [GroupBucket::class])->entries; - } - - public function remove(string $bucketId): void - { - $this->filesystem->rm($this->keyPath($bucketId)->parentDirectory()); - } - - public function set(string $bucketId, array $entries): void - { - $stream = $this->filesystem->writeTo($this->keyPath($bucketId)); - $stream->append($this->serializer->serialize(new GroupBucket($entries))); - $stream->close(); - } - - private function keyPath(string $key): Path - { - return $this->cacheDir->suffix(NativePHPHash::xxh128($key) . '/' . $key . '.php.cache'); - } -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php deleted file mode 100644 index 7885259db3..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/ExternalAggregationStorage.php +++ /dev/null @@ -1,121 +0,0 @@ - - */ - private array $hot = []; - - /** - * @var list - */ - private array $spilledBuckets = []; - - public function __construct( - private readonly GroupBucketsCache $cache, - private readonly Unit $memoryLimit, - ) { - $this->consumption = new Consumption(false); - } - - /** - * @return Generator - */ - public function all(): Generator - { - if ($this->spilledBuckets === []) { - foreach ($this->hot as $entry) { - yield $entry; - } - - return; - } - - /** @var array $merged */ - $merged = []; - - foreach ($this->spilledBuckets as $bucketId) { - foreach ($this->cache->get($bucketId) as $hash => $entry) { - if (!array_key_exists($hash, $merged)) { - $merged[$hash] = $entry; - - continue; - } - - foreach ($merged[$hash]->aggregators as $index => $aggregator) { - $partial = $entry->aggregators[$index]; - - if ( - $aggregator instanceof MergeableAggregatingFunction - && $partial instanceof MergeableAggregatingFunction - ) { - $aggregator->merge($partial); - } - } - } - - $this->cache->remove($bucketId); - } - - foreach ($merged as $entry) { - yield $entry; - } - } - - public function entry(string $hash, array $values, array $prototypes): GroupEntry - { - if (!array_key_exists($hash, $this->hot)) { - $aggregators = []; - - foreach ($prototypes as $prototype) { - $aggregators[] = clone $prototype; - } - - $this->hot[$hash] = new GroupEntry($values, $aggregators); - } - - return $this->hot[$hash]; - } - - public function flush(): void - { - if ($this->spilledBuckets !== [] && $this->hot !== []) { - $this->spill(); - } - } - - public function save(string $hash, GroupEntry $entry): void - { - if ($this->consumption->currentDiff()->isGreaterThan($this->memoryLimit)) { - $this->spill(); - } - } - - private function spill(): void - { - $bucketId = bin2hex(random_bytes(16)); - $this->cache->set($bucketId, $this->hot); - $this->spilledBuckets[] = $bucketId; - $this->hot = []; - $this->consumption = new Consumption(false); - } -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php deleted file mode 100644 index 38860554e8..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucket.php +++ /dev/null @@ -1,15 +0,0 @@ - $entries - */ - public function __construct( - public array $entries, - ) {} -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php deleted file mode 100644 index 025e523968..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupBucketsCache.php +++ /dev/null @@ -1,23 +0,0 @@ - - */ - public function get(string $bucketId): array; - - public function remove(string $bucketId): void; - - /** - * @param array $entries - */ - public function set(string $bucketId, array $entries): void; -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php deleted file mode 100644 index 40f90004cf..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/GroupEntry.php +++ /dev/null @@ -1,19 +0,0 @@ - $values - * @param array $aggregators - */ - public function __construct( - public array $values, - public array $aggregators, - ) {} -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php deleted file mode 100644 index d15fa2c2f2..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/KvAggregationStorage.php +++ /dev/null @@ -1,109 +0,0 @@ - - */ - private array $hashes = []; - - public function __construct( - private readonly CacheInterface $cache, - private readonly Serializer $serializer = new NativePHPSerializer(), - private readonly string $namespace = 'flow_group_by_', - ) {} - - /** - * @return Generator - */ - public function all(): Generator - { - foreach (array_keys($this->hashes) as $hash) { - $entry = $this->read($hash); - - if ($entry === null) { - throw $this->evicted($hash); - } - - yield $entry; - - $this->cache->delete($this->namespace . $hash); - } - } - - public function entry(string $hash, array $values, array $prototypes): GroupEntry - { - $existing = $this->read($hash); - - if ($existing !== null) { - return $existing; - } - - if (isset($this->hashes[$hash])) { - throw $this->evicted($hash); - } - - $aggregators = []; - - foreach ($prototypes as $prototype) { - $aggregators[] = clone $prototype; - } - - $this->hashes[$hash] = true; - - return new GroupEntry($values, $aggregators); - } - - public function flush(): void {} - - public function save(string $hash, GroupEntry $entry): void - { - $this->hashes[$hash] = true; - - if (!$this->cache->set($this->namespace . $hash, $this->serializer->serialize($entry))) { - throw new RuntimeException( - 'Failed to write aggregation group to the KV store (store full?). Increase its capacity ' - . '(e.g. apc.shm_size, Redis maxmemory) or use AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL.', - ); - } - } - - private function evicted(string $hash): RuntimeException - { - return new RuntimeException( - 'Aggregation group "' - . $hash - . '" was evicted from the KV store mid-aggregation; the result ' - . 'would be incorrect. Increase the store capacity (e.g. apc.shm_size, Redis maxmemory) or use ' - . 'AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL.', - ); - } - - private function read(string $hash): ?GroupEntry - { - // @mago-ignore analysis:mixed-assignment - $serialized = $this->cache->get($this->namespace . $hash); - - if (!is_string($serialized)) { - return null; - } - - return $this->serializer->unserialize($serialized, [GroupEntry::class]); - } -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php b/src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php deleted file mode 100644 index d87dc6f2af..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/Storage/MemoryAggregationStorage.php +++ /dev/null @@ -1,49 +0,0 @@ - - */ - private array $groups = []; - - /** - * @return Generator - */ - public function all(): Generator - { - foreach ($this->groups as $group) { - yield $group; - } - } - - public function entry(string $hash, array $values, array $prototypes): GroupEntry - { - if (!array_key_exists($hash, $this->groups)) { - $aggregators = []; - - foreach ($prototypes as $prototype) { - $aggregators[] = clone $prototype; - } - - $this->groups[$hash] = new GroupEntry($values, $aggregators); - } - - return $this->groups[$hash]; - } - - public function flush(): void {} - - public function save(string $hash, GroupEntry $entry): void {} -} diff --git a/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php b/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php index bc0ed3433b..53e01179cb 100644 --- a/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php +++ b/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php @@ -6,16 +6,21 @@ use Flow\ETL\FlowContext; use Flow\ETL\GroupBy; -use Flow\ETL\GroupBy\Storage\AggregationStorage; -use Flow\ETL\GroupBy\Storage\BucketsCache\FilesystemGroupBucketsCache; -use Flow\ETL\GroupBy\Storage\ExternalAggregationStorage; -use Flow\ETL\GroupBy\Storage\MemoryAggregationStorage; +use Flow\ETL\GroupBy\AggregatingAlgorithm; +use Flow\ETL\GroupBy\ExternalAggregation; +use Flow\ETL\GroupBy\MemoryAggregation; use Flow\ETL\Processor; +use Flow\ETL\Sort\ExternalSort; +use Flow\ETL\Sort\ExternalSort\BucketsCache\FilesystemBucketsCache; use Generator; /** * Groups all rows and applies aggregation functions. * + * Picks the aggregation algorithm from configuration (mirroring SortingProcessor): in-memory + * by default, or an external sort-based fold for bounded memory. Pivot and global aggregation + * (no group-by columns) always run in memory. + * * @internal */ final readonly class GroupByProcessor implements Processor @@ -26,38 +31,33 @@ public function __construct( public function process(Generator $rows, FlowContext $context): Generator { - $this->groupBy->useStorage($this->createStorage($context)); + if ($this->groupBy->isPivot()) { + yield from $this->groupBy->pivotResult($rows, $context); - foreach ($rows as $batch) { - $this->groupBy->group($batch, $context); + return; } - yield $this->groupBy->result($context); + yield from $this->algorithm($context)->aggregate($rows, $context, $this->groupBy); } - private function createStorage(FlowContext $context): AggregationStorage + private function algorithm(FlowContext $context): AggregatingAlgorithm { $config = $context->config->aggregation; - if ($config->storage !== null) { - return $config->storage; - } - - if ( - $config->strategy->useMemory() - || $this->groupBy->isPivot() - || !$this->groupBy->allAggregationsMergeable() - ) { - return new MemoryAggregationStorage(); + if ($config->algorithm->useMemory() || $this->groupBy->references()->count() === 0) { + return new MemoryAggregation(); } - return new ExternalAggregationStorage( - new FilesystemGroupBucketsCache( - $context->filesystem($config->filesystemProtocol), - $context->config->serializer(), - $context->config->cache->localFilesystemCacheDir->suffix('/flow-php-group-by/'), + return new ExternalAggregation( + new ExternalSort( + new FilesystemBucketsCache( + $context->filesystem($config->filesystemProtocol), + $context->config->serializer(), + 100, + $context->config->cache->localFilesystemCacheDir->suffix('/flow-php-group-by/'), + ), + $context->config->cache->externalSortBucketsCount, ), - $config->memoryLimit, ); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php similarity index 55% rename from src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php rename to src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php index 3a19f8f7a1..0dfbbbec41 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByStorageTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php @@ -5,24 +5,22 @@ namespace Flow\ETL\Tests\Integration\DataFrame; use Flow\ETL\Config\ConfigBuilder; -use Flow\ETL\Dataset\Memory\Unit; -use Flow\ETL\GroupBy\AggregationStorageStrategy; -use Flow\ETL\GroupBy\Storage\KvAggregationStorage; +use Flow\ETL\GroupBy\AggregationAlgorithms; use Flow\ETL\Tests\Double\FakeRandomOrdersExtractor; use Flow\ETL\Tests\FlowIntegrationTestCase; -use Symfony\Component\Cache\Adapter\ApcuAdapter; -use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Psr16Cache; use function Flow\ETL\DSL\collect; use function Flow\ETL\DSL\config_builder; use function Flow\ETL\DSL\count; use function Flow\ETL\DSL\df; +use function Flow\ETL\DSL\first; use function Flow\ETL\DSL\from_array; +use function Flow\ETL\DSL\last; +use function Flow\ETL\DSL\ref; use function Flow\ETL\DSL\sum; use function is_numeric; -final class GroupByStorageTest extends FlowIntegrationTestCase +final class GroupByAggregationTest extends FlowIntegrationTestCase { private const int ORDERS = 20_000; @@ -31,56 +29,51 @@ final class GroupByStorageTest extends FlowIntegrationTestCase */ private static ?array $orders = null; - public function test_apcu_kv_storage_produces_identical_results(): void + public function test_first_and_last_preserve_input_order_under_external_spill(): void { - if (!ApcuAdapter::isSupported()) { - self::markTestSkipped( - 'APCu (with apc.enable_cli=1) is required to evaluate APCu as an aggregation storage backend.', - ); + // 2000 rows interleaved into two groups, each spanning multiple spilled sort runs; + // v is the input position so first/last are unambiguous in input order. + $data = []; + + for ($i = 0; $i < 2000; $i++) { + $data[] = ['g' => ($i % 2) === 0 ? 'a' : 'b', 'v' => $i]; } - $cache = new Psr16Cache(new ApcuAdapter('flow_group_by_test')); - $cache->clear(); + $run = static fn(ConfigBuilder $config): array => df($config) + ->read(from_array($data)) + ->groupBy('g') + ->aggregate(first(ref('v')), last(ref('v'))) + ->sortBy(ref('g')) + ->fetch() + ->toArray(); - $expected = $this->aggregateBySeller(config_builder()); - $actual = $this->aggregateBySeller(config_builder()->aggregationStore(new KvAggregationStorage($cache))); + $memory = $run(config_builder()); + $external = $run(config_builder()->aggregationAlgorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION)); - self::assertEqualsCanonicalizing($expected, $actual); + self::assertSame($memory, $external); + self::assertSame( + [ + ['g' => 'a', 'v_first' => 0, 'v_last' => 1998], + ['g' => 'b', 'v_first' => 1, 'v_last' => 1999], + ], + $external, + ); } - public function test_high_cardinality_group_by_spills_and_stays_correct(): void + public function test_high_cardinality_group_by_is_identical_between_memory_and_external(): void { $memory = $this->aggregateByEmail(config_builder()); - $external = $this->aggregateByEmail( - config_builder() - ->aggregationStorage(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL) - ->aggregationMemoryLimit(Unit::fromKb(256)), - ); + $external = $this->aggregateByEmail(config_builder()->aggregationAlgorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION)); self::assertEqualsCanonicalizing($memory, $external); } - public function test_kv_storage_produces_identical_results(): void - { - $expected = $this->aggregateBySeller(config_builder()); - - $actual = $this->aggregateBySeller(config_builder()->aggregationStore(new KvAggregationStorage(new Psr16Cache( - new ArrayAdapter(), - )))); - - self::assertEqualsCanonicalizing($expected, $actual); - } - - public function test_seller_aggregation_is_identical_between_memory_and_external_spill_storage(): void + public function test_seller_aggregation_is_identical_between_memory_and_external(): void { $memory = $this->aggregateBySeller(config_builder()); - $external = $this->aggregateBySeller( - config_builder() - ->aggregationStorage(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL) - ->aggregationMemoryLimit(Unit::fromKb(256)), - ); + $external = $this->aggregateBySeller(config_builder()->aggregationAlgorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION)); self::assertEqualsCanonicalizing($memory, $external); self::assertCount(5, $memory); diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php deleted file mode 100644 index aea7423f04..0000000000 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/Storage/ExternalAggregationStorageTest.php +++ /dev/null @@ -1,67 +0,0 @@ -cacheDir->suffix('/external-aggregation-storage/'); - $this->fs()->rm($cacheDir); - - $context = flow_context(); - $storage = new ExternalAggregationStorage( - new FilesystemGroupBucketsCache($this->fs(), $this->serializer(), $cacheDir), - Unit::fromBytes(0), - ); - - $input = [ - ['type' => 'a', 'v' => 1], - ['type' => 'b', 'v' => 2], - ['type' => 'a', 'v' => 3], - ['type' => 'a', 'v' => 4], - ['type' => 'b', 'v' => 5], - ]; - - foreach ($input as $record) { - $entry = $storage->entry($record['type'], ['type' => $record['type']], [sum(ref('v'))]); - - foreach ($entry->aggregators as $aggregator) { - $aggregator->aggregate(row(str_entry('type', $record['type']), int_entry('v', $record['v'])), $context); - } - - $storage->save($record['type'], $entry); - } - - self::assertNotNull($this->fs()->status($cacheDir)); - - $results = []; - - foreach ($storage->all() as $group) { - /** @var int $sum */ - $sum = $group->aggregators[0]->result($context->entryFactory())->value(); - $results[(string) $group->values['type']] = $sum; - } - - ksort($results); - - self::assertSame(['a' => 8, 'b' => 7], $results); - - $this->fs()->rm($cacheDir); - } -} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php index 112e62b03e..2763f800e0 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php @@ -4,68 +4,30 @@ namespace Flow\ETL\Tests\Unit\Config\Aggregation; -use Flow\ETL\Config\Aggregation\AggregationConfig; use Flow\ETL\Config\Aggregation\AggregationConfigBuilder; -use Flow\ETL\Dataset\Memory\Unit; -use Flow\ETL\GroupBy\AggregationStorageStrategy; -use Flow\ETL\GroupBy\Storage\KvAggregationStorage; +use Flow\ETL\GroupBy\AggregationAlgorithms; use Flow\ETL\Tests\FlowTestCase; -use Symfony\Component\Cache\Adapter\ArrayAdapter; -use Symfony\Component\Cache\Psr16Cache; - -use function putenv; final class AggregationConfigBuilderTest extends FlowTestCase { - public function test_custom_memory_limit_overrides_default(): void - { - $config = (new AggregationConfigBuilder()) - ->memoryLimit(Unit::fromMb(16)) - ->build(); - - self::assertSame(Unit::fromMb(16)->inBytes(), $config->memoryLimit->inBytes()); - } - - public function test_default_strategy_is_memory(): void + public function test_default_algorithm_is_memory(): void { $config = (new AggregationConfigBuilder())->build(); - self::assertSame(AggregationStorageStrategy::MEMORY, $config->strategy); - self::assertNull($config->storage); - self::assertGreaterThan(0, $config->memoryLimit->inBytes()); - } - - public function test_injecting_a_custom_storage(): void - { - $storage = new KvAggregationStorage(new Psr16Cache(new ArrayAdapter())); - - $config = (new AggregationConfigBuilder()) - ->storage($storage) - ->build(); - - self::assertSame($storage, $config->storage); - } - - public function test_memory_limit_is_read_from_environment_variable(): void - { - putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV . '=10M'); - - try { - $config = (new AggregationConfigBuilder())->build(); - - self::assertSame(Unit::fromMb(10)->inBytes(), $config->memoryLimit->inBytes()); - } finally { - putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); - } + self::assertSame(AggregationAlgorithms::MEMORY_AGGREGATION, $config->algorithm); + self::assertTrue($config->algorithm->useMemory()); + self::assertSame('file', $config->filesystemProtocol); } - public function test_selecting_the_external_strategy(): void + public function test_selecting_the_external_algorithm(): void { $config = (new AggregationConfigBuilder()) - ->strategy(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL) + ->algorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION) + ->filesystemProtocol('memory') ->build(); - self::assertSame(AggregationStorageStrategy::MEMORY_FALLBACK_EXTERNAL, $config->strategy); - self::assertFalse($config->strategy->useMemory()); + self::assertSame(AggregationAlgorithms::EXTERNAL_AGGREGATION, $config->algorithm); + self::assertFalse($config->algorithm->useMemory()); + self::assertSame('memory', $config->filesystemProtocol); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php deleted file mode 100644 index 8f6d059be3..0000000000 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php +++ /dev/null @@ -1,194 +0,0 @@ -aggregate(row(int_entry('v', 1)), $context); - $left->aggregate(row(int_entry('v', 2)), $context); - - $right = average(ref('v')); - $right->aggregate(row(int_entry('v', 3)), $context); - - $left->merge($right); - - self::assertEquals(2, $left->result($context->entryFactory())->value()); - } - - public function test_collect_merge_appends_values(): void - { - $context = flow_context(); - - $left = collect(ref('v')); - $left->aggregate(row(int_entry('v', 1)), $context); - $left->aggregate(row(int_entry('v', 2)), $context); - - $right = collect(ref('v')); - $right->aggregate(row(int_entry('v', 3)), $context); - - $left->merge($right); - - self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); - } - - public function test_collect_unique_merge_unions_values(): void - { - $context = flow_context(); - - $left = collect_unique(ref('v')); - $left->aggregate(row(int_entry('v', 1)), $context); - $left->aggregate(row(int_entry('v', 2)), $context); - - $right = collect_unique(ref('v')); - $right->aggregate(row(int_entry('v', 2)), $context); - $right->aggregate(row(int_entry('v', 3)), $context); - - $left->merge($right); - - self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); - } - - public function test_count_merge_adds_counts(): void - { - $context = flow_context(); - - $left = count(); - $left->aggregate(row(int_entry('v', 1)), $context); - $left->aggregate(row(int_entry('v', 2)), $context); - - $right = count(); - $right->aggregate(row(int_entry('v', 3)), $context); - - $left->merge($right); - - self::assertSame(3, $left->result($context->entryFactory())->value()); - } - - public function test_first_merge_keeps_earlier_value(): void - { - $context = flow_context(); - - $left = first(ref('v')); - $left->aggregate(row(int_entry('v', 1)), $context); - - $right = first(ref('v')); - $right->aggregate(row(int_entry('v', 9)), $context); - - $left->merge($right); - - self::assertSame(1, $left->result($context->entryFactory())->value()); - } - - public function test_last_merge_keeps_later_value(): void - { - $context = flow_context(); - - $left = last(ref('v')); - $left->aggregate(row(int_entry('v', 1)), $context); - - $right = last(ref('v')); - $right->aggregate(row(int_entry('v', 9)), $context); - - $left->merge($right); - - self::assertSame(9, $left->result($context->entryFactory())->value()); - } - - public function test_max_merge_keeps_greatest(): void - { - $context = flow_context(); - - $left = max(ref('v')); - $left->aggregate(row(int_entry('v', 5)), $context); - $left->aggregate(row(int_entry('v', 3)), $context); - - $right = max(ref('v')); - $right->aggregate(row(int_entry('v', 9)), $context); - - $left->merge($right); - - self::assertSame(9, $left->result($context->entryFactory())->value()); - } - - public function test_merge_with_different_function_type_throws(): void - { - $this->expectException(InvalidArgumentException::class); - - sum(ref('v'))->merge(count()); - } - - public function test_min_merge_keeps_smallest(): void - { - $context = flow_context(); - - $left = min(ref('v')); - $left->aggregate(row(int_entry('v', 5)), $context); - $left->aggregate(row(int_entry('v', 3)), $context); - - $right = min(ref('v')); - $right->aggregate(row(int_entry('v', 1)), $context); - - $left->merge($right); - - self::assertSame(1, $left->result($context->entryFactory())->value()); - } - - public function test_string_aggregate_merge_concatenates_values(): void - { - $context = flow_context(); - - $left = new StringAggregate(ref('v'), ',', SortOrder::ASC); - $left->aggregate(row(str_entry('v', 'b')), $context); - $left->aggregate(row(str_entry('v', 'a')), $context); - - $right = new StringAggregate(ref('v'), ',', SortOrder::ASC); - $right->aggregate(row(str_entry('v', 'c')), $context); - - $left->merge($right); - - self::assertSame('a,b,c', $left->result($context->entryFactory())->value()); - } - - public function test_sum_merge_adds_partial_sums(): void - { - $context = flow_context(); - - $left = sum(ref('v')); - $left->aggregate(row(int_entry('v', 1)), $context); - $left->aggregate(row(int_entry('v', 2)), $context); - - $right = sum(ref('v')); - $right->aggregate(row(int_entry('v', 3)), $context); - - $left->merge($right); - - self::assertSame(6, $left->result($context->entryFactory())->value()); - } -} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php deleted file mode 100644 index 4621ff2f9f..0000000000 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/Storage/KvAggregationStorageTest.php +++ /dev/null @@ -1,93 +0,0 @@ -entry($group, ['type' => $group], [sum(ref('v'))]); - $entry->aggregators[0]->aggregate(row(str_entry('type', $group), int_entry('v', 1)), $context); - $storage->save($group, $entry); - } - - $cache->delete('flow_group_by_a'); - - $this->expectException(RuntimeException::class); - - iterator_to_array($storage->all()); - } - - public function test_detects_eviction_on_subsequent_access(): void - { - $cache = new Psr16Cache(new ArrayAdapter()); - $storage = new KvAggregationStorage($cache); - $context = flow_context(); - - $entry = $storage->entry('a', ['type' => 'a'], [sum(ref('v'))]); - $entry->aggregators[0]->aggregate(row(str_entry('type', 'a'), int_entry('v', 1)), $context); - $storage->save('a', $entry); - - $cache->delete('flow_group_by_a'); - - $this->expectException(RuntimeException::class); - - $storage->entry('a', ['type' => 'a'], [sum(ref('v'))]); - } - - public function test_round_trip_produces_correct_aggregates(): void - { - $cache = new Psr16Cache(new ArrayAdapter()); - $storage = new KvAggregationStorage($cache); - $context = flow_context(); - - $input = [ - ['type' => 'a', 'v' => 1], - ['type' => 'b', 'v' => 2], - ['type' => 'a', 'v' => 3], - ]; - - foreach ($input as $record) { - $entry = $storage->entry($record['type'], ['type' => $record['type']], [sum(ref('v'))]); - $entry->aggregators[0]->aggregate( - row(str_entry('type', $record['type']), int_entry('v', $record['v'])), - $context, - ); - $storage->save($record['type'], $entry); - } - - $results = []; - - foreach ($storage->all() as $group) { - /** @var int $value */ - $value = $group->aggregators[0]->result($context->entryFactory())->value(); - $results[(string) $group->values['type']] = $value; - } - - ksort($results); - - self::assertSame(['a' => 4, 'b' => 2], $results); - } -} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupByTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupByTest.php index 2d97fc5e58..89926efe6f 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupByTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupByTest.php @@ -6,6 +6,8 @@ use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\GroupBy; +use Flow\ETL\Processor\GroupByProcessor; +use Flow\ETL\Rows; use Flow\ETL\Tests\FlowTestCase; use function Flow\ETL\DSL\config; @@ -24,32 +26,20 @@ public function test_group_by_missing_entry(): void { $groupBy = new GroupBy('type'); - $groupBy->group( - rows(row(str_entry('type', 'a')), row(str_entry('not-type', 'b')), row(str_entry('type', 'c'))), - flow_context(), - ); - static::assertEquals( rows(row(str_entry('type', 'a')), row(null_entry('type')), row(str_entry('type', 'c'))), - $groupBy->result(flow_context(config())), + $this->aggregate($groupBy, rows( + row(str_entry('type', 'a')), + row(str_entry('not-type', 'b')), + row(str_entry('type', 'c')), + )), ); } public function test_group_by_with_aggregation(): void { $group = new GroupBy('type'); - $group->aggregate(sum(ref('id'))); - $group->group( - rows( - row(int_entry('id', 1), str_entry('type', 'a')), - row(int_entry('id', 2), str_entry('type', 'b')), - row(int_entry('id', 3), str_entry('type', 'c')), - row(int_entry('id', 4), str_entry('type', 'a')), - row(int_entry('id', 5), str_entry('type', 'd')), - ), - flow_context(), - ); static::assertEquals( rows( @@ -58,7 +48,13 @@ public function test_group_by_with_aggregation(): void row(int_entry('id_sum', 3), str_entry('type', 'c')), row(int_entry('id_sum', 5), str_entry('type', 'd')), ), - $group->result(flow_context(config())), + $this->aggregate($group, rows( + row(int_entry('id', 1), str_entry('type', 'a')), + row(int_entry('id', 2), str_entry('type', 'b')), + row(int_entry('id', 3), str_entry('type', 'c')), + row(int_entry('id', 4), str_entry('type', 'a')), + row(int_entry('id', 5), str_entry('type', 'd')), + )), ); } @@ -72,27 +68,10 @@ public function test_group_by_with_empty_aggregations(): void public function test_group_by_with_pivoting(): void { - $rows = rows( - row(str_entry('product', 'Banana'), int_entry('amount', 1000), str_entry('country', 'USA')), - row(str_entry('product', 'Carrots'), int_entry('amount', 1500), str_entry('country', 'USA')), - row(str_entry('product', 'Beans'), int_entry('amount', 1600), str_entry('country', 'USA')), - row(str_entry('product', 'Orange'), int_entry('amount', 2000), str_entry('country', 'USA')), - row(str_entry('product', 'Orange'), int_entry('amount', 2000), str_entry('country', 'USA')), - row(str_entry('product', 'Banana'), int_entry('amount', 400), str_entry('country', 'China')), - row(str_entry('product', 'Carrots'), int_entry('amount', 1200), str_entry('country', 'China')), - row(str_entry('product', 'Beans'), int_entry('amount', 1500), str_entry('country', 'China')), - row(str_entry('product', 'Orange'), int_entry('amount', 4000), str_entry('country', 'China')), - row(str_entry('product', 'Banana'), int_entry('amount', 2000), str_entry('country', 'Canada')), - row(str_entry('product', 'Carrots'), int_entry('amount', 2000), str_entry('country', 'Canada')), - row(str_entry('product', 'Beans'), int_entry('amount', 2000), str_entry('country', 'Mexico')), - ); - $group = new GroupBy(ref('product')); $group->aggregate(sum(ref('amount'))); $group->pivot(ref('country')); - $group->group($rows, flow_context()); - static::assertEquals( rows( row( @@ -124,29 +103,52 @@ public function test_group_by_with_pivoting(): void int_entry('USA', 4000), ), ), - $group->result(flow_context(config()))->sortBy(ref('product')), + $this->aggregate($group, rows( + row(str_entry('product', 'Banana'), int_entry('amount', 1000), str_entry('country', 'USA')), + row(str_entry('product', 'Carrots'), int_entry('amount', 1500), str_entry('country', 'USA')), + row(str_entry('product', 'Beans'), int_entry('amount', 1600), str_entry('country', 'USA')), + row(str_entry('product', 'Orange'), int_entry('amount', 2000), str_entry('country', 'USA')), + row(str_entry('product', 'Orange'), int_entry('amount', 2000), str_entry('country', 'USA')), + row(str_entry('product', 'Banana'), int_entry('amount', 400), str_entry('country', 'China')), + row(str_entry('product', 'Carrots'), int_entry('amount', 1200), str_entry('country', 'China')), + row(str_entry('product', 'Beans'), int_entry('amount', 1500), str_entry('country', 'China')), + row(str_entry('product', 'Orange'), int_entry('amount', 4000), str_entry('country', 'China')), + row(str_entry('product', 'Banana'), int_entry('amount', 2000), str_entry('country', 'Canada')), + row(str_entry('product', 'Carrots'), int_entry('amount', 2000), str_entry('country', 'Canada')), + row(str_entry('product', 'Beans'), int_entry('amount', 2000), str_entry('country', 'Mexico')), + ))->sortBy(ref('product')), ); } public function test_group_by_with_pivoting_with_null_pivot_column(): void { - $rows = rows( - row(str_entry('product', 'Banana'), str_entry('country', 'USA'), int_entry('amount', 1000)), - row(str_entry('product', 'Apple'), str_entry('country', null), int_entry('amount', 400)), - ); - $group = new GroupBy(ref('product')); $group->aggregate(sum(ref('amount'))); $group->pivot(ref('country')); - $group->group($rows, flow_context()); - static::assertEquals( rows( row(str_entry('product', 'Apple'), null_entry('USA')), row(str_entry('product', 'Banana'), int_entry('USA', 1000)), ), - $group->result(flow_context(config()))->sortBy(ref('product')), + $this->aggregate($group, rows( + row(str_entry('product', 'Banana'), str_entry('country', 'USA'), int_entry('amount', 1000)), + row(str_entry('product', 'Apple'), str_entry('country', null), int_entry('amount', 400)), + ))->sortBy(ref('product')), ); } + + private function aggregate(GroupBy $groupBy, Rows $input): Rows + { + $result = new Rows(); + + $output = (new GroupByProcessor($groupBy))->process((static fn() => yield $input)(), flow_context(config())); + + /** @var Rows $batch */ + foreach ($output as $batch) { + $result = $result->merge($batch); + } + + return $result; + } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Processor/GroupByProcessorTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Processor/GroupByProcessorTest.php index fa42188bb7..0659e97f71 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Processor/GroupByProcessorTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Processor/GroupByProcessorTest.php @@ -89,7 +89,12 @@ public function test_handles_empty_input(): void /** @var list $result */ $result = iterator_to_array($processor->process($generator, flow_context())); - static::assertCount(1, $result); - static::assertCount(0, $result[0]); + $total = 0; + + foreach ($result as $batch) { + $total += $batch->count(); + } + + static::assertSame(0, $total); } } From e47c88008ed7a8ad61ea4087b7d91d5c29658036 Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Tue, 7 Jul 2026 08:15:24 +0200 Subject: [PATCH 3/8] refactor(flow-php/etl): adaptive hash aggregation with spill and merge Non-pivot aggregation now runs through hash aggregation with adaptive spilling: groups accumulate in an in-memory hash map, and once the configured memory limit is exceeded the map is spilled to a hash-sorted bucket and cleared. If nothing spills the map is the result; otherwise a streaming k-way merge folds partial aggregates by group key, so spilled state is O(groups), not O(rows). - merge() moves onto the base AggregatingFunction contract (no separate opt-in interface); every built-in aggregator implements it and self-validates by reference and result-affecting arguments - HashAggregation + GroupState / BucketGroup / GroupsMinHeap / GroupBucketsCache (FilesystemGroupBucketsCache), mirroring the external-sort building blocks - GroupBy becomes a spec (references/aggregations/hash/keyValues/newAggregators); pivot stays memory-only - config: aggregationMemoryLimit(Unit) (default unbounded, opt-in spilling) replaces the algorithm enum; output streamed in batches - fix multi-column group-by key hash collision (length-prefix the values) --- .../Config/Aggregation/AggregationConfig.php | 6 +- .../Aggregation/AggregationConfigBuilder.php | 31 ++- .../etl/src/Flow/ETL/Config/ConfigBuilder.php | 9 +- .../Flow/ETL/Function/AggregatingFunction.php | 2 + .../etl/src/Flow/ETL/Function/Average.php | 17 ++ .../etl/src/Flow/ETL/Function/Collect.php | 10 + .../src/Flow/ETL/Function/CollectUnique.php | 14 ++ src/core/etl/src/Flow/ETL/Function/Count.php | 13 ++ src/core/etl/src/Flow/ETL/Function/First.php | 9 + src/core/etl/src/Flow/ETL/Function/Last.php | 11 ++ src/core/etl/src/Flow/ETL/Function/Max.php | 19 ++ src/core/etl/src/Flow/ETL/Function/Min.php | 19 ++ .../src/Flow/ETL/Function/StringAggregate.php | 18 ++ src/core/etl/src/Flow/ETL/Function/Sum.php | 10 + src/core/etl/src/Flow/ETL/GroupBy.php | 28 +-- .../Flow/ETL/GroupBy/AggregatingAlgorithm.php | 25 --- .../ETL/GroupBy/AggregationAlgorithms.php | 25 --- .../etl/src/Flow/ETL/GroupBy/BucketGroup.php | 17 ++ .../FilesystemGroupBucketsCache.php | 96 +++++++++ .../Flow/ETL/GroupBy/ExternalAggregation.php | 110 ----------- .../Flow/ETL/GroupBy/GroupBucketsCache.php | 25 +++ .../etl/src/Flow/ETL/GroupBy/GroupState.php | 22 +++ .../src/Flow/ETL/GroupBy/GroupsMinHeap.php | 36 ++++ .../src/Flow/ETL/GroupBy/HashAggregation.php | 187 ++++++++++++++++++ .../Flow/ETL/GroupBy/MemoryAggregation.php | 62 ------ .../Flow/ETL/Processor/GroupByProcessor.php | 38 ++-- .../DataFrame/GroupByAggregationTest.php | 65 ++++-- .../AggregationConfigBuilderTest.php | 16 +- .../Function/AggregatingFunctionMergeTest.php | 113 +++++++++++ 29 files changed, 740 insertions(+), 313 deletions(-) delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/BucketGroup.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/ExternalAggregation.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/GroupState.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/GroupsMinHeap.php create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php delete mode 100644 src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php diff --git a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php index 1109d60a1c..cfcb9bd5ea 100644 --- a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php +++ b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfig.php @@ -4,12 +4,14 @@ namespace Flow\ETL\Config\Aggregation; -use Flow\ETL\GroupBy\AggregationAlgorithms; +use Flow\ETL\Dataset\Memory\Unit; final readonly class AggregationConfig { + public const string AGGREGATION_MAX_MEMORY_ENV = 'FLOW_AGGREGATION_MAX_MEMORY'; + public function __construct( - public AggregationAlgorithms $algorithm, + public Unit $memoryLimit, public string $filesystemProtocol = 'file', ) {} } diff --git a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php index 35277349c6..56446c2a6b 100644 --- a/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php +++ b/src/core/etl/src/Flow/ETL/Config/Aggregation/AggregationConfigBuilder.php @@ -4,24 +4,28 @@ namespace Flow\ETL\Config\Aggregation; -use Flow\ETL\GroupBy\AggregationAlgorithms; +use Flow\ETL\Dataset\Memory\Unit; + +use function getenv; +use function is_string; + +use const PHP_INT_MAX; final class AggregationConfigBuilder { - private AggregationAlgorithms $algorithm = AggregationAlgorithms::MEMORY_AGGREGATION; - private string $filesystemProtocol = 'file'; - public function algorithm(AggregationAlgorithms $algorithm): self - { - $this->algorithm = $algorithm; - - return $this; - } + private ?Unit $memoryLimit = null; public function build(): AggregationConfig { - return new AggregationConfig($this->algorithm, $this->filesystemProtocol); + if ($this->memoryLimit === null) { + $envLimit = getenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); + + $this->memoryLimit = is_string($envLimit) ? Unit::fromString($envLimit) : Unit::fromBytes(PHP_INT_MAX); + } + + return new AggregationConfig($this->memoryLimit, $this->filesystemProtocol); } public function filesystemProtocol(string $protocol): self @@ -30,4 +34,11 @@ public function filesystemProtocol(string $protocol): self return $this; } + + public function memoryLimit(Unit $memoryLimit): self + { + $this->memoryLimit = $memoryLimit; + + return $this; + } } diff --git a/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php b/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php index f98012c89b..609add4da6 100644 --- a/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php +++ b/src/core/etl/src/Flow/ETL/Config/ConfigBuilder.php @@ -15,7 +15,6 @@ use Flow\ETL\Config\Telemetry\TelemetryOptions; use Flow\ETL\Dataset\Memory\Unit; use Flow\ETL\Filesystem\FilesystemStreams; -use Flow\ETL\GroupBy\AggregationAlgorithms; use Flow\ETL\NativePHPRandomValueGenerator; use Flow\ETL\Pipeline\Optimizer; use Flow\ETL\Pipeline\Optimizer\BatchSizeOptimization; @@ -84,16 +83,16 @@ public function __construct() : PackageVersion::get('flow-php/etl'); } - public function aggregationAlgorithm(AggregationAlgorithms $algorithm): self + public function aggregationFilesystem(string $protocol): self { - $this->aggregation->algorithm($algorithm); + $this->aggregation->filesystemProtocol($protocol); return $this; } - public function aggregationFilesystem(string $protocol): self + public function aggregationMemoryLimit(Unit $unit): self { - $this->aggregation->filesystemProtocol($protocol); + $this->aggregation->memoryLimit($unit); return $this; } diff --git a/src/core/etl/src/Flow/ETL/Function/AggregatingFunction.php b/src/core/etl/src/Flow/ETL/Function/AggregatingFunction.php index 925e32da6b..b65743a9c3 100644 --- a/src/core/etl/src/Flow/ETL/Function/AggregatingFunction.php +++ b/src/core/etl/src/Flow/ETL/Function/AggregatingFunction.php @@ -13,6 +13,8 @@ interface AggregatingFunction { public function aggregate(Row $row, FlowContext $context): void; + public function merge(self $other): void; + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Average.php b/src/core/etl/src/Flow/ETL/Function/Average.php index d404adea22..00fe1a8775 100644 --- a/src/core/etl/src/Flow/ETL/Function/Average.php +++ b/src/core/etl/src/Flow/ETL/Function/Average.php @@ -82,6 +82,23 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return (new Calculator())->divide($sum, $count, $this->scale, $this->rounding); } + public function merge(AggregatingFunction $other): void + { + if ( + !$other instanceof self + || !$this->ref->is($other->ref) + || $this->scale !== $other->scale + || $this->rounding !== $other->rounding + ) { + throw new InvalidArgumentException( + 'Cannot merge ' . self::class . ' aggregating a different reference, scale or rounding', + ); + } + + $this->sum += $other->sum; + $this->count += $other->count; + } + public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/Function/Collect.php b/src/core/etl/src/Flow/ETL/Function/Collect.php index 12209f5710..93614b95e4 100644 --- a/src/core/etl/src/Flow/ETL/Function/Collect.php +++ b/src/core/etl/src/Flow/ETL/Function/Collect.php @@ -11,6 +11,7 @@ use Flow\ETL\Row\EntryFactory; use Flow\ETL\Row\Reference; +use function array_merge; use function current; use function Flow\ETL\DSL\to_entry; @@ -41,6 +42,15 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + $this->collection = array_merge($this->collection, $other->collection); + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/CollectUnique.php b/src/core/etl/src/Flow/ETL/Function/CollectUnique.php index 3f87b0ff09..e4c9173c94 100644 --- a/src/core/etl/src/Flow/ETL/Function/CollectUnique.php +++ b/src/core/etl/src/Flow/ETL/Function/CollectUnique.php @@ -49,6 +49,20 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + /** @var mixed $value */ + foreach ($other->collection as $value) { + if (!in_array($value, $this->collection, true)) { + $this->collection[] = $value; + } + } + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Count.php b/src/core/etl/src/Flow/ETL/Function/Count.php index 52e585ef3c..ddd9459dfc 100644 --- a/src/core/etl/src/Flow/ETL/Function/Count.php +++ b/src/core/etl/src/Flow/ETL/Function/Count.php @@ -80,6 +80,19 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return $count; } + public function merge(AggregatingFunction $other): void + { + if ( + !$other instanceof self + || ($this->ref === null) !== ($other->ref === null) + || ($this->ref !== null && $other->ref !== null && !$this->ref->is($other->ref)) + ) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + $this->count += $other->count; + } + public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/Function/First.php b/src/core/etl/src/Flow/ETL/Function/First.php index 36126f0176..7be217b5a6 100644 --- a/src/core/etl/src/Flow/ETL/Function/First.php +++ b/src/core/etl/src/Flow/ETL/Function/First.php @@ -37,6 +37,15 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + $this->first ??= $other->first; + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Last.php b/src/core/etl/src/Flow/ETL/Function/Last.php index d5c1e76f76..6dd8f36b8d 100644 --- a/src/core/etl/src/Flow/ETL/Function/Last.php +++ b/src/core/etl/src/Flow/ETL/Function/Last.php @@ -35,6 +35,17 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + if ($other->last !== null) { + $this->last = $other->last; + } + } + /** * @return Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Max.php b/src/core/etl/src/Flow/ETL/Function/Max.php index 6794783b8c..36b88e69f4 100644 --- a/src/core/etl/src/Flow/ETL/Function/Max.php +++ b/src/core/etl/src/Flow/ETL/Function/Max.php @@ -52,6 +52,25 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + if ($other->max === null) { + return; + } + + if ($this->max === null) { + $this->max = $other->max; + + return; + } + + $this->max = max($this->max, $other->max); + } + /** * @return Entry|Entry|Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Min.php b/src/core/etl/src/Flow/ETL/Function/Min.php index 4960439cae..c3d5578fd5 100644 --- a/src/core/etl/src/Flow/ETL/Function/Min.php +++ b/src/core/etl/src/Flow/ETL/Function/Min.php @@ -52,6 +52,25 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + if ($other->min === null) { + return; + } + + if ($this->min === null) { + $this->min = $other->min; + + return; + } + + $this->min = min($this->min, $other->min); + } + /** * @return Entry|Entry|Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/StringAggregate.php b/src/core/etl/src/Flow/ETL/Function/StringAggregate.php index 13f1e468a3..f190a7a203 100644 --- a/src/core/etl/src/Flow/ETL/Function/StringAggregate.php +++ b/src/core/etl/src/Flow/ETL/Function/StringAggregate.php @@ -4,6 +4,7 @@ namespace Flow\ETL\Function; +use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\FlowContext; use Flow\ETL\Row; use Flow\ETL\Row\Entry; @@ -11,6 +12,7 @@ use Flow\ETL\Row\Reference; use Flow\ETL\Row\SortOrder; +use function array_merge; use function count; use function Flow\ETL\DSL\str_entry; use function implode; @@ -40,6 +42,22 @@ public function aggregate(Row $row, FlowContext $context): void } } + public function merge(AggregatingFunction $other): void + { + if ( + !$other instanceof self + || !$this->ref->is($other->ref) + || $this->separator !== $other->separator + || $this->sort !== $other->sort + ) { + throw new InvalidArgumentException( + 'Cannot merge ' . self::class . ' aggregating a different reference, separator or sort', + ); + } + + $this->values = array_merge($this->values, $other->values); + } + /** * @return Row\Entry */ diff --git a/src/core/etl/src/Flow/ETL/Function/Sum.php b/src/core/etl/src/Flow/ETL/Function/Sum.php index 77415475c3..4289b5263a 100644 --- a/src/core/etl/src/Flow/ETL/Function/Sum.php +++ b/src/core/etl/src/Flow/ETL/Function/Sum.php @@ -68,6 +68,16 @@ public function apply(Row $row, Rows $partition, FlowContext $context): mixed return $sum; } + public function merge(AggregatingFunction $other): void + { + if (!$other instanceof self || !$this->ref->is($other->ref)) { + throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); + } + + // @mago-ignore analysis:possibly-invalid-argument + $this->sum = (new Calculator())->add($this->sum, $other->sum); + } + public function over(Window $window): WindowFunction { $this->window = $window; diff --git a/src/core/etl/src/Flow/ETL/GroupBy.php b/src/core/etl/src/Flow/ETL/GroupBy.php index 0ce6f8ca6f..4594280273 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy.php +++ b/src/core/etl/src/Flow/ETL/GroupBy.php @@ -27,18 +27,11 @@ use function Flow\Types\DSL\type_integer; use function Flow\Types\DSL\type_string; use function Flow\Types\DSL\type_union; -use function implode; use function is_array; use function is_scalar; use function serialize; +use function strlen; -/** - * Specification of a group-by: the key columns and the aggregations to run. - * - * The actual accumulation of non-pivot groups is done by the aggregating algorithms - * (see {@see \Flow\ETL\GroupBy\AggregatingAlgorithm}); this class only exposes the spec - * and small helpers they need. Pivot is a memory-only cross-tab and is executed here. - */ final class GroupBy { private const int RESULT_BATCH_SIZE = 1000; @@ -73,8 +66,6 @@ public function aggregate(AggregatingFunction ...$aggregator): void } /** - * Build the output row for one finalized group. - * * @param array $keyValues * @param array $aggregators */ @@ -103,8 +94,6 @@ public function aggregations(): array } /** - * A stable string key for the given group-by values (used to bucket equal groups). - * * @param array $values */ public function hash(array $values): string @@ -131,7 +120,13 @@ public function hash(array $values): string } } - return NativePHPHash::xxh128(implode('', $stringValues)); + $key = ''; + + foreach ($stringValues as $stringValue) { + $key .= strlen($stringValue) . ':' . $stringValue; + } + + return NativePHPHash::xxh128($key); } public function isPivot(): bool @@ -140,8 +135,6 @@ public function isPivot(): bool } /** - * The group-by key values for a single row (missing columns become null). - * * @return array */ public function keyValues(Row $row): array @@ -160,8 +153,6 @@ public function keyValues(Row $row): array } /** - * A fresh set of aggregator instances for a new group. - * * @return array */ public function newAggregators(): array @@ -178,9 +169,6 @@ public function pivot(Reference $ref): void } /** - * Execute the pivot cross-tab. Pivot is memory-only (it must know every distinct pivot - * value to build the columns), so it always aggregates in memory and streams the result. - * * @param Generator $rows * * @return Generator diff --git a/src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php b/src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php deleted file mode 100644 index 8e4f54e301..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/AggregatingAlgorithm.php +++ /dev/null @@ -1,25 +0,0 @@ - $rows - * - * @return Generator - */ - public function aggregate(Generator $rows, FlowContext $context, GroupBy $groupBy): Generator; -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php b/src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php deleted file mode 100644 index 5ef1902306..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/AggregationAlgorithms.php +++ /dev/null @@ -1,25 +0,0 @@ - $chunkSize + */ + public function __construct( + private Filesystem $filesystem, + private Serializer $serializer = new NativePHPSerializer(), + private int $chunkSize = 100, + ?Path $cacheDir = null, + ) { + // @mago-ignore analysis:impossible-condition,redundant-comparison + if ($this->chunkSize < 1) { + throw new InvalidArgumentException('Chunk size must be greater than 0'); + } + + $this->cacheDir = ($cacheDir ?? $this->filesystem->getSystemTmpDir())->suffix('/flow-php-group-by/'); + } + + public function get(string $bucketId): Generator + { + $path = $this->keyPath($bucketId); + + if (!$this->filesystem->status($path)) { + return; + } + + $stream = $this->filesystem->readFrom($path); + + foreach ($stream->readLines() as $line) { + $separator = strpos($line, ' '); + $hash = substr($line, 0, $separator); + $serialized = substr($line, $separator + 1); + + yield [$hash, $this->serializer->unserialize($serialized, [GroupState::class])]; + } + + $stream->close(); + } + + public function remove(string $bucketId): void + { + $this->filesystem->rm($this->keyPath($bucketId)->parentDirectory()); + } + + public function set(string $bucketId, iterable $groups): void + { + $stream = $this->filesystem->writeTo($this->keyPath($bucketId)); + + $buffer = ''; + $counter = 0; + + foreach ($groups as [$hash, $state]) { + $buffer .= $hash . ' ' . $this->serializer->serialize($state) . "\n"; + $counter++; + + if ($counter >= $this->chunkSize) { + $stream->append($buffer); + $buffer = ''; + $counter = 0; + } + } + + if ($counter > 0) { + $stream->append($buffer); + } + + $stream->close(); + } + + private function keyPath(string $key): Path + { + return $this->cacheDir->suffix(NativePHPHash::xxh128($key) . '/' . $key . '.php.cache'); + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/ExternalAggregation.php b/src/core/etl/src/Flow/ETL/GroupBy/ExternalAggregation.php deleted file mode 100644 index c8324cf22f..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/ExternalAggregation.php +++ /dev/null @@ -1,110 +0,0 @@ -references()->all(); - $sortRefs[] = ref(self::INPUT_ORDINAL); - - $sorted = $this->sort->sortGenerator($this->withInputOrdinal($rows), $context, References::init(...$sortRefs)); - - $currentHash = null; - /** @var array $currentKey */ - $currentKey = []; - /** @var array $aggregators */ - $aggregators = []; - $buffer = []; - - foreach ($sorted as $batch) { - foreach ($batch as $row) { - $keyValues = $groupBy->keyValues($row); - $hash = $groupBy->hash($keyValues); - - if ($hash !== $currentHash) { - if ($currentHash !== null) { - $buffer[] = $groupBy->aggregatedRow($currentKey, $aggregators, $context->entryFactory()); - - if (count($buffer) >= self::BATCH_SIZE) { - yield new Rows(...$buffer); - $buffer = []; - } - } - - $currentHash = $hash; - $currentKey = $keyValues; - $aggregators = $groupBy->newAggregators(); - } - - foreach ($aggregators as $aggregator) { - $aggregator->aggregate($row, $context); - } - } - } - - if ($currentHash !== null) { - $buffer[] = $groupBy->aggregatedRow($currentKey, $aggregators, $context->entryFactory()); - } - - if ($buffer !== []) { - yield new Rows(...$buffer); - } - } - - /** - * @param Generator $rows - * - * @return Generator - */ - private function withInputOrdinal(Generator $rows): Generator - { - $ordinal = 0; - - foreach ($rows as $batch) { - $tagged = []; - - foreach ($batch as $row) { - $tagged[] = $row->add(int_entry(self::INPUT_ORDINAL, $ordinal)); - $ordinal++; - } - - yield new Rows(...$tagged); - } - } -} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php new file mode 100644 index 0000000000..1c1fd5352b --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php @@ -0,0 +1,25 @@ + + */ + public function get(string $bucketId): Generator; + + public function remove(string $bucketId): void; + + /** + * @param iterable $groups + */ + public function set(string $bucketId, iterable $groups): void; +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/GroupState.php b/src/core/etl/src/Flow/ETL/GroupBy/GroupState.php new file mode 100644 index 0000000000..ded39a6a25 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/GroupState.php @@ -0,0 +1,22 @@ + $keyValues + * @param array $aggregators + */ + public function __construct( + public array $keyValues, + public array $aggregators, + ) {} +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/GroupsMinHeap.php b/src/core/etl/src/Flow/ETL/GroupBy/GroupsMinHeap.php new file mode 100644 index 0000000000..8bd8180101 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/GroupsMinHeap.php @@ -0,0 +1,36 @@ + + */ +final class GroupsMinHeap extends SplMinHeap +{ + public function insert(mixed $value): true + { + if (!$value instanceof BucketGroup) { + throw new InvalidArgumentException('Value inserted into GroupsMinHeap must be a ' . BucketGroup::class); + } + + parent::insert($value); + + return true; + } + + /** + * @param BucketGroup $value1 + * @param BucketGroup $value2 + */ + protected function compare($value1, $value2): int + { + return [$value2->hash, $value2->bucketIndex] <=> [$value1->hash, $value1->bucketIndex]; + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php b/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php new file mode 100644 index 0000000000..cd2bb712c1 --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php @@ -0,0 +1,187 @@ + $rows + * + * @return Generator + */ + public function aggregate(Generator $rows, FlowContext $context, GroupBy $groupBy): Generator + { + $consumption = new Consumption(false); + /** @var array $hot */ + $hot = []; + /** @var list $buckets */ + $buckets = []; + + foreach ($rows as $batch) { + foreach ($batch as $row) { + $keyValues = $groupBy->keyValues($row); + $hash = $groupBy->hash($keyValues); + + if (!array_key_exists($hash, $hot)) { + $hot[$hash] = new GroupState($keyValues, $groupBy->newAggregators()); + } + + foreach ($hot[$hash]->aggregators as $aggregator) { + $aggregator->aggregate($row, $context); + } + } + + if ($consumption->currentDiff()->isGreaterThan($this->memoryLimit)) { + $buckets[] = $this->spill($hot); + $hot = []; + $consumption = new Consumption(false); + } + } + + $groups = $buckets === [] ? $this->iterate($hot) : $this->merge($buckets, $hot); + + yield from $this->emit($groups, $groupBy, $context); + } + + /** + * @param Generator $groups + * + * @return Generator + */ + private function emit(Generator $groups, GroupBy $groupBy, FlowContext $context): Generator + { + $buffer = []; + + foreach ($groups as $state) { + $buffer[] = $groupBy->aggregatedRow($state->keyValues, $state->aggregators, $context->entryFactory()); + + if (count($buffer) >= self::BATCH_SIZE) { + yield new Rows(...$buffer); + $buffer = []; + } + } + + if ($buffer !== []) { + yield new Rows(...$buffer); + } + } + + /** + * @param array $hot + * + * @return Generator + */ + private function iterate(array $hot): Generator + { + foreach ($hot as $state) { + yield $state; + } + } + + /** + * @param list $buckets + * @param array $hot + * + * @return Generator + */ + private function merge(array $buckets, array $hot): Generator + { + ksort($hot); + + /** @var array> $cursors */ + $cursors = []; + + foreach ($buckets as $index => $bucketId) { + $cursors[$index] = $this->cache->get($bucketId); + } + + $cursors[count($buckets)] = (static function () use ($hot): Generator { + foreach ($hot as $hash => $state) { + yield [$hash, $state]; + } + })(); + + $heap = new GroupsMinHeap(); + + foreach ($cursors as $index => $cursor) { + $this->advance($cursor, $index, $heap); + } + + while (!$heap->isEmpty()) { + $current = $heap->extract(); + $merged = $current->state; + $this->advance($cursors[$current->bucketIndex], $current->bucketIndex, $heap); + + while (!$heap->isEmpty() && $heap->top()->hash === $current->hash) { + $next = $heap->extract(); + + foreach ($merged->aggregators as $index => $aggregator) { + $aggregator->merge($next->state->aggregators[$index]); + } + + $this->advance($cursors[$next->bucketIndex], $next->bucketIndex, $heap); + } + + yield $merged; + } + + foreach ($buckets as $bucketId) { + $this->cache->remove($bucketId); + } + } + + /** + * @param Generator $cursor + */ + private function advance(Generator $cursor, int $bucketIndex, GroupsMinHeap $heap): void + { + if ($cursor->valid()) { + [$hash, $state] = $cursor->current(); + $heap->insert(new BucketGroup($hash, $bucketIndex, $state)); + $cursor->next(); + } + } + + /** + * @param array $hot + */ + private function spill(array $hot): string + { + ksort($hot); + + $bucketId = bin2hex(random_bytes(16)); + + $this->cache->set($bucketId, (static function () use ($hot): Generator { + foreach ($hot as $hash => $state) { + yield [$hash, $state]; + } + })()); + + return $bucketId; + } +} diff --git a/src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php b/src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php deleted file mode 100644 index daac717d81..0000000000 --- a/src/core/etl/src/Flow/ETL/GroupBy/MemoryAggregation.php +++ /dev/null @@ -1,62 +0,0 @@ -, aggregators: array}> $groups */ - $groups = []; - - foreach ($rows as $batch) { - foreach ($batch as $row) { - $keyValues = $groupBy->keyValues($row); - $hash = $groupBy->hash($keyValues); - - if (!array_key_exists($hash, $groups)) { - $groups[$hash] = ['keyValues' => $keyValues, 'aggregators' => $groupBy->newAggregators()]; - } - - foreach ($groups[$hash]['aggregators'] as $aggregator) { - $aggregator->aggregate($row, $context); - } - } - } - - $buffer = []; - - foreach ($groups as $group) { - $buffer[] = $groupBy->aggregatedRow($group['keyValues'], $group['aggregators'], $context->entryFactory()); - - if (count($buffer) >= self::BATCH_SIZE) { - yield new Rows(...$buffer); - $buffer = []; - } - } - - if ($buffer !== []) { - yield new Rows(...$buffer); - } - } -} diff --git a/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php b/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php index 53e01179cb..ccc4eb59e5 100644 --- a/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php +++ b/src/core/etl/src/Flow/ETL/Processor/GroupByProcessor.php @@ -6,21 +6,14 @@ use Flow\ETL\FlowContext; use Flow\ETL\GroupBy; -use Flow\ETL\GroupBy\AggregatingAlgorithm; -use Flow\ETL\GroupBy\ExternalAggregation; -use Flow\ETL\GroupBy\MemoryAggregation; +use Flow\ETL\GroupBy\BucketsCache\FilesystemGroupBucketsCache; +use Flow\ETL\GroupBy\HashAggregation; use Flow\ETL\Processor; -use Flow\ETL\Sort\ExternalSort; -use Flow\ETL\Sort\ExternalSort\BucketsCache\FilesystemBucketsCache; use Generator; /** * Groups all rows and applies aggregation functions. * - * Picks the aggregation algorithm from configuration (mirroring SortingProcessor): in-memory - * by default, or an external sort-based fold for bounded memory. Pivot and global aggregation - * (no group-by columns) always run in memory. - * * @internal */ final readonly class GroupByProcessor implements Processor @@ -37,27 +30,18 @@ public function process(Generator $rows, FlowContext $context): Generator return; } - yield from $this->algorithm($context)->aggregate($rows, $context, $this->groupBy); - } - - private function algorithm(FlowContext $context): AggregatingAlgorithm - { $config = $context->config->aggregation; - if ($config->algorithm->useMemory() || $this->groupBy->references()->count() === 0) { - return new MemoryAggregation(); - } - - return new ExternalAggregation( - new ExternalSort( - new FilesystemBucketsCache( - $context->filesystem($config->filesystemProtocol), - $context->config->serializer(), - 100, - $context->config->cache->localFilesystemCacheDir->suffix('/flow-php-group-by/'), - ), - $context->config->cache->externalSortBucketsCount, + $aggregation = new HashAggregation( + $config->memoryLimit, + new FilesystemGroupBucketsCache( + $context->filesystem($config->filesystemProtocol), + $context->config->serializer(), + 100, + $context->config->cache->localFilesystemCacheDir->suffix('/flow-php-group-by/'), ), ); + + yield from $aggregation->aggregate($rows, $context, $this->groupBy); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php index 0dfbbbec41..857c7edd72 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php @@ -5,7 +5,7 @@ namespace Flow\ETL\Tests\Integration\DataFrame; use Flow\ETL\Config\ConfigBuilder; -use Flow\ETL\GroupBy\AggregationAlgorithms; +use Flow\ETL\Dataset\Memory\Unit; use Flow\ETL\Tests\Double\FakeRandomOrdersExtractor; use Flow\ETL\Tests\FlowIntegrationTestCase; @@ -29,17 +29,15 @@ final class GroupByAggregationTest extends FlowIntegrationTestCase */ private static ?array $orders = null; - public function test_first_and_last_preserve_input_order_under_external_spill(): void + public function test_first_and_last_preserve_input_order_across_spilled_buckets(): void { - // 2000 rows interleaved into two groups, each spanning multiple spilled sort runs; - // v is the input position so first/last are unambiguous in input order. $data = []; - for ($i = 0; $i < 2000; $i++) { - $data[] = ['g' => ($i % 2) === 0 ? 'a' : 'b', 'v' => $i]; + for ($i = 0; $i < 400; $i++) { + $data[] = ['g' => $i % 2 === 0 ? 'a' : 'b', 'v' => $i]; } - $run = static fn(ConfigBuilder $config): array => df($config) + $run = static fn (ConfigBuilder $config): array => df($config) ->read(from_array($data)) ->groupBy('g') ->aggregate(first(ref('v')), last(ref('v'))) @@ -48,42 +46,71 @@ public function test_first_and_last_preserve_input_order_under_external_spill(): ->toArray(); $memory = $run(config_builder()); - $external = $run(config_builder()->aggregationAlgorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION)); + $spilled = $run(config_builder()->aggregationMemoryLimit(Unit::fromBytes(1))); - self::assertSame($memory, $external); + self::assertSame($memory, $spilled); self::assertSame( [ - ['g' => 'a', 'v_first' => 0, 'v_last' => 1998], - ['g' => 'b', 'v_first' => 1, 'v_last' => 1999], + ['g' => 'a', 'v_first' => 0, 'v_last' => 398], + ['g' => 'b', 'v_first' => 1, 'v_last' => 399], ], - $external, + $spilled, ); } - public function test_high_cardinality_group_by_is_identical_between_memory_and_external(): void + public function test_high_cardinality_group_by_is_identical_when_spilling(): void { $memory = $this->aggregateByEmail(config_builder()); + $spilled = $this->aggregateByEmail(config_builder()->aggregationMemoryLimit(Unit::fromKb(256))); - $external = $this->aggregateByEmail(config_builder()->aggregationAlgorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION)); + self::assertEqualsCanonicalizing($memory, $spilled); + } + + public function test_multi_column_keys_do_not_collide(): void + { + $data = [ + ['x' => 'a', 'y' => 'bc'], + ['x' => 'ab', 'y' => 'c'], + ['x' => 'a', 'y' => 'bc'], + ]; + + $run = static fn (ConfigBuilder $config): array => df($config) + ->read(from_array($data)) + ->groupBy('x', 'y') + ->aggregate(count()) + ->sortBy(ref('x')) + ->fetch() + ->toArray(); - self::assertEqualsCanonicalizing($memory, $external); + $expected = [ + ['x' => 'a', 'y' => 'bc', '_count' => 2], + ['x' => 'ab', 'y' => 'c', '_count' => 1], + ]; + + self::assertSame($expected, $run(config_builder())); + self::assertSame($expected, $run(config_builder()->aggregationMemoryLimit(Unit::fromBytes(1)))); } - public function test_seller_aggregation_is_identical_between_memory_and_external(): void + public function test_seller_aggregation_is_identical_when_spilling(): void { $memory = $this->aggregateBySeller(config_builder()); + $spilled = $this->aggregateBySeller(config_builder()->aggregationMemoryLimit(Unit::fromKb(256))); - $external = $this->aggregateBySeller(config_builder()->aggregationAlgorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION)); - - self::assertEqualsCanonicalizing($memory, $external); + self::assertEqualsCanonicalizing($memory, $spilled); self::assertCount(5, $memory); } + /** + * @return array> + */ private function aggregateByEmail(ConfigBuilder $config): array { return df($config)->read(from_array($this->orders()))->groupBy('email')->aggregate(count())->fetch()->toArray(); } + /** + * @return array> + */ private function aggregateBySeller(ConfigBuilder $config): array { return df($config) diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php index 2763f800e0..7a5bf424c5 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php @@ -5,29 +5,29 @@ namespace Flow\ETL\Tests\Unit\Config\Aggregation; use Flow\ETL\Config\Aggregation\AggregationConfigBuilder; -use Flow\ETL\GroupBy\AggregationAlgorithms; +use Flow\ETL\Dataset\Memory\Unit; use Flow\ETL\Tests\FlowTestCase; +use const PHP_INT_MAX; + final class AggregationConfigBuilderTest extends FlowTestCase { - public function test_default_algorithm_is_memory(): void + public function test_default_memory_limit_is_unbounded(): void { $config = (new AggregationConfigBuilder())->build(); - self::assertSame(AggregationAlgorithms::MEMORY_AGGREGATION, $config->algorithm); - self::assertTrue($config->algorithm->useMemory()); + self::assertSame(PHP_INT_MAX, $config->memoryLimit->inBytes()); self::assertSame('file', $config->filesystemProtocol); } - public function test_selecting_the_external_algorithm(): void + public function test_setting_a_memory_limit_and_filesystem(): void { $config = (new AggregationConfigBuilder()) - ->algorithm(AggregationAlgorithms::EXTERNAL_AGGREGATION) + ->memoryLimit(Unit::fromMb(16)) ->filesystemProtocol('memory') ->build(); - self::assertSame(AggregationAlgorithms::EXTERNAL_AGGREGATION, $config->algorithm); - self::assertFalse($config->algorithm->useMemory()); + self::assertSame(Unit::fromMb(16)->inBytes(), $config->memoryLimit->inBytes()); self::assertSame('memory', $config->filesystemProtocol); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php new file mode 100644 index 0000000000..a5e8b43c11 --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php @@ -0,0 +1,113 @@ +aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = average(ref('v')); + $right->aggregate(row(int_entry('v', 6)), $context); + + $left->merge($right); + + self::assertSame(3, $left->result($context->entryFactory())->value()); + } + + public function test_collect_merges_in_order(): void + { + $context = flow_context(); + + $left = collect(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + + $right = collect(ref('v')); + $right->aggregate(row(int_entry('v', 2)), $context); + + $left->merge($right); + + self::assertSame([1, 2], $left->result($context->entryFactory())->value()); + } + + public function test_first_keeps_the_earlier_partial(): void + { + $context = flow_context(); + + $left = first(ref('v')); + $left->aggregate(row(int_entry('v', 10)), $context); + + $right = first(ref('v')); + $right->aggregate(row(int_entry('v', 20)), $context); + + $left->merge($right); + + self::assertSame(10, $left->result($context->entryFactory())->value()); + } + + public function test_last_keeps_the_later_partial(): void + { + $context = flow_context(); + + $left = last(ref('v')); + $left->aggregate(row(int_entry('v', 10)), $context); + + $right = last(ref('v')); + $right->aggregate(row(int_entry('v', 20)), $context); + + $left->merge($right); + + self::assertSame(20, $left->result($context->entryFactory())->value()); + } + + public function test_merging_a_different_function_type_throws(): void + { + $this->expectException(InvalidArgumentException::class); + + sum(ref('v'))->merge(count()); + } + + public function test_merging_the_same_function_on_a_different_reference_throws(): void + { + $this->expectException(InvalidArgumentException::class); + + sum(ref('a'))->merge(sum(ref('b'))); + } + + public function test_sum_merges_partial_sums(): void + { + $context = flow_context(); + + $left = sum(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = sum(ref('v')); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame(6, $left->result($context->entryFactory())->value()); + } +} From 2d2734728c74c2dbc1c69f5024ce4d3f2d17bcab Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Tue, 7 Jul 2026 08:43:10 +0200 Subject: [PATCH 4/8] test(flow-php/etl): fix static analysis and cover aggregation merge - guard strpos()===false and drop unused mago pragmas flagged by CI static analysis - annotate the bucket-cursor generators as Generator and narrow current() in the k-way merge - cover merge() on all 10 aggregators (success, null branches, self-validation), the min-heap ordering/guard, the bucket cache round-trip, and the memory-limit env var --- src/core/etl/src/Flow/ETL/GroupBy.php | 1 - .../FilesystemGroupBucketsCache.php | 14 +- .../Flow/ETL/GroupBy/GroupBucketsCache.php | 2 +- .../src/Flow/ETL/GroupBy/HashAggregation.php | 39 ++-- .../FilesystemGroupBucketsCacheTest.php | 50 +++++ .../AggregationConfigBuilderTest.php | 16 ++ .../Function/AggregatingFunctionMergeTest.php | 202 +++++++++++++++++- .../Tests/Unit/GroupBy/GroupsMinHeapTest.php | 38 ++++ 8 files changed, 337 insertions(+), 25 deletions(-) create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php create mode 100644 src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php diff --git a/src/core/etl/src/Flow/ETL/GroupBy.php b/src/core/etl/src/Flow/ETL/GroupBy.php index 4594280273..7c816a3a1d 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy.php +++ b/src/core/etl/src/Flow/ETL/GroupBy.php @@ -220,7 +220,6 @@ public function pivotResult(Generator $rows, FlowContext $context): Generator $pivotValue = type_union(type_string(), type_integer())->assert($pivotValue); if (!array_key_exists($pivotValue, $pivotedTable[$indexValue])) { - // @mago-ignore analysis:invalid-property-assignment-value,possibly-invalid-clone $pivotedTable[$indexValue][$pivotValue] = clone current($this->aggregations); } diff --git a/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php index a8d3f1c4bb..892ba2e276 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php +++ b/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php @@ -4,7 +4,6 @@ namespace Flow\ETL\GroupBy\BucketsCache; -use Flow\ETL\Exception\InvalidArgumentException; use Flow\ETL\GroupBy\GroupBucketsCache; use Flow\ETL\GroupBy\GroupState; use Flow\ETL\Hash\NativePHPHash; @@ -30,14 +29,12 @@ public function __construct( private int $chunkSize = 100, ?Path $cacheDir = null, ) { - // @mago-ignore analysis:impossible-condition,redundant-comparison - if ($this->chunkSize < 1) { - throw new InvalidArgumentException('Chunk size must be greater than 0'); - } - $this->cacheDir = ($cacheDir ?? $this->filesystem->getSystemTmpDir())->suffix('/flow-php-group-by/'); } + /** + * @return Generator + */ public function get(string $bucketId): Generator { $path = $this->keyPath($bucketId); @@ -50,6 +47,11 @@ public function get(string $bucketId): Generator foreach ($stream->readLines() as $line) { $separator = strpos($line, ' '); + + if ($separator === false) { + continue; + } + $hash = substr($line, 0, $separator); $serialized = substr($line, $separator + 1); diff --git a/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php index 1c1fd5352b..40c329a3c9 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php +++ b/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php @@ -12,7 +12,7 @@ interface GroupBucketsCache { /** - * @return Generator + * @return Generator */ public function get(string $bucketId): Generator; diff --git a/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php b/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php index cd2bb712c1..06d265fade 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php +++ b/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php @@ -113,18 +113,14 @@ private function merge(array $buckets, array $hot): Generator { ksort($hot); - /** @var array> $cursors */ + /** @var array> $cursors */ $cursors = []; foreach ($buckets as $index => $bucketId) { $cursors[$index] = $this->cache->get($bucketId); } - $cursors[count($buckets)] = (static function () use ($hot): Generator { - foreach ($hot as $hash => $state) { - yield [$hash, $state]; - } - })(); + $cursors[count($buckets)] = $this->hotCursor($hot); $heap = new GroupsMinHeap(); @@ -156,14 +152,29 @@ private function merge(array $buckets, array $hot): Generator } /** - * @param Generator $cursor + * @param Generator $cursor */ private function advance(Generator $cursor, int $bucketIndex, GroupsMinHeap $heap): void { - if ($cursor->valid()) { - [$hash, $state] = $cursor->current(); - $heap->insert(new BucketGroup($hash, $bucketIndex, $state)); - $cursor->next(); + if (!$cursor->valid()) { + return; + } + + /** @var array{0: string, 1: GroupState} $item */ + $item = $cursor->current(); + $heap->insert(new BucketGroup($item[0], $bucketIndex, $item[1])); + $cursor->next(); + } + + /** + * @param array $hot + * + * @return Generator + */ + private function hotCursor(array $hot): Generator + { + foreach ($hot as $hash => $state) { + yield [$hash, $state]; } } @@ -176,11 +187,7 @@ private function spill(array $hot): string $bucketId = bin2hex(random_bytes(16)); - $this->cache->set($bucketId, (static function () use ($hot): Generator { - foreach ($hot as $hash => $state) { - yield [$hash, $state]; - } - })()); + $this->cache->set($bucketId, $this->hotCursor($hot)); return $bucketId; } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php new file mode 100644 index 0000000000..4358b59b28 --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php @@ -0,0 +1,50 @@ +fs(), $this->serializer(), 100, $this->cacheDir); + + self::assertSame([], iterator_to_array($cache->get('does-not-exist'))); + } + + public function test_round_trip_and_remove(): void + { + $cache = new FilesystemGroupBucketsCache($this->fs(), $this->serializer(), 2, $this->cacheDir); + + $groups = [ + ['aaa', new GroupState(['type' => 'a'], [sum(ref('v'))])], + ['bbb', new GroupState(['type' => 'b'], [sum(ref('v'))])], + ['ccc', new GroupState(['type' => 'c'], [sum(ref('v'))])], + ]; + + $cache->set('bucket-1', (static function () use ($groups) { + yield from $groups; + })()); + + $read = []; + + foreach ($cache->get('bucket-1') as [$hash, $state]) { + $read[$hash] = $state->keyValues; + } + + self::assertSame(['aaa' => ['type' => 'a'], 'bbb' => ['type' => 'b'], 'ccc' => ['type' => 'c']], $read); + + $cache->remove('bucket-1'); + + self::assertSame([], iterator_to_array($cache->get('bucket-1'))); + } +} diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php index 7a5bf424c5..1dc0315a04 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php @@ -4,10 +4,13 @@ namespace Flow\ETL\Tests\Unit\Config\Aggregation; +use Flow\ETL\Config\Aggregation\AggregationConfig; use Flow\ETL\Config\Aggregation\AggregationConfigBuilder; use Flow\ETL\Dataset\Memory\Unit; use Flow\ETL\Tests\FlowTestCase; +use function putenv; + use const PHP_INT_MAX; final class AggregationConfigBuilderTest extends FlowTestCase @@ -20,6 +23,19 @@ public function test_default_memory_limit_is_unbounded(): void self::assertSame('file', $config->filesystemProtocol); } + public function test_memory_limit_is_read_from_the_environment(): void + { + putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV . '=32M'); + + try { + $config = (new AggregationConfigBuilder())->build(); + + self::assertSame(Unit::fromMb(32)->inBytes(), $config->memoryLimit->inBytes()); + } finally { + putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); + } + } + public function test_setting_a_memory_limit_and_filesystem(): void { $config = (new AggregationConfigBuilder()) diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php index a5e8b43c11..6e0c564827 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php @@ -5,17 +5,23 @@ namespace Flow\ETL\Tests\Unit\Function; use Flow\ETL\Exception\InvalidArgumentException; +use Flow\ETL\Function\StringAggregate; +use Flow\ETL\Row\SortOrder; use Flow\ETL\Tests\FlowTestCase; use function Flow\ETL\DSL\average; use function Flow\ETL\DSL\collect; +use function Flow\ETL\DSL\collect_unique; use function Flow\ETL\DSL\count; use function Flow\ETL\DSL\first; use function Flow\ETL\DSL\flow_context; use function Flow\ETL\DSL\int_entry; use function Flow\ETL\DSL\last; +use function Flow\ETL\DSL\max; +use function Flow\ETL\DSL\min; use function Flow\ETL\DSL\ref; use function Flow\ETL\DSL\row; +use function Flow\ETL\DSL\str_entry; use function Flow\ETL\DSL\sum; final class AggregatingFunctionMergeTest extends FlowTestCase @@ -36,6 +42,13 @@ public function test_average_merges_partial_sums_and_counts(): void self::assertSame(3, $left->result($context->entryFactory())->value()); } + public function test_average_rejects_a_different_scale(): void + { + $this->expectException(InvalidArgumentException::class); + + average(ref('v'), 2)->merge(average(ref('v'), 4)); + } + public function test_collect_merges_in_order(): void { $context = flow_context(); @@ -45,10 +58,66 @@ public function test_collect_merges_in_order(): void $right = collect(ref('v')); $right->aggregate(row(int_entry('v', 2)), $context); + $right->aggregate(row(int_entry('v', 3)), $context); $left->merge($right); - self::assertSame([1, 2], $left->result($context->entryFactory())->value()); + self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); + } + + public function test_collect_unique_merges_as_a_union(): void + { + $context = flow_context(); + + $left = collect_unique(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = collect_unique(ref('v')); + $right->aggregate(row(int_entry('v', 2)), $context); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); + } + + public function test_count_merges_counts(): void + { + $context = flow_context(); + + $left = count(ref('v')); + $left->aggregate(row(int_entry('v', 1)), $context); + $left->aggregate(row(int_entry('v', 2)), $context); + + $right = count(ref('v')); + $right->aggregate(row(int_entry('v', 3)), $context); + + $left->merge($right); + + self::assertSame(3, $left->result($context->entryFactory())->value()); + } + + public function test_count_without_reference_merges(): void + { + $context = flow_context(); + + $left = count(); + $left->aggregate(row(int_entry('v', 1)), $context); + + $right = count(); + $right->aggregate(row(int_entry('v', 2)), $context); + + $left->merge($right); + + self::assertSame(2, $left->result($context->entryFactory())->value()); + } + + public function test_count_rejects_a_referenced_and_unreferenced_mix(): void + { + $this->expectException(InvalidArgumentException::class); + + count(ref('v'))->merge(count()); } public function test_first_keeps_the_earlier_partial(): void @@ -66,6 +135,20 @@ public function test_first_keeps_the_earlier_partial(): void self::assertSame(10, $left->result($context->entryFactory())->value()); } + public function test_first_takes_the_other_when_empty(): void + { + $context = flow_context(); + + $left = first(ref('v')); + + $right = first(ref('v')); + $right->aggregate(row(int_entry('v', 20)), $context); + + $left->merge($right); + + self::assertSame(20, $left->result($context->entryFactory())->value()); + } + public function test_last_keeps_the_later_partial(): void { $context = flow_context(); @@ -81,6 +164,59 @@ public function test_last_keeps_the_later_partial(): void self::assertSame(20, $left->result($context->entryFactory())->value()); } + public function test_last_keeps_current_when_other_is_empty(): void + { + $context = flow_context(); + + $left = last(ref('v')); + $left->aggregate(row(int_entry('v', 10)), $context); + + $left->merge(last(ref('v'))); + + self::assertSame(10, $left->result($context->entryFactory())->value()); + } + + public function test_max_keeps_the_greatest(): void + { + $context = flow_context(); + + $left = max(ref('v')); + $left->aggregate(row(int_entry('v', 5)), $context); + + $right = max(ref('v')); + $right->aggregate(row(int_entry('v', 9)), $context); + + $left->merge($right); + + self::assertSame(9, $left->result($context->entryFactory())->value()); + } + + public function test_max_takes_the_other_when_empty(): void + { + $context = flow_context(); + + $left = max(ref('v')); + + $right = max(ref('v')); + $right->aggregate(row(int_entry('v', 9)), $context); + + $left->merge($right); + + self::assertSame(9, $left->result($context->entryFactory())->value()); + } + + public function test_max_keeps_current_when_other_is_empty(): void + { + $context = flow_context(); + + $left = max(ref('v')); + $left->aggregate(row(int_entry('v', 5)), $context); + + $left->merge(max(ref('v'))); + + self::assertSame(5, $left->result($context->entryFactory())->value()); + } + public function test_merging_a_different_function_type_throws(): void { $this->expectException(InvalidArgumentException::class); @@ -95,6 +231,70 @@ public function test_merging_the_same_function_on_a_different_reference_throws() sum(ref('a'))->merge(sum(ref('b'))); } + public function test_min_keeps_the_smallest(): void + { + $context = flow_context(); + + $left = min(ref('v')); + $left->aggregate(row(int_entry('v', 5)), $context); + + $right = min(ref('v')); + $right->aggregate(row(int_entry('v', 2)), $context); + + $left->merge($right); + + self::assertSame(2, $left->result($context->entryFactory())->value()); + } + + public function test_min_takes_the_other_when_empty(): void + { + $context = flow_context(); + + $left = min(ref('v')); + + $right = min(ref('v')); + $right->aggregate(row(int_entry('v', 2)), $context); + + $left->merge($right); + + self::assertSame(2, $left->result($context->entryFactory())->value()); + } + + public function test_min_keeps_current_when_other_is_empty(): void + { + $context = flow_context(); + + $left = min(ref('v')); + $left->aggregate(row(int_entry('v', 5)), $context); + + $left->merge(min(ref('v'))); + + self::assertSame(5, $left->result($context->entryFactory())->value()); + } + + public function test_string_aggregate_merges_values(): void + { + $context = flow_context(); + + $left = new StringAggregate(ref('v'), ',', SortOrder::ASC); + $left->aggregate(row(str_entry('v', 'b')), $context); + + $right = new StringAggregate(ref('v'), ',', SortOrder::ASC); + $right->aggregate(row(str_entry('v', 'a')), $context); + $right->aggregate(row(str_entry('v', 'c')), $context); + + $left->merge($right); + + self::assertSame('a,b,c', $left->result($context->entryFactory())->value()); + } + + public function test_string_aggregate_rejects_a_different_separator(): void + { + $this->expectException(InvalidArgumentException::class); + + (new StringAggregate(ref('v'), ','))->merge(new StringAggregate(ref('v'), ';')); + } + public function test_sum_merges_partial_sums(): void { $context = flow_context(); diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php new file mode 100644 index 0000000000..d8aa49159f --- /dev/null +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php @@ -0,0 +1,38 @@ +insert(new BucketGroup('b', 0, new GroupState([], []))); + $heap->insert(new BucketGroup('a', 2, new GroupState([], []))); + $heap->insert(new BucketGroup('a', 1, new GroupState([], []))); + + $order = []; + + while (!$heap->isEmpty()) { + $group = $heap->extract(); + $order[] = $group->hash . $group->bucketIndex; + } + + self::assertSame(['a1', 'a2', 'b0'], $order); + } + + public function test_rejects_a_non_bucket_group(): void + { + $this->expectException(InvalidArgumentException::class); + + (new GroupsMinHeap())->insert('not-a-bucket-group'); + } +} From 2f6072a719ab2390b2b8d2b9c40e3b559c234483 Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Tue, 7 Jul 2026 08:49:40 +0200 Subject: [PATCH 5/8] fix(flow-php/etl): resolve remaining static analysis findings - drop the unused mago pragma in Sum::merge - hoist the pivot prototype behind an instanceof guard so clone is never applied to a possibly-false current() - drop over-specific @return docblocks that toArray() cannot satisfy --- src/core/etl/src/Flow/ETL/Function/Sum.php | 1 - src/core/etl/src/Flow/ETL/GroupBy.php | 8 +++++++- .../Integration/DataFrame/GroupByAggregationTest.php | 6 ------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/core/etl/src/Flow/ETL/Function/Sum.php b/src/core/etl/src/Flow/ETL/Function/Sum.php index 4289b5263a..9addb4f814 100644 --- a/src/core/etl/src/Flow/ETL/Function/Sum.php +++ b/src/core/etl/src/Flow/ETL/Function/Sum.php @@ -74,7 +74,6 @@ public function merge(AggregatingFunction $other): void throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); } - // @mago-ignore analysis:possibly-invalid-argument $this->sum = (new Calculator())->add($this->sum, $other->sum); } diff --git a/src/core/etl/src/Flow/ETL/GroupBy.php b/src/core/etl/src/Flow/ETL/GroupBy.php index 7c816a3a1d..264fbb886e 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy.php +++ b/src/core/etl/src/Flow/ETL/GroupBy.php @@ -181,6 +181,12 @@ public function pivotResult(Generator $rows, FlowContext $context): Generator throw new RuntimeException('pivotResult() called without a pivot reference'); } + $aggregation = current($this->aggregations); + + if (!$aggregation instanceof AggregatingFunction) { + throw new RuntimeException('Pivot requires exactly one aggregation'); + } + /** @var array|bool|float|int|object|string> $pivotColumns */ $pivotColumns = []; /** @var array|bool|float|int|object|string>> $pivotedTable */ @@ -220,7 +226,7 @@ public function pivotResult(Generator $rows, FlowContext $context): Generator $pivotValue = type_union(type_string(), type_integer())->assert($pivotValue); if (!array_key_exists($pivotValue, $pivotedTable[$indexValue])) { - $pivotedTable[$indexValue][$pivotValue] = clone current($this->aggregations); + $pivotedTable[$indexValue][$pivotValue] = clone $aggregation; } $aggregator = $pivotedTable[$indexValue][$pivotValue]; diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php index 857c7edd72..631c975c34 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php @@ -100,17 +100,11 @@ public function test_seller_aggregation_is_identical_when_spilling(): void self::assertCount(5, $memory); } - /** - * @return array> - */ private function aggregateByEmail(ConfigBuilder $config): array { return df($config)->read(from_array($this->orders()))->groupBy('email')->aggregate(count())->fetch()->toArray(); } - /** - * @return array> - */ private function aggregateBySeller(ConfigBuilder $config): array { return df($config) From b854046a42fc79019ea7cebee5faf02443c3ef0c Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Tue, 7 Jul 2026 08:56:15 +0200 Subject: [PATCH 6/8] style(flow-php/etl): apply mago formatting --- src/core/etl/src/Flow/ETL/Function/Count.php | 2 +- .../Integration/DataFrame/GroupByAggregationTest.php | 6 +++--- .../GroupBy/FilesystemGroupBucketsCacheTest.php | 9 ++++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/core/etl/src/Flow/ETL/Function/Count.php b/src/core/etl/src/Flow/ETL/Function/Count.php index ddd9459dfc..8fc456391a 100644 --- a/src/core/etl/src/Flow/ETL/Function/Count.php +++ b/src/core/etl/src/Flow/ETL/Function/Count.php @@ -85,7 +85,7 @@ public function merge(AggregatingFunction $other): void if ( !$other instanceof self || ($this->ref === null) !== ($other->ref === null) - || ($this->ref !== null && $other->ref !== null && !$this->ref->is($other->ref)) + || $this->ref !== null && $other->ref !== null && !$this->ref->is($other->ref) ) { throw new InvalidArgumentException('Cannot merge ' . self::class . ' aggregating a different reference'); } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php index 631c975c34..6e1e837b41 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php @@ -34,10 +34,10 @@ public function test_first_and_last_preserve_input_order_across_spilled_buckets( $data = []; for ($i = 0; $i < 400; $i++) { - $data[] = ['g' => $i % 2 === 0 ? 'a' : 'b', 'v' => $i]; + $data[] = ['g' => ($i % 2) === 0 ? 'a' : 'b', 'v' => $i]; } - $run = static fn (ConfigBuilder $config): array => df($config) + $run = static fn(ConfigBuilder $config): array => df($config) ->read(from_array($data)) ->groupBy('g') ->aggregate(first(ref('v')), last(ref('v'))) @@ -74,7 +74,7 @@ public function test_multi_column_keys_do_not_collide(): void ['x' => 'a', 'y' => 'bc'], ]; - $run = static fn (ConfigBuilder $config): array => df($config) + $run = static fn(ConfigBuilder $config): array => df($config) ->read(from_array($data)) ->groupBy('x', 'y') ->aggregate(count()) diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php index 4358b59b28..ecafda4acb 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php @@ -31,9 +31,12 @@ public function test_round_trip_and_remove(): void ['ccc', new GroupState(['type' => 'c'], [sum(ref('v'))])], ]; - $cache->set('bucket-1', (static function () use ($groups) { - yield from $groups; - })()); + $cache->set( + 'bucket-1', + (static function () use ($groups) { + yield from $groups; + })(), + ); $read = []; From a59d0feaff52666911f39ee2acaf4dd92aa2ff50 Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Tue, 7 Jul 2026 09:02:19 +0200 Subject: [PATCH 7/8] style(flow-php/etl): use static:: assertions in new aggregation tests --- .../DataFrame/GroupByAggregationTest.php | 14 ++++---- .../FilesystemGroupBucketsCacheTest.php | 6 ++-- .../AggregationConfigBuilderTest.php | 10 +++--- .../Function/AggregatingFunctionMergeTest.php | 34 +++++++++---------- .../Tests/Unit/GroupBy/GroupsMinHeapTest.php | 2 +- 5 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php index 6e1e837b41..87ec052ccb 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/DataFrame/GroupByAggregationTest.php @@ -48,8 +48,8 @@ public function test_first_and_last_preserve_input_order_across_spilled_buckets( $memory = $run(config_builder()); $spilled = $run(config_builder()->aggregationMemoryLimit(Unit::fromBytes(1))); - self::assertSame($memory, $spilled); - self::assertSame( + static::assertSame($memory, $spilled); + static::assertSame( [ ['g' => 'a', 'v_first' => 0, 'v_last' => 398], ['g' => 'b', 'v_first' => 1, 'v_last' => 399], @@ -63,7 +63,7 @@ public function test_high_cardinality_group_by_is_identical_when_spilling(): voi $memory = $this->aggregateByEmail(config_builder()); $spilled = $this->aggregateByEmail(config_builder()->aggregationMemoryLimit(Unit::fromKb(256))); - self::assertEqualsCanonicalizing($memory, $spilled); + static::assertEqualsCanonicalizing($memory, $spilled); } public function test_multi_column_keys_do_not_collide(): void @@ -87,8 +87,8 @@ public function test_multi_column_keys_do_not_collide(): void ['x' => 'ab', 'y' => 'c', '_count' => 1], ]; - self::assertSame($expected, $run(config_builder())); - self::assertSame($expected, $run(config_builder()->aggregationMemoryLimit(Unit::fromBytes(1)))); + static::assertSame($expected, $run(config_builder())); + static::assertSame($expected, $run(config_builder()->aggregationMemoryLimit(Unit::fromBytes(1)))); } public function test_seller_aggregation_is_identical_when_spilling(): void @@ -96,8 +96,8 @@ public function test_seller_aggregation_is_identical_when_spilling(): void $memory = $this->aggregateBySeller(config_builder()); $spilled = $this->aggregateBySeller(config_builder()->aggregationMemoryLimit(Unit::fromKb(256))); - self::assertEqualsCanonicalizing($memory, $spilled); - self::assertCount(5, $memory); + static::assertEqualsCanonicalizing($memory, $spilled); + static::assertCount(5, $memory); } private function aggregateByEmail(ConfigBuilder $config): array diff --git a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php index ecafda4acb..2cc4628b0c 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Integration/GroupBy/FilesystemGroupBucketsCacheTest.php @@ -18,7 +18,7 @@ public function test_missing_bucket_yields_nothing(): void { $cache = new FilesystemGroupBucketsCache($this->fs(), $this->serializer(), 100, $this->cacheDir); - self::assertSame([], iterator_to_array($cache->get('does-not-exist'))); + static::assertSame([], iterator_to_array($cache->get('does-not-exist'))); } public function test_round_trip_and_remove(): void @@ -44,10 +44,10 @@ public function test_round_trip_and_remove(): void $read[$hash] = $state->keyValues; } - self::assertSame(['aaa' => ['type' => 'a'], 'bbb' => ['type' => 'b'], 'ccc' => ['type' => 'c']], $read); + static::assertSame(['aaa' => ['type' => 'a'], 'bbb' => ['type' => 'b'], 'ccc' => ['type' => 'c']], $read); $cache->remove('bucket-1'); - self::assertSame([], iterator_to_array($cache->get('bucket-1'))); + static::assertSame([], iterator_to_array($cache->get('bucket-1'))); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php index 1dc0315a04..3dd6226210 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Config/Aggregation/AggregationConfigBuilderTest.php @@ -19,8 +19,8 @@ public function test_default_memory_limit_is_unbounded(): void { $config = (new AggregationConfigBuilder())->build(); - self::assertSame(PHP_INT_MAX, $config->memoryLimit->inBytes()); - self::assertSame('file', $config->filesystemProtocol); + static::assertSame(PHP_INT_MAX, $config->memoryLimit->inBytes()); + static::assertSame('file', $config->filesystemProtocol); } public function test_memory_limit_is_read_from_the_environment(): void @@ -30,7 +30,7 @@ public function test_memory_limit_is_read_from_the_environment(): void try { $config = (new AggregationConfigBuilder())->build(); - self::assertSame(Unit::fromMb(32)->inBytes(), $config->memoryLimit->inBytes()); + static::assertSame(Unit::fromMb(32)->inBytes(), $config->memoryLimit->inBytes()); } finally { putenv(AggregationConfig::AGGREGATION_MAX_MEMORY_ENV); } @@ -43,7 +43,7 @@ public function test_setting_a_memory_limit_and_filesystem(): void ->filesystemProtocol('memory') ->build(); - self::assertSame(Unit::fromMb(16)->inBytes(), $config->memoryLimit->inBytes()); - self::assertSame('memory', $config->filesystemProtocol); + static::assertSame(Unit::fromMb(16)->inBytes(), $config->memoryLimit->inBytes()); + static::assertSame('memory', $config->filesystemProtocol); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php index 6e0c564827..1c3b07defe 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/AggregatingFunctionMergeTest.php @@ -39,7 +39,7 @@ public function test_average_merges_partial_sums_and_counts(): void $left->merge($right); - self::assertSame(3, $left->result($context->entryFactory())->value()); + static::assertSame(3, $left->result($context->entryFactory())->value()); } public function test_average_rejects_a_different_scale(): void @@ -62,7 +62,7 @@ public function test_collect_merges_in_order(): void $left->merge($right); - self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); + static::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); } public function test_collect_unique_merges_as_a_union(): void @@ -79,7 +79,7 @@ public function test_collect_unique_merges_as_a_union(): void $left->merge($right); - self::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); + static::assertSame([1, 2, 3], $left->result($context->entryFactory())->value()); } public function test_count_merges_counts(): void @@ -95,7 +95,7 @@ public function test_count_merges_counts(): void $left->merge($right); - self::assertSame(3, $left->result($context->entryFactory())->value()); + static::assertSame(3, $left->result($context->entryFactory())->value()); } public function test_count_without_reference_merges(): void @@ -110,7 +110,7 @@ public function test_count_without_reference_merges(): void $left->merge($right); - self::assertSame(2, $left->result($context->entryFactory())->value()); + static::assertSame(2, $left->result($context->entryFactory())->value()); } public function test_count_rejects_a_referenced_and_unreferenced_mix(): void @@ -132,7 +132,7 @@ public function test_first_keeps_the_earlier_partial(): void $left->merge($right); - self::assertSame(10, $left->result($context->entryFactory())->value()); + static::assertSame(10, $left->result($context->entryFactory())->value()); } public function test_first_takes_the_other_when_empty(): void @@ -146,7 +146,7 @@ public function test_first_takes_the_other_when_empty(): void $left->merge($right); - self::assertSame(20, $left->result($context->entryFactory())->value()); + static::assertSame(20, $left->result($context->entryFactory())->value()); } public function test_last_keeps_the_later_partial(): void @@ -161,7 +161,7 @@ public function test_last_keeps_the_later_partial(): void $left->merge($right); - self::assertSame(20, $left->result($context->entryFactory())->value()); + static::assertSame(20, $left->result($context->entryFactory())->value()); } public function test_last_keeps_current_when_other_is_empty(): void @@ -173,7 +173,7 @@ public function test_last_keeps_current_when_other_is_empty(): void $left->merge(last(ref('v'))); - self::assertSame(10, $left->result($context->entryFactory())->value()); + static::assertSame(10, $left->result($context->entryFactory())->value()); } public function test_max_keeps_the_greatest(): void @@ -188,7 +188,7 @@ public function test_max_keeps_the_greatest(): void $left->merge($right); - self::assertSame(9, $left->result($context->entryFactory())->value()); + static::assertSame(9, $left->result($context->entryFactory())->value()); } public function test_max_takes_the_other_when_empty(): void @@ -202,7 +202,7 @@ public function test_max_takes_the_other_when_empty(): void $left->merge($right); - self::assertSame(9, $left->result($context->entryFactory())->value()); + static::assertSame(9, $left->result($context->entryFactory())->value()); } public function test_max_keeps_current_when_other_is_empty(): void @@ -214,7 +214,7 @@ public function test_max_keeps_current_when_other_is_empty(): void $left->merge(max(ref('v'))); - self::assertSame(5, $left->result($context->entryFactory())->value()); + static::assertSame(5, $left->result($context->entryFactory())->value()); } public function test_merging_a_different_function_type_throws(): void @@ -243,7 +243,7 @@ public function test_min_keeps_the_smallest(): void $left->merge($right); - self::assertSame(2, $left->result($context->entryFactory())->value()); + static::assertSame(2, $left->result($context->entryFactory())->value()); } public function test_min_takes_the_other_when_empty(): void @@ -257,7 +257,7 @@ public function test_min_takes_the_other_when_empty(): void $left->merge($right); - self::assertSame(2, $left->result($context->entryFactory())->value()); + static::assertSame(2, $left->result($context->entryFactory())->value()); } public function test_min_keeps_current_when_other_is_empty(): void @@ -269,7 +269,7 @@ public function test_min_keeps_current_when_other_is_empty(): void $left->merge(min(ref('v'))); - self::assertSame(5, $left->result($context->entryFactory())->value()); + static::assertSame(5, $left->result($context->entryFactory())->value()); } public function test_string_aggregate_merges_values(): void @@ -285,7 +285,7 @@ public function test_string_aggregate_merges_values(): void $left->merge($right); - self::assertSame('a,b,c', $left->result($context->entryFactory())->value()); + static::assertSame('a,b,c', $left->result($context->entryFactory())->value()); } public function test_string_aggregate_rejects_a_different_separator(): void @@ -308,6 +308,6 @@ public function test_sum_merges_partial_sums(): void $left->merge($right); - self::assertSame(6, $left->result($context->entryFactory())->value()); + static::assertSame(6, $left->result($context->entryFactory())->value()); } } diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php index d8aa49159f..5fe062c8cc 100644 --- a/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php +++ b/src/core/etl/tests/Flow/ETL/Tests/Unit/GroupBy/GroupsMinHeapTest.php @@ -26,7 +26,7 @@ public function test_orders_by_hash_then_bucket_index(): void $order[] = $group->hash . $group->bucketIndex; } - self::assertSame(['a1', 'a2', 'b0'], $order); + static::assertSame(['a1', 'a2', 'b0'], $order); } public function test_rejects_a_non_bucket_group(): void From 69df77e74b736907b9e451eb5f7190b9c1fe960d Mon Sep 17 00:00:00 2001 From: Aleksander Kowalski Date: Tue, 7 Jul 2026 12:13:37 +0200 Subject: [PATCH 8/8] fix: --- .../BucketsCache/FilesystemGroupBucketsCache.php | 9 +++++---- .../src/Flow/ETL/GroupBy/GroupBucketsCache.php | 4 ++-- .../etl/src/Flow/ETL/GroupBy/HashAggregation.php | 14 +++++++------- .../etl/src/Flow/ETL/GroupBy/HashedGroup.php | 16 ++++++++++++++++ .../GroupBy/FilesystemGroupBucketsCacheTest.php | 11 ++++++----- 5 files changed, 36 insertions(+), 18 deletions(-) create mode 100644 src/core/etl/src/Flow/ETL/GroupBy/HashedGroup.php diff --git a/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php index 892ba2e276..3e35c3897e 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php +++ b/src/core/etl/src/Flow/ETL/GroupBy/BucketsCache/FilesystemGroupBucketsCache.php @@ -6,6 +6,7 @@ use Flow\ETL\GroupBy\GroupBucketsCache; use Flow\ETL\GroupBy\GroupState; +use Flow\ETL\GroupBy\HashedGroup; use Flow\ETL\Hash\NativePHPHash; use Flow\Filesystem\Filesystem; use Flow\Filesystem\Path; @@ -33,7 +34,7 @@ public function __construct( } /** - * @return Generator + * @return Generator */ public function get(string $bucketId): Generator { @@ -55,7 +56,7 @@ public function get(string $bucketId): Generator $hash = substr($line, 0, $separator); $serialized = substr($line, $separator + 1); - yield [$hash, $this->serializer->unserialize($serialized, [GroupState::class])]; + yield new HashedGroup($hash, $this->serializer->unserialize($serialized, [GroupState::class])); } $stream->close(); @@ -73,8 +74,8 @@ public function set(string $bucketId, iterable $groups): void $buffer = ''; $counter = 0; - foreach ($groups as [$hash, $state]) { - $buffer .= $hash . ' ' . $this->serializer->serialize($state) . "\n"; + foreach ($groups as $group) { + $buffer .= $group->hash . ' ' . $this->serializer->serialize($group->state) . "\n"; $counter++; if ($counter >= $this->chunkSize) { diff --git a/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php b/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php index 40c329a3c9..0db68173c1 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php +++ b/src/core/etl/src/Flow/ETL/GroupBy/GroupBucketsCache.php @@ -12,14 +12,14 @@ interface GroupBucketsCache { /** - * @return Generator + * @return Generator */ public function get(string $bucketId): Generator; public function remove(string $bucketId): void; /** - * @param iterable $groups + * @param iterable $groups */ public function set(string $bucketId, iterable $groups): void; } diff --git a/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php b/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php index 06d265fade..1519c1799c 100644 --- a/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php +++ b/src/core/etl/src/Flow/ETL/GroupBy/HashAggregation.php @@ -113,7 +113,7 @@ private function merge(array $buckets, array $hot): Generator { ksort($hot); - /** @var array> $cursors */ + /** @var array> $cursors */ $cursors = []; foreach ($buckets as $index => $bucketId) { @@ -152,7 +152,7 @@ private function merge(array $buckets, array $hot): Generator } /** - * @param Generator $cursor + * @param Generator $cursor */ private function advance(Generator $cursor, int $bucketIndex, GroupsMinHeap $heap): void { @@ -160,21 +160,21 @@ private function advance(Generator $cursor, int $bucketIndex, GroupsMinHeap $hea return; } - /** @var array{0: string, 1: GroupState} $item */ - $item = $cursor->current(); - $heap->insert(new BucketGroup($item[0], $bucketIndex, $item[1])); + /** @var HashedGroup $group */ + $group = $cursor->current(); + $heap->insert(new BucketGroup($group->hash, $bucketIndex, $group->state)); $cursor->next(); } /** * @param array $hot * - * @return Generator + * @return Generator */ private function hotCursor(array $hot): Generator { foreach ($hot as $hash => $state) { - yield [$hash, $state]; + yield new HashedGroup($hash, $state); } } diff --git a/src/core/etl/src/Flow/ETL/GroupBy/HashedGroup.php b/src/core/etl/src/Flow/ETL/GroupBy/HashedGroup.php new file mode 100644 index 0000000000..15830f06aa --- /dev/null +++ b/src/core/etl/src/Flow/ETL/GroupBy/HashedGroup.php @@ -0,0 +1,16 @@ +fs(), $this->serializer(), 2, $this->cacheDir); $groups = [ - ['aaa', new GroupState(['type' => 'a'], [sum(ref('v'))])], - ['bbb', new GroupState(['type' => 'b'], [sum(ref('v'))])], - ['ccc', new GroupState(['type' => 'c'], [sum(ref('v'))])], + new HashedGroup('aaa', new GroupState(['type' => 'a'], [sum(ref('v'))])), + new HashedGroup('bbb', new GroupState(['type' => 'b'], [sum(ref('v'))])), + new HashedGroup('ccc', new GroupState(['type' => 'c'], [sum(ref('v'))])), ]; $cache->set( @@ -40,8 +41,8 @@ public function test_round_trip_and_remove(): void $read = []; - foreach ($cache->get('bucket-1') as [$hash, $state]) { - $read[$hash] = $state->keyValues; + foreach ($cache->get('bucket-1') as $group) { + $read[$group->hash] = $group->state->keyValues; } static::assertSame(['aaa' => ['type' => 'a'], 'bbb' => ['type' => 'b'], 'ccc' => ['type' => 'c']], $read);