Skip to content

Commit 410c7ff

Browse files
KocKoc
committed
Fix: Large table causes sql error (cache cells)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent 662f9c4 commit 410c7ff

7 files changed

Lines changed: 161 additions & 15 deletions

File tree

‎appinfo/info.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ Have a good time and manage whatever you want.
5757
<post-migration>
5858
<step>OCA\Tables\Migration\NewDbStructureRepairStep</step>
5959
<step>OCA\Tables\Migration\DbRowSleeveSequence</step>
60+
<step>OCA\Tables\Migration\CacheSleeveCells</step>
6061
</post-migration>
6162
</repair-steps>
6263
<commands>

‎lib/Db/Row2Mapper.php‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -733,9 +733,11 @@ public function insert(Row2 $row): Row2 {
733733
}
734734

735735
// write all cells to its db-table
736+
$cells = [];
736737
foreach ($row->getData() as $cell) {
737-
$this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy());
738+
$cells[$cell['columnId']] = $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy());
738739
}
740+
$rowSleeve->setCells(json_encode($cells));
739741

740742
return $row;
741743
}
@@ -752,9 +754,9 @@ public function update(Row2 $row): Row2 {
752754

753755
// update meta data for sleeve
754756
try {
755-
$sleeve = $this->rowSleeveMapper->find($row->getId());
756-
$this->updateMetaData($sleeve);
757-
$this->rowSleeveMapper->update($sleeve);
757+
$rowSleeve = $this->rowSleeveMapper->find($row->getId());
758+
$this->updateMetaData($rowSleeve);
759+
$this->rowSleeveMapper->update($rowSleeve);
758760
} catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) {
759761
$this->logger->error($e->getMessage(), ['exception' => $e]);
760762
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
@@ -763,9 +765,11 @@ public function update(Row2 $row): Row2 {
763765
$this->columnMapper->preloadColumns(array_column($changedCells, 'columnId'));
764766

765767
// write all changed cells to its db-table
768+
$cachedCells = $rowSleeve->getCachedCellsArray();
766769
foreach ($changedCells as $cell) {
767-
$this->insertOrUpdateCell($sleeve->getId(), $cell['columnId'], $cell['value']);
770+
$cachedCells[$cell['columnId']] = $this->insertOrUpdateCell($rowSleeve->getId(), $cell['columnId'], $cell['value']);
768771
}
772+
$rowSleeve->setCachedCells(json_encode($cachedCells));
769773

770774
return $row;
771775
}
@@ -817,9 +821,11 @@ private function updateMetaData($entity, bool $setCreate = false, ?string $lastE
817821
/**
818822
* Insert a cell to its specific db-table
819823
*
824+
* @return array<string, mixed> normalized cell data
825+
*
820826
* @throws InternalError
821827
*/
822-
private function insertCell(int $rowId, int $columnId, $value, ?string $lastEditAt = null, ?string $lastEditBy = null): void {
828+
private function insertCell(int $rowId, int $columnId, $value, ?string $lastEditAt = null, ?string $lastEditBy = null): array {
823829
try {
824830
$column = $this->columnMapper->find($columnId);
825831
} catch (DoesNotExistException $e) {
@@ -829,7 +835,7 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit
829835

830836
// insert new cell
831837
$cellMapper = $this->getCellMapper($column);
832-
838+
$cachedCell = [];
833839
try {
834840
$cellClassName = 'OCA\Tables\Db\RowCell' . ucfirst($column->getType());
835841
if ($cellMapper->hasMultipleValues()) {
@@ -841,6 +847,7 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit
841847
$this->updateMetaData($cell, false, $lastEditAt, $lastEditBy);
842848
$cellMapper->applyDataToEntity($column, $cell, $val);
843849
$cellMapper->insert($cell);
850+
$cachedCell[] = $cellMapper->toArray($cell);
844851
}
845852
} else {
846853
/** @var RowCellSuper $cell */
@@ -850,50 +857,62 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit
850857
$this->updateMetaData($cell, false, $lastEditAt, $lastEditBy);
851858
$cellMapper->applyDataToEntity($column, $cell, $value);
852859
$cellMapper->insert($cell);
860+
$cachedCell = $cellMapper->toArray($cell);
853861
}
854862
} catch (Exception $e) {
855863
$this->logger->error($e->getMessage(), ['exception' => $e]);
856864
throw new InternalError('Failed to insert column: ' . $e->getMessage(), 0, $e);
857865
}
866+
867+
return $cachedCell;
858868
}
859869

860870
/**
861871
* @param RowCellSuper $cell
862872
* @param RowCellMapperSuper $mapper
863873
* @param mixed $value the value should be parsed to the correct format within the row service
864874
* @param Column $column
875+
*
876+
* @return array<string, mixed> normalized cell data
865877
* @throws InternalError
866878
*/
867-
private function updateCell(RowCellSuper $cell, RowCellMapperSuper $mapper, $value, Column $column): void {
868-
$this->getCellMapper($column)->applyDataToEntity($column, $cell, $value);
879+
private function updateCell(RowCellSuper $cell, RowCellMapperSuper $mapper, $value, Column $column): array {
880+
$cellMapper = $this->getCellMapper($column);
881+
$cellMapper->applyDataToEntity($column, $cell, $value);
869882
$this->updateMetaData($cell);
870883
$mapper->updateWrapper($cell);
884+
885+
return $cellMapper->toArray($cell);
871886
}
872887

873888
/**
889+
* @return array<string, mixed> normalized cell data
874890
* @throws InternalError
875891
*/
876-
private function insertOrUpdateCell(int $rowId, int $columnId, $value): void {
892+
private function insertOrUpdateCell(int $rowId, int $columnId, $value): array {
877893
$column = $this->columnMapper->find($columnId);
878894
$cellMapper = $this->getCellMapper($column);
895+
$cachedCell = [];
879896
try {
880897
if ($cellMapper->hasMultipleValues()) {
881-
$this->atomic(function () use ($cellMapper, $rowId, $columnId, $value) {
898+
$this->atomic(function () use ($cellMapper, $rowId, $columnId, $value, &$cachedCell) {
882899
// For a usergroup field with mutiple values, each is inserted as a new cell
883900
// we need to delete all previous cells for this row and column, otherwise we get duplicates
884901
$cellMapper->deleteAllForColumnAndRow($columnId, $rowId);
885-
$this->insertCell($rowId, $columnId, $value);
902+
$cachedCell = $this->insertCell($rowId, $columnId, $value);
886903
}, $this->db);
887904
} else {
888905
$cell = $cellMapper->findByRowAndColumn($rowId, $columnId);
889-
$this->updateCell($cell, $cellMapper, $value, $column);
906+
$cachedCell = $this->updateCell($cell, $cellMapper, $value, $column);
890907
}
891908
} catch (DoesNotExistException) {
892-
$this->insertCell($rowId, $columnId, $value);
909+
$cachedCell = $this->insertCell($rowId, $columnId, $value);
893910
} catch (MultipleObjectsReturnedException|Exception $e) {
894911
$this->logger->error($e->getMessage(), ['exception' => $e]);
895-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
912+
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), $e->getCode(), $e);
896913
}
914+
915+
return $cachedCell;
897916
}
898917

899918
private function getCellMapper(Column $column): RowCellMapperSuper {

‎lib/Db/RowCellMapperSuper.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ public function applyDataToEntity(Column $column, RowCellSuper $cell, $data): vo
4949
$cell->setValue($data);
5050
}
5151

52+
public function toArray(RowCellSuper $cell): array {
53+
return ['value' => $cell->getValue()];
54+
}
55+
5256
public function getDbParamType() {
5357
return IQueryBuilder::PARAM_STR;
5458
}

‎lib/Db/RowCellUsergroupMapper.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ public function formatEntity(Column $column, RowCellSuper $cell) {
5656
];
5757
}
5858

59+
public function toArray(RowCellSuper $cell): array {
60+
return [
61+
'value' => $cell->getValue(),
62+
'value_type' => $cell->getValueType(),
63+
];
64+
}
65+
5966
public function hasMultipleValues(): bool {
6067
return true;
6168
}

‎lib/Db/RowSleeve.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
* @psalm-suppress PropertyNotSetInConstructor
1616
* @method getTableId(): ?int
1717
* @method setTableId(int $columnId)
18+
* @method getCachedCells(): string
19+
* @method setCachedCells(string $cachedCells)
1820
* @method getCreatedBy(): string
1921
* @method setCreatedBy(string $createdBy)
2022
* @method getCreatedAt(): string
@@ -26,6 +28,7 @@
2628
*/
2729
class RowSleeve extends Entity implements JsonSerializable {
2830
protected ?int $tableId = null;
31+
protected ?string $cachedCells = null;
2932
protected ?string $createdBy = null;
3033
protected ?string $createdAt = null;
3134
protected ?string $lastEditBy = null;
@@ -34,12 +37,21 @@ class RowSleeve extends Entity implements JsonSerializable {
3437
public function __construct() {
3538
$this->addType('id', 'integer');
3639
$this->addType('tableId', 'integer');
40+
$this->addType('cachedCells', 'string');
41+
}
42+
43+
/**
44+
* @return array<int, mixed> Indexed by column ID
45+
*/
46+
public function getCachedCellsArray(): array {
47+
return json_decode($this->cachedCells, true) ?: [];
3748
}
3849

3950
public function jsonSerialize(): array {
4051
return [
4152
'id' => $this->id,
4253
'tableId' => $this->tableId,
54+
'cachedCells' => $this->cachedCells,
4355
'createdBy' => $this->createdBy,
4456
'createdAt' => $this->createdAt,
4557
'lastEditBy' => $this->lastEditBy,

‎lib/Migration/CacheSleeveCells.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
namespace OCA\Tables\Migration;
9+
10+
use OCA\Tables\Db\Row2Mapper;
11+
use OCP\IConfig;
12+
use OCP\IDBConnection;
13+
use OCP\Migration\IOutput;
14+
use OCP\Migration\IRepairStep;
15+
use Psr\Log\LoggerInterface;
16+
17+
class CacheSleeveCells implements IRepairStep {
18+
public function __construct(
19+
protected IDBConnection $db,
20+
protected Row2Mapper $rowMapper,
21+
protected IConfig $config,
22+
protected LoggerInterface $logger,
23+
) {
24+
}
25+
26+
/**
27+
* @inheritDoc
28+
*/
29+
public function getName() {
30+
return 'Caches cells to the row-sleeves table';
31+
}
32+
33+
/**
34+
* @inheritDoc
35+
*/
36+
public function run(IOutput $output) {
37+
$cachingSleeveCellsComplete = $this->config->getAppValue('tables', 'cachingSleeveCellsComplete', 'false') === 'true';
38+
if (!$cachingSleeveCellsComplete) {
39+
return;
40+
}
41+
42+
// fixme: get all tables
43+
// get columns for each tables
44+
// for each column call Row2Mapper::getRows
45+
while ($sleeveToProcess = $this->getSleeveToProcess()) {
46+
$rowIds = array_column($sleeveToProcess, 'id');
47+
}
48+
49+
$this->config->setAppValue('tables', 'cachingSleeveCellsComplete', 'true');
50+
}
51+
52+
private function getSleeveToProcess(): array
53+
{
54+
$qb = $this->db->getQueryBuilder();
55+
56+
return $qb->select('*')
57+
->from('tables_row_sleeves')
58+
->where($qb->expr()->isNull('cached_cells'))
59+
->setMaxResults(1000)
60+
->executeQuery()
61+
->fetchAll();
62+
}
63+
}

‎lib/Migration/Version001010Date20251229000000.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/** @noinspection PhpUnused */
4+
5+
declare(strict_types=1);
6+
/**
7+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
8+
* SPDX-License-Identifier: AGPL-3.0-or-later
9+
*/
10+
namespace OCA\Tables\Migration;
11+
12+
use Closure;
13+
use OCP\DB\Exception;
14+
use OCP\DB\ISchemaWrapper;
15+
use OCP\DB\Types;
16+
use OCP\Migration\IOutput;
17+
use OCP\Migration\SimpleMigrationStep;
18+
19+
class Version001010Date20251229000000 extends SimpleMigrationStep {
20+
/**
21+
* @param IOutput $output
22+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
23+
* @param array $options
24+
* @return null|ISchemaWrapper
25+
* @throws Exception
26+
*/
27+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
28+
/** @var ISchemaWrapper $schema */
29+
$schema = $schemaClosure();
30+
31+
$table = $schema->getTable('tables_row_sleeves');
32+
if (!$table->hasColumn('tables_row_sleeves')) {
33+
$table->addColumn('cells', Types::JSON, [
34+
'notnull' => false,
35+
]);
36+
}
37+
38+
return $schema;
39+
}
40+
}

0 commit comments

Comments
 (0)