Skip to content

Commit d12753a

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

File tree

7 files changed

+304
-162
lines changed

7 files changed

+304
-162
lines changed

‎lib/Db/Row2Mapper.php‎

Lines changed: 22 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
use DateTime;
1111
use DateTimeImmutable;
1212
use OCA\Tables\Constants\UsergroupType;
13+
use OCA\Tables\Db\RowLoader\CachedRowLoader;
14+
use OCA\Tables\Db\RowLoader\NormalizedRowLoader;
1315
use OCA\Tables\Errors\InternalError;
1416
use OCA\Tables\Errors\NotFoundError;
1517
use OCA\Tables\Helper\ColumnsHelper;
@@ -18,18 +20,12 @@
1820
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
1921
use OCP\AppFramework\Db\TTransactional;
2022
use OCP\DB\Exception;
21-
use OCP\DB\IResult;
2223
use OCP\DB\QueryBuilder\IQueryBuilder;
2324
use OCP\IDBConnection;
24-
use OCP\Server;
25-
use Psr\Container\ContainerExceptionInterface;
26-
use Psr\Container\NotFoundExceptionInterface;
2725
use Psr\Log\LoggerInterface;
2826
use Throwable;
2927

3028
class Row2Mapper {
31-
private const MAX_DB_PARAMETERS = 65535;
32-
3329
use TTransactional;
3430

3531
private RowSleeveMapper $rowSleeveMapper;
@@ -40,15 +36,26 @@ class Row2Mapper {
4036
protected ColumnMapper $columnMapper;
4137

4238
private ColumnsHelper $columnsHelper;
39+
/**
40+
* @var array<RowLoader\RowLoader::LOADER_*, RowLoader\RowLoader>
41+
*/
42+
private array $rowLoaders;
4343

44-
public function __construct(?string $userId, IDBConnection $db, LoggerInterface $logger, UserHelper $userHelper, RowSleeveMapper $rowSleeveMapper, ColumnsHelper $columnsHelper, ColumnMapper $columnMapper) {
44+
public function __construct(?string $userId, IDBConnection $db, LoggerInterface $logger, UserHelper $userHelper, RowSleeveMapper $rowSleeveMapper, ColumnsHelper $columnsHelper, ColumnMapper $columnMapper,
45+
NormalizedRowLoader $normalizedRowLoader,
46+
CachedRowLoader $cachedRowLoader,
47+
) {
4548
$this->rowSleeveMapper = $rowSleeveMapper;
4649
$this->userId = $userId;
4750
$this->db = $db;
4851
$this->logger = $logger;
4952
$this->userHelper = $userHelper;
5053
$this->columnsHelper = $columnsHelper;
5154
$this->columnMapper = $columnMapper;
55+
$this->rowLoaders = [
56+
RowLoader\RowLoader::LOADER_NORMALIZED => $normalizedRowLoader,
57+
RowLoader\RowLoader::LOADER_CACHED => $cachedRowLoader,
58+
];
5259
}
5360

5461
/**
@@ -60,7 +67,7 @@ public function delete(Row2 $row): Row2 {
6067
$this->db->beginTransaction();
6168
try {
6269
foreach ($this->columnsHelper->columns as $columnType) {
63-
$this->getCellMapperFromType($columnType)->deleteAllForRow($row->getId());
70+
$this->columnsHelper->getCellMapperFromType($columnType)->deleteAllForRow($row->getId());
6471
}
6572
$this->rowSleeveMapper->deleteById($row->getId());
6673
$this->db->commit();
@@ -178,7 +185,7 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null,
178185
$wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $sort, $limit, $offset);
179186

180187
// Get rows without SQL sorting
181-
$rows = $this->getRows($wantedRowIdsArray, $showColumnIds);
188+
$rows = $this->getRows($wantedRowIdsArray, $showColumnIds, RowLoader\RowLoader::LOADER_CACHED);
182189

183190
// Sort rows in PHP to preserve the order from getWantedRowIds
184191
return $this->sortRowsByIds($rows, $wantedRowIdsArray);
@@ -191,92 +198,16 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null,
191198
/**
192199
* @param array $rowIds
193200
* @param array $columnIds
201+
* @param RowLoader\RowLoader::LOADER_* $loader
194202
* @return Row2[]
195203
* @throws InternalError
196204
*/
197-
private function getRows(array $rowIds, array $columnIds): array {
205+
private function getRows(array $rowIds, array $columnIds, string $loader = RowLoader\RowLoader::LOADER_NORMALIZED ): array {
198206
if (empty($rowIds)) {
199207
return [];
200208
}
201209

202-
$columnTypesCount = count($this->columnsHelper->columns);
203-
if ($columnTypesCount === 0) {
204-
return [];
205-
}
206-
207-
$columnsCount = count($columnIds);
208-
$maxParamsPerType = floor(self::MAX_DB_PARAMETERS / $columnTypesCount) ;
209-
$calculatedChunkSize = (int)($maxParamsPerType - $columnsCount);
210-
$chunkSize = max(1, $calculatedChunkSize);
211-
212-
$rowIdChunks = array_chunk($rowIds, $chunkSize);
213-
$chunks = [];
214-
foreach ($rowIdChunks as $chunkedRowIds) {
215-
$chunks[] = $this->getRowsChunk($chunkedRowIds, $columnIds);
216-
}
217-
218-
return array_merge(...$chunks);
219-
}
220-
221-
/**
222-
* Builds and executes the UNION ALL query for a specific chunk of rows.
223-
* @param array $rowIds
224-
* @param array $columnIds
225-
* @return Row2[]
226-
* @throws InternalError
227-
*/
228-
private function getRowsChunk(array $rowIds, array $columnIds): array {
229-
$qb = $this->db->getQueryBuilder();
230-
231-
$qbSqlForColumnTypes = null;
232-
foreach ($this->columnsHelper->columns as $columnType) {
233-
$qbTmp = $this->db->getQueryBuilder();
234-
$qbTmp->select('row_id', 'column_id', 'last_edit_at', 'last_edit_by')
235-
->selectAlias($qb->expr()->castColumn('value', IQueryBuilder::PARAM_STR), 'value');
236-
237-
// This is not ideal but I cannot think of a good way to abstract this away into the mapper right now
238-
// Ideally we dynamically construct this query depending on what additional selects the column type requires
239-
// however the union requires us to match the exact number of selects for each column type
240-
if ($columnType === Column::TYPE_USERGROUP) {
241-
$qbTmp->selectAlias($qb->expr()->castColumn('value_type', IQueryBuilder::PARAM_STR), 'value_type');
242-
} else {
243-
$qbTmp->selectAlias($qbTmp->createFunction('NULL'), 'value_type');
244-
}
245-
246-
$qbTmp
247-
->from('tables_row_cells_' . $columnType)
248-
->where($qb->expr()->in('column_id', $qb->createNamedParameter($columnIds, IQueryBuilder::PARAM_INT_ARRAY, ':columnIds')))
249-
->andWhere($qb->expr()->in('row_id', $qb->createNamedParameter($rowIds, IQueryBuilder::PARAM_INT_ARRAY, ':rowsIds')));
250-
251-
if ($qbSqlForColumnTypes) {
252-
$qbSqlForColumnTypes .= ' UNION ALL ' . $qbTmp->getSQL() . ' ';
253-
} else {
254-
$qbSqlForColumnTypes = '(' . $qbTmp->getSQL();
255-
}
256-
}
257-
$qbSqlForColumnTypes .= ')';
258-
259-
$qb->select('row_id', 'column_id', 'created_by', 'created_at', 't1.last_edit_by', 't1.last_edit_at', 'value', 'table_id')
260-
// Also should be more generic (see above)
261-
->addSelect('value_type')
262-
->from($qb->createFunction($qbSqlForColumnTypes), 't1')
263-
->innerJoin('t1', 'tables_row_sleeves', 'rs', 'rs.id = t1.row_id');
264-
265-
try {
266-
$result = $qb->executeQuery();
267-
} catch (Exception $e) {
268-
$this->logger->error($e->getMessage(), ['exception' => $e]);
269-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), );
270-
}
271-
272-
try {
273-
$sleeves = $this->rowSleeveMapper->findMultiple($rowIds);
274-
} catch (Exception $e) {
275-
$this->logger->error($e->getMessage(), ['exception' => $e]);
276-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
277-
}
278-
279-
return $this->parseEntities($result, $sleeves);
210+
return $this->rowLoaders[$loader]->getRows($rowIds, $columnIds);
280211
}
281212

282213
/**
@@ -652,61 +583,6 @@ private function getSqlOperator(string $operator, IQueryBuilder $qb, string $col
652583
}
653584
}
654585

655-
/**
656-
* @param IResult $result
657-
* @param RowSleeve[] $sleeves
658-
* @return Row2[]
659-
* @throws InternalError
660-
*/
661-
private function parseEntities(IResult $result, array $sleeves): array {
662-
$rows = [];
663-
foreach ($sleeves as $sleeve) {
664-
$rows[$sleeve->getId()] = new Row2();
665-
$rows[$sleeve->getId()]->setId($sleeve->getId());
666-
$rows[$sleeve->getId()]->setCreatedBy($sleeve->getCreatedBy());
667-
$rows[$sleeve->getId()]->setCreatedAt($sleeve->getCreatedAt());
668-
$rows[$sleeve->getId()]->setLastEditBy($sleeve->getLastEditBy());
669-
$rows[$sleeve->getId()]->setLastEditAt($sleeve->getLastEditAt());
670-
$rows[$sleeve->getId()]->setTableId($sleeve->getTableId());
671-
}
672-
673-
$rowValues = [];
674-
$keyToColumnId = [];
675-
$keyToRowId = [];
676-
$cellMapperCache = [];
677-
678-
while ($rowData = $result->fetch()) {
679-
if (!isset($rowData['row_id'], $rows[$rowData['row_id']])) {
680-
break;
681-
}
682-
683-
$column = $this->columnMapper->find($rowData['column_id']);
684-
$columnType = $column->getType();
685-
if (!isset($cellMapperCache[$columnType])) {
686-
$cellMapperCache[$columnType] = $this->getCellMapperFromType($columnType);
687-
}
688-
$value = $cellMapperCache[$columnType]->formatRowData($column, $rowData);
689-
$compositeKey = (string)$rowData['row_id'] . ',' . (string)$rowData['column_id'];
690-
if ($cellMapperCache[$columnType]->hasMultipleValues()) {
691-
if (array_key_exists($compositeKey, $rowValues)) {
692-
$rowValues[$compositeKey][] = $value;
693-
} else {
694-
$rowValues[$compositeKey] = [$value];
695-
}
696-
} else {
697-
$rowValues[$compositeKey] = $value;
698-
}
699-
$keyToColumnId[$compositeKey] = $rowData['column_id'];
700-
$keyToRowId[$compositeKey] = $rowData['row_id'];
701-
}
702-
703-
foreach ($rowValues as $compositeKey => $value) {
704-
$rows[$keyToRowId[$compositeKey]]->addCell($keyToColumnId[$compositeKey], $value);
705-
}
706-
707-
return array_values($rows);
708-
}
709-
710586
/**
711587
* @throws InternalError
712588
*/
@@ -735,7 +611,7 @@ public function insert(Row2 $row): Row2 {
735611
foreach ($row->getData() as $cell) {
736612
$cachedCells[$cell['columnId']] = $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy());
737613
}
738-
$rowSleeve->setCells(json_encode($cachedCells));
614+
$rowSleeve->setCachedCellsArray($cachedCells);
739615
$this->rowSleeveMapper->update($rowSleeve);
740616

741617
return $row;
@@ -768,7 +644,7 @@ public function update(Row2 $row): Row2 {
768644
foreach ($changedCells as $cell) {
769645
$cachedCells[$cell['columnId']] = $this->insertOrUpdateCell($rowSleeve->getId(), $cell['columnId'], $cell['value']);
770646
}
771-
$rowSleeve->setCachedCells(json_encode($cachedCells));
647+
$rowSleeve->setCachedCellsArray($cachedCells);
772648
$this->rowSleeveMapper->update($rowSleeve);
773649

774650
return $row;
@@ -916,18 +792,7 @@ private function insertOrUpdateCell(int $rowId, int $columnId, $value): array {
916792
}
917793

918794
private function getCellMapper(Column $column): RowCellMapperSuper {
919-
return $this->getCellMapperFromType($column->getType());
920-
}
921-
922-
private function getCellMapperFromType(string $columnType): RowCellMapperSuper {
923-
$cellMapperClassName = 'OCA\Tables\Db\RowCell' . ucfirst($columnType) . 'Mapper';
924-
/** @var RowCellMapperSuper $cellMapper */
925-
try {
926-
return Server::get($cellMapperClassName);
927-
} catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) {
928-
$this->logger->error($e->getMessage(), ['exception' => $e]);
929-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
930-
}
795+
return $this->columnsHelper->getCellMapperFromType($column->getType());
931796
}
932797

933798
/**

‎lib/Db/RowLoader/CachedRowLoader.php‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
namespace OCA\Tables\Db\RowLoader;
4+
5+
use OCA\Tables\Db\Row2;
6+
use OCA\Tables\Db\RowSleeve;
7+
use OCA\Tables\Db\RowSleeveMapper;
8+
use OCA\Tables\Errors\InternalError;
9+
use OCP\DB\Exception;
10+
use Psr\Log\LoggerInterface;
11+
12+
class CachedRowLoader implements RowLoader
13+
{
14+
public function __construct(
15+
private RowSleeveMapper $rowSleeveMapper,
16+
private LoggerInterface $logger,
17+
) {
18+
}
19+
20+
public function getRows(array $rowIds, array $columnIds): array {
21+
$chunks = [];
22+
foreach (array_chunk($rowIds, self::MAX_DB_PARAMETERS) as $chunkedRowIds) {
23+
$chunks[] = $this->getRowsChunk($chunkedRowIds, $columnIds);
24+
}
25+
26+
return array_merge(...$chunks);
27+
}
28+
29+
/**
30+
* @param array $rowIds
31+
* @param array $columnIds
32+
* @return Row2[]
33+
* @throws InternalError
34+
*/
35+
private function getRowsChunk(array $rowIds, array $columnIds): array {
36+
try {
37+
$sleeves = $this->rowSleeveMapper->findMultiple($rowIds);
38+
} catch (Exception $e) {
39+
$this->logger->error($e->getMessage(), ['exception' => $e]);
40+
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), $e->getCode(), $e);
41+
}
42+
43+
return $this->parseEntities($sleeves, $columnIds);
44+
}
45+
46+
/**
47+
* @param RowSleeve[] $sleeves
48+
* @param int[] $columnIds
49+
* @return Row2[]
50+
* @throws InternalError
51+
*/
52+
private function parseEntities(array $sleeves, array $columnIds): array {
53+
$rows = [];
54+
foreach ($sleeves as $sleeve) {
55+
$row = new Row2();
56+
$row->setId($sleeve->getId());
57+
$row->setCreatedBy($sleeve->getCreatedBy());
58+
$row->setCreatedAt($sleeve->getCreatedAt());
59+
$row->setLastEditBy($sleeve->getLastEditBy());
60+
$row->setLastEditAt($sleeve->getLastEditAt());
61+
$row->setTableId($sleeve->getTableId());
62+
63+
$cachedCells = $sleeve->getCachedCellsArray();
64+
foreach ($columnIds as $columnId) {
65+
if (isset($cachedCells[$columnId])) {
66+
$row->addCell($columnId, $cachedCells[$columnId]);
67+
}
68+
}
69+
70+
$rows[] = $row;
71+
}
72+
73+
return $rows;
74+
}
75+
}

0 commit comments

Comments
 (0)