Skip to content

Commit 5b5e444

Browse files
MauricioFauthMauricioFauth
authored
Merge pull request #18618 from kamil-tekiela/Refactor-routine-execution
Refactor routine execution
2 parents 1fc7f7f + 72c689b commit 5b5e444

21 files changed

Lines changed: 377 additions & 517 deletions

‎libraries/classes/ConfigStorage/Relation.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,8 @@ public function tryUpgradeTransformations(): bool
361361
// skips result sets of query as we are not interested in it
362362
/** @infection-ignore-all */
363363
do {
364-
$hasResult = (
365-
$this->dbi->moreResults(Connection::TYPE_CONTROL)
366-
&& $this->dbi->nextResult(Connection::TYPE_CONTROL)
367-
);
368-
} while ($hasResult);
364+
$hasResult = $this->dbi->nextResult(Connection::TYPE_CONTROL);
365+
} while ($hasResult !== false);
369366

370367
$error = $this->dbi->getError(Connection::TYPE_CONTROL);
371368

‎libraries/classes/Controllers/Import/ImportController.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,6 @@ public function __invoke(ServerRequest $request): void
738738
$GLOBALS['table'], // table
739739
null, // find_real_end
740740
null, // sql_query_for_bookmark - see below
741-
null, // extra_data
742741
null, // message_to_show
743742
null, // sql_data
744743
$GLOBALS['goto'], // goto

‎libraries/classes/Controllers/Sql/SqlController.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ public function __invoke(ServerRequest $request): void
4949
$GLOBALS['unlim_num_rows'] ??= null;
5050
$GLOBALS['import_text'] ??= null;
5151
$GLOBALS['disp_query'] ??= null;
52-
$GLOBALS['extra_data'] ??= null;
5352
$GLOBALS['message_to_show'] ??= null;
5453
$GLOBALS['disp_message'] ??= null;
5554
$GLOBALS['complete_query'] ??= null;
@@ -207,7 +206,6 @@ public function __invoke(ServerRequest $request): void
207206
$GLOBALS['table'],
208207
$GLOBALS['find_real_end'] ?? null,
209208
$GLOBALS['import_text'] ?? null,
210-
$GLOBALS['extra_data'] ?? null,
211209
$GLOBALS['message_to_show'] ?? null,
212210
null,
213211
$GLOBALS['goto'],

‎libraries/classes/Controllers/Table/DeleteRowsController.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ public function __invoke(ServerRequest $request): void
9191
null,
9292
null,
9393
null,
94-
null,
9594
$GLOBALS['goto'] ?? '',
9695
$GLOBALS['disp_query'] ?? null,
9796
$GLOBALS['disp_message'] ?? null,

‎libraries/classes/Controllers/Table/SearchController.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,6 @@ public function doSelectionAction(): void
276276
$GLOBALS['table'], // table
277277
null, // find_real_end
278278
null, // sql_query_for_bookmark
279-
null, // extra_data
280279
null, // message_to_show
281280
null, // sql_data
282281
$GLOBALS['goto'], // goto

‎libraries/classes/Controllers/Table/Structure/BrowseController.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ private function displayTableBrowseForSelectedColumns(string $goto): void
6767
$GLOBALS['table'], // table
6868
null, // find_real_end
6969
null, // sql_query_for_bookmark
70-
null, // extra_data
7170
null, // message_to_show
7271
null, // sql_data
7372
$goto, // goto

‎libraries/classes/Database/MultiTableQuery.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ public static function displayResults(string $sqlQuery, string $db): string
9292
null, // table
9393
null, // find_real_end
9494
null, // sql_query_for_bookmark - see below
95-
null, // extra_data
9695
null, // message_to_show
9796
null, // sql_data
9897
$goto, // goto

‎libraries/classes/Database/Routines.php‎

Lines changed: 85 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ public function getQueryFromRequest(): string
846846
private function getQueriesFromRoutineForm(array $routine): array
847847
{
848848
$queries = [];
849-
$endQuery = [];
849+
$outParams = [];
850850
$args = [];
851851
$allFunctions = $this->dbi->types->getAllFunctions();
852852
for ($i = 0; $i < $routine['item_num_params']; $i++) {
@@ -856,23 +856,23 @@ private function getQueriesFromRoutineForm(array $routine): array
856856
$value = implode(',', $value);
857857
}
858858

859-
$value = $this->dbi->escapeString($value);
860859
if (
861860
! empty($_POST['funcs'][$routine['item_param_name'][$i]])
862861
&& in_array($_POST['funcs'][$routine['item_param_name'][$i]], $allFunctions)
863862
) {
864-
$queries[] = 'SET @p' . $i . '='
865-
. $_POST['funcs'][$routine['item_param_name'][$i]]
866-
. "('" . $value . "');\n";
863+
$queries[] = sprintf(
864+
'SET @p%d=%s(%s);',
865+
$i,
866+
$_POST['funcs'][$routine['item_param_name'][$i]],
867+
$this->dbi->quoteString($value),
868+
);
867869
} else {
868-
$queries[] = 'SET @p' . $i . "='" . $value . "';\n";
870+
$queries[] = 'SET @p' . $i . '=' . $this->dbi->quoteString($value) . ';';
869871
}
870-
871-
$args[] = '@p' . $i;
872-
} else {
873-
$args[] = '@p' . $i;
874872
}
875873

874+
$args[] = '@p' . $i;
875+
876876
if ($routine['item_type'] !== 'PROCEDURE') {
877877
continue;
878878
}
@@ -881,131 +881,120 @@ private function getQueriesFromRoutineForm(array $routine): array
881881
continue;
882882
}
883883

884-
$endQuery[] = '@p' . $i . ' AS '
885-
. Util::backquote($routine['item_param_name'][$i]);
884+
$outParams[] = '@p' . $i . ' AS ' . Util::backquote($routine['item_param_name'][$i]);
886885
}
887886

888887
if ($routine['item_type'] === 'PROCEDURE') {
889-
$queries[] = 'CALL ' . Util::backquote($routine['item_name'])
890-
. '(' . implode(', ', $args) . ");\n";
891-
if ($endQuery !== []) {
892-
$queries[] = 'SELECT ' . implode(', ', $endQuery) . ";\n";
888+
$queries[] = sprintf(
889+
'CALL %s(%s);',
890+
Util::backquote($routine['item_name']),
891+
implode(', ', $args),
892+
);
893+
if ($outParams !== []) {
894+
$queries[] = 'SELECT ' . implode(', ', $outParams) . ';';
893895
}
894896
} else {
895-
$queries[] = 'SELECT ' . Util::backquote($routine['item_name'])
896-
. '(' . implode(', ', $args) . ') '
897-
. 'AS ' . Util::backquote($routine['item_name'])
898-
. ";\n";
897+
$queries[] = sprintf(
898+
'SELECT %s(%s) AS %s;',
899+
Util::backquote($routine['item_name']),
900+
implode(', ', $args),
901+
Util::backquote($routine['item_name']),
902+
);
899903
}
900904

901905
return $queries;
902906
}
903907

904908
/**
905-
* @param mixed[]|null $routine
909+
* @param mixed[] $routine
906910
*
907911
* @psalm-return array{string, Message}
908912
*/
909-
public function handleExecuteRoutine(array|null $routine): array
913+
public function handleExecuteRoutine(array $routine): array
910914
{
911-
$queries = is_array($routine) ? $this->getQueriesFromRoutineForm($routine) : [];
912-
913-
// Get all the queries as one SQL statement
914-
$multipleQuery = implode('', $queries);
915+
$queries = $this->getQueriesFromRoutineForm($routine);
915916

916-
$outcome = true;
917917
$affected = 0;
918-
919-
// Execute query
920-
if (! $this->dbi->tryMultiQuery($multipleQuery)) {
921-
$outcome = false;
922-
}
923-
924-
// Generate output
925-
$output = '';
918+
$resultHtmlTables = '';
926919
$nbResultsetToDisplay = 0;
927-
if ($outcome) {
928-
// Pass the SQL queries through the "pretty printer"
929-
$output = Generator::formatSql(implode("\n", $queries));
930-
931-
// Display results
932-
$output .= '<div class="card my-3"><div class="card-header">';
933-
$output .= sprintf(
934-
__('Execution results of routine %s'),
935-
Util::backquote(htmlspecialchars($routine['item_name'])),
936-
);
937-
$output .= '</div><div class="card-body">';
938920

939-
do {
940-
$result = $this->dbi->storeResult();
921+
foreach ($queries as $query) {
922+
$result = $this->dbi->tryQuery($query);
941923

942-
if ($result !== false && $result->numRows() > 0) {
943-
$output .= '<table class="table table-striped w-auto"><tr>';
924+
// Generate output
925+
while ($result !== false) {
926+
if ($result->numRows() > 0) {
927+
$resultHtmlTables .= '<table class="table table-striped w-auto"><tr>';
944928
foreach ($result->getFieldNames() as $field) {
945-
$output .= '<th>';
946-
$output .= htmlspecialchars($field);
947-
$output .= '</th>';
929+
$resultHtmlTables .= '<th>';
930+
$resultHtmlTables .= htmlspecialchars($field);
931+
$resultHtmlTables .= '</th>';
948932
}
949933

950-
$output .= '</tr>';
934+
$resultHtmlTables .= '</tr>';
951935

952936
foreach ($result as $row) {
953-
$output .= '<tr>' . $this->browseRow($row) . '</tr>';
937+
$resultHtmlTables .= '<tr>' . $this->browseRow($row) . '</tr>';
954938
}
955939

956-
$output .= '</table>';
940+
$resultHtmlTables .= '</table>';
957941
$nbResultsetToDisplay++;
958942
$affected = $result->numRows();
959943
}
960944

961-
if (! $this->dbi->moreResults()) {
962-
break;
963-
}
964-
965-
unset($result);
966-
967-
$outcome = $this->dbi->nextResult();
968-
} while ($outcome);
969-
}
945+
$result = $this->dbi->nextResult();
946+
}
970947

971-
if ($outcome) {
972-
$output .= '</div></div>';
973-
974-
$message = __('Your SQL query has been executed successfully.');
975-
if ($routine['item_type'] === 'PROCEDURE') {
976-
$message .= '<br>';
977-
978-
// TODO : message need to be modified according to the
979-
// output from the routine
980-
$message .= sprintf(
981-
_ngettext(
982-
'%d row affected by the last statement inside the procedure.',
983-
'%d rows affected by the last statement inside the procedure.',
984-
(int) $affected,
985-
),
986-
$affected,
948+
// We must check for an error after fetching the results because
949+
// either tryQuery might have produced an error or any of nextResult calls.
950+
if ($this->dbi->getError() !== '') {
951+
$message = Message::error(
952+
sprintf(
953+
__('The following query has failed: "%s"'),
954+
htmlspecialchars($query),
955+
)
956+
. '<br><br>'
957+
. __('MySQL said: ') . $this->dbi->getError(),
987958
);
959+
960+
return ['', $message];
988961
}
962+
}
989963

990-
$message = Message::success($message);
964+
// Pass the SQL queries through the "pretty printer"
965+
$output = Generator::formatSql(implode("\n", $queries));
966+
// Display results
967+
$output .= '<div class="card my-3"><div class="card-header">';
968+
$output .= sprintf(
969+
__('Execution results of routine %s'),
970+
htmlspecialchars(Util::backquote($routine['item_name'])),
971+
);
972+
$output .= '</div><div class="card-body">';
973+
$output .= $resultHtmlTables;
974+
$output .= '</div></div>';
991975

992-
if ($nbResultsetToDisplay == 0) {
993-
$notice = __('MySQL returned an empty result set (i.e. zero rows).');
994-
$output .= Message::notice($notice)->getDisplay();
995-
}
996-
} else {
997-
$output = '';
998-
$message = Message::error(
999-
sprintf(
1000-
__('The following query has failed: "%s"'),
1001-
htmlspecialchars($multipleQuery),
1002-
)
1003-
. '<br><br>'
1004-
. __('MySQL said: ') . $this->dbi->getError(),
976+
$message = __('Your SQL query has been executed successfully.');
977+
if ($routine['item_type'] === 'PROCEDURE') {
978+
$message .= '<br>';
979+
980+
// TODO : message need to be modified according to the
981+
// output from the routine
982+
$message .= sprintf(
983+
_ngettext(
984+
'%d row affected by the last statement inside the procedure.',
985+
'%d rows affected by the last statement inside the procedure.',
986+
(int) $affected,
987+
),
988+
$affected,
1005989
);
1006990
}
1007991

1008-
return [$output, $message];
992+
if ($nbResultsetToDisplay === 0) {
993+
$notice = __('MySQL returned an empty result set (i.e. zero rows).');
994+
$output .= Message::notice($notice)->getDisplay();
995+
}
996+
997+
return [$output, Message::success($message)];
1009998
}
1010999

10111000
/**

‎libraries/classes/DatabaseInterface.php‎

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,44 +1626,19 @@ public function selectDb(string|DatabaseName $dbname, int $connectionType = Conn
16261626
return $this->extension->selectDb($dbname, $this->connections[$connectionType]);
16271627
}
16281628

1629-
/**
1630-
* Check if there are any more query results from a multi query
1631-
*
1632-
* @psalm-param ConnectionType $connectionType
1633-
*/
1634-
public function moreResults(int $connectionType = Connection::TYPE_USER): bool
1635-
{
1636-
if (! isset($this->connections[$connectionType])) {
1637-
return false;
1638-
}
1639-
1640-
return $this->extension->moreResults($this->connections[$connectionType]);
1641-
}
1642-
16431629
/**
16441630
* Prepare next result from multi_query
16451631
*
16461632
* @psalm-param ConnectionType $connectionType
16471633
*/
1648-
public function nextResult(int $connectionType = Connection::TYPE_USER): bool
1634+
public function nextResult(int $connectionType = Connection::TYPE_USER): ResultInterface|false
16491635
{
16501636
if (! isset($this->connections[$connectionType])) {
16511637
return false;
16521638
}
16531639

1654-
return $this->extension->nextResult($this->connections[$connectionType]);
1655-
}
1656-
1657-
/**
1658-
* Store the result returned from multi query
1659-
*
1660-
* @psalm-param ConnectionType $connectionType
1661-
*
1662-
* @return ResultInterface|false false when empty results / result set when not empty
1663-
*/
1664-
public function storeResult(int $connectionType = Connection::TYPE_USER): ResultInterface|false
1665-
{
1666-
if (! isset($this->connections[$connectionType])) {
1640+
// TODO: Figure out if we really need to check the return value of this function.
1641+
if (! $this->extension->nextResult($this->connections[$connectionType])) {
16671642
return false;
16681643
}
16691644

‎libraries/classes/Dbal/DbalInterface.php‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -488,28 +488,12 @@ public function connect(Server $currentServer, int $connectionType, int|null $ta
488488
*/
489489
public function selectDb(string|DatabaseName $dbname, int $connectionType = Connection::TYPE_USER): bool;
490490

491-
/**
492-
* Check if there are any more query results from a multi query
493-
*
494-
* @psalm-param ConnectionType $connectionType
495-
*/
496-
public function moreResults(int $connectionType = Connection::TYPE_USER): bool;
497-
498491
/**
499492
* Prepare next result from multi_query
500493
*
501494
* @psalm-param ConnectionType $connectionType
502495
*/
503-
public function nextResult(int $connectionType = Connection::TYPE_USER): bool;
504-
505-
/**
506-
* Store the result returned from multi query
507-
*
508-
* @psalm-param ConnectionType $connectionType
509-
*
510-
* @return mixed false when empty results / result set when not empty
511-
*/
512-
public function storeResult(int $connectionType = Connection::TYPE_USER): mixed;
496+
public function nextResult(int $connectionType = Connection::TYPE_USER): ResultInterface|false;
513497

514498
/**
515499
* Returns a string representing the type of connection used

0 commit comments

Comments
 (0)