Skip to content

Commit

Permalink
work around group_concat max len being too small edge case for the se…
Browse files Browse the repository at this point in the history 
…lect in ArchiveSelector.getArchiveIds (#17496)
  • Loading branch information
@diosmosis
diosmosis authored Apr 26, 2021
1 parent 634a9a5 commit 88faa0a
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 2 deletions.
24 changes: 22 additions & 2 deletions core/DataAccess/ArchiveSelector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
use Piwik\ArchiveProcessor;
use Piwik\ArchiveProcessor\Rules;
use Piwik\Common;
use Piwik\Container\StaticContainer;
use Piwik\Date;
use Piwik\Db;
use Piwik\Period;
use Piwik\Period\Range;
use Piwik\Segment;
use Piwik\SettingsServer;
use Psr\Log\LoggerInterface;

/**
* Data Access object used to query archives
Expand Down Expand Up @@ -129,6 +131,7 @@ public static function getArchiveIdAndVisits(ArchiveProcessor\Parameters $params
* @param Segment $segment
* @param string[] $plugins List of plugin names for which data is being requested.
* @param bool $includeInvalidated true to include archives that are DONE_INVALIDATED, false if only DONE_OK.
* @param bool $_skipSetGroupConcatMaxLen for tests
* @return array Archive IDs are grouped by archive name and period range, ie,
* array(
* 'VisitsSummary.done' => array(
Expand All @@ -137,8 +140,17 @@ public static function getArchiveIdAndVisits(ArchiveProcessor\Parameters $params
* )
* @throws
*/
public static function getArchiveIds($siteIds, $periods, $segment, $plugins, $includeInvalidated = true)
public static function getArchiveIds($siteIds, $periods, $segment, $plugins, $includeInvalidated = true, $_skipSetGroupConcatMaxLen = false)
{
$logger = StaticContainer::get(LoggerInterface::class);
if (!$_skipSetGroupConcatMaxLen) {
try {
Db::get()->query('SET SESSION group_concat_max_len=' . (128 * 1024));
} catch (\Exception $ex) {
$logger->info("Could not set group_concat_max_len MySQL session variable.");
}
}

if (empty($siteIds)) {
throw new \Exception("Website IDs could not be read from the request, ie. idSite=");
}
Expand Down Expand Up @@ -208,7 +220,15 @@ public static function getArchiveIds($siteIds, $periods, $segment, $plugins, $in
$archives = $row['archives'];
$pairs = explode(',', $archives);
foreach ($pairs as $pair) {
list($idarchive, $doneFlag, $value) = explode('|', $pair);
$parts = explode('|', $pair);
if (count($parts) != 3) { // GROUP_CONCAT got cut off, have to ignore the rest
// note: in this edge case, we end up not selecting the all plugins archive because it will be older than the partials.
// not ideal, but it avoids an exception.
$logger->info("GROUP_CONCAT got cut off in ArchiveSelector." . __FUNCTION__ . ' for idsite = ' . $row['idsite'] . ', period = ' . $dateStr);
continue;
}

list($idarchive, $doneFlag, $value) = $parts;

$result[$doneFlag][$dateStr][] = $idarchive;
if (strpos($doneFlag, '.') === false // all plugins archive
Expand Down
38 changes: 38 additions & 0 deletions tests/PHPUnit/Integration/DataAccess/ArchiveSelectorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,44 @@ protected static function configureFixture($fixture)
$fixture->createSuperUser = true;
}

public function test_getArchiveIds_handlesCutOffGroupConcat()
{
Db::get()->query('SET SESSION group_concat_max_len=' . 20);

$archiveRows = [
['idarchive' => 1, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done', 'value' => 1],
['idarchive' => 2, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 3, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 4, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 5, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 6, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 7, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 8, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 9, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 10, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 11, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 12, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 13, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 14, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 15, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
['idarchive' => 16, 'idsite' => 1, 'period' => 1, 'date1' => '2020-03-01', 'date2' => '2020-03-01', 'name' => 'done.Funnels', 'value' => 5],
];

$this->insertArchiveData($archiveRows);

$archiveIds = ArchiveSelector::getArchiveIds([1], [Factory::build('day', '2020-03-01')], new Segment('', [1]), ['Funnels'],
true, true);

$expected = [
'done.Funnels' => [
'2020-03-01,2020-03-01' => [
'16',
],
],
];
$this->assertEquals($expected, $archiveIds);
}

/**
* @dataProvider getTestDataForGetArchiveIds
*/
Expand Down

0 comments on commit 88faa0a

Please sign in to comment.