Skip to content

Commit

Permalink
Preserve check constraint during column move
Browse files Browse the repository at this point in the history
Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>
  • Loading branch information
MoonE committed Apr 19, 2024
1 parent 5cc606a commit b852c27
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 99 deletions.
8 changes: 7 additions & 1 deletion resources/js/src/table/structure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,15 @@ AJAX.registerOnload('table/structure.js', function () {
designerModalPreviewModal.addEventListener('shown.bs.modal', () => {
const modalBody = designerModalPreviewModal.querySelector('.modal-body');
const $form = $('#move_column_form');
const serialized = $form.serialize();
if (serialized === $form.data('serialized-unmoved')) {
modalBody.innerHTML = '';
return;

Check failure on line 344 in resources/js/src/table/structure.ts

View workflow job for this annotation

GitHub Actions / Lint JavaScript files

Expected blank line before this statement
}

const formUrl = $form.attr('action');
const sep = CommonParams.get('arg_separator');
const formData = $form.serialize() +
const formData = serialized +
sep + 'preview_sql=1' +
sep + 'ajax_request=1';
$.post({
Expand Down
173 changes: 75 additions & 98 deletions src/Controllers/Table/Structure/MoveColumnsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,136 +11,54 @@
use PhpMyAdmin\Http\ServerRequest;
use PhpMyAdmin\Message;
use PhpMyAdmin\ResponseRenderer;
use PhpMyAdmin\Table\Table;
use PhpMyAdmin\SqlParser\Components\CreateDefinition;
use PhpMyAdmin\SqlParser\Parser;
use PhpMyAdmin\SqlParser\Statements\CreateStatement;
use PhpMyAdmin\Template;
use PhpMyAdmin\Util;

use function __;
use function array_column;
use function array_diff;
use function array_is_list;
use function array_keys;
use function array_search;
use function array_splice;
use function assert;
use function count;
use function implode;
use function in_array;
use function is_array;
use function mb_strtoupper;
use function sprintf;
use function str_replace;

final class MoveColumnsController implements InvocableController
{
private Table $tableObj;

public function __construct(
private readonly ResponseRenderer $response,
private readonly Template $template,
private readonly DatabaseInterface $dbi,
) {
$this->tableObj = $this->dbi->getTable(Current::$database, Current::$table);
}

public function __invoke(ServerRequest $request): Response|null
{
$moveColumns = $request->getParsedBodyParam('move_columns');
if (! is_array($moveColumns) || ! $request->isAjax()) {
$previewSql = $request->getParsedBodyParam('preview_sql') === '1';
if (! is_array($moveColumns) || ! array_is_list($moveColumns) || ! $this->response->isAjax()) {
$this->response->setRequestStatus(false);

return null;
}

$this->dbi->selectDb(Current::$database);

/**
* load the definitions for all columns
*/
$columns = $this->dbi->getColumnsFull(Current::$database, Current::$table);
$columnNames = array_column($columns, 'Field');
$changes = [];

// @see https://mariadb.com/kb/en/library/changes-improvements-in-mariadb-102/#information-schema
$usesLiteralNull = $this->dbi->isMariaDB() && $this->dbi->getVersion() >= 100200;
$defaultNullValue = $usesLiteralNull ? 'NULL' : null;
// move columns from first to last
for ($i = 0, $l = count($moveColumns); $i < $l; $i++) {
$column = $moveColumns[$i];
// is this column already correctly placed?
if ($columnNames[$i] == $column) {
continue;
}

// it is not, let's move it to index $i
$data = $columns[$column];
$extractedColumnSpec = Util::extractColumnSpec($data['Type']);
if (isset($data['Extra']) && $data['Extra'] === 'on update CURRENT_TIMESTAMP') {
$extractedColumnSpec['attribute'] = $data['Extra'];
unset($data['Extra']);
}
$createTableSql = $this->dbi->getTable(Current::$database, Current::$table)->showCreate();
$sqlQuery = $this->generateAlterTableSql($createTableSql, $moveColumns);

$timeType = $data['Type'] === 'timestamp' || $data['Type'] === 'datetime';
$timeDefault = $data['Default'] === 'CURRENT_TIMESTAMP' || $data['Default'] === 'current_timestamp()';
$currentTimestamp = $timeType && $timeDefault;

$uuidType = $data['Type'] === 'uuid';
$uuidDefault = $data['Default'] === 'UUID' || $data['Default'] === 'uuid()';
$uuid = $uuidType && $uuidDefault;

// @see https://mariadb.com/kb/en/library/information-schema-columns-table/#examples
if ($data['Null'] === 'YES' && in_array($data['Default'], [$defaultNullValue, null])) {
$defaultType = 'NULL';
} elseif ($currentTimestamp) {
$defaultType = 'CURRENT_TIMESTAMP';
} elseif ($uuid) {
$defaultType = 'UUID';
} elseif ($data['Default'] === null) {
$defaultType = 'NONE';
} else {
$defaultType = 'USER_DEFINED';
}

$virtual = ['VIRTUAL', 'PERSISTENT', 'VIRTUAL GENERATED', 'STORED GENERATED'];
$data['Virtuality'] = '';
$data['Expression'] = '';
if (isset($data['Extra']) && in_array($data['Extra'], $virtual, true)) {
$data['Virtuality'] = str_replace(' GENERATED', '', $data['Extra']);
$expressions = $this->tableObj->getColumnGenerationExpression($column);
$data['Expression'] = is_array($expressions) ? $expressions[$column] : null;
}

$changes[] = 'CHANGE ' . Table::generateAlter(
$column,
$column,
mb_strtoupper($extractedColumnSpec['type']),
$extractedColumnSpec['spec_in_brackets'],
$extractedColumnSpec['attribute'],
$data['Collation'] ?? '',
$data['Null'] === 'YES' ? 'YES' : 'NO',
$defaultType,
$data['Default'] ?? '',
$data['Extra'] ?? '',
$data['COLUMN_COMMENT'] ?? '',
$data['Virtuality'],
$data['Expression'],
$i === 0 ? '-first' : $columnNames[$i - 1],
);
// update current column_names array, first delete old position
$columnNames = array_diff($columnNames, [$column]);

// insert moved column
array_splice($columnNames, $i, 0, $column);
}

if ($changes === [] && ! isset($_REQUEST['preview_sql'])) { // should never happen
if ($sqlQuery === null) {
$this->response->setRequestStatus(false);

return null;
}

// query for moving the columns
$sqlQuery = sprintf(
'ALTER TABLE %s %s',
Util::backquote(Current::$table),
implode(', ', $changes),
);

if (isset($_REQUEST['preview_sql'])) { // preview sql
if ($previewSql) {
$this->response->addJSON(
'sql_data',
$this->template->render('preview_sql', ['query_data' => $sqlQuery]),
Expand All @@ -162,8 +80,67 @@ public function __invoke(ServerRequest $request): Response|null
__('The columns have been moved successfully.'),
);
$this->response->addJSON('message', $message);
$this->response->addJSON('columns', $columnNames);
$this->response->addJSON('columns', $moveColumns);

return null;
}

/**
* @param array<int,mixed> $moveColumns
* @psalm-param list<mixed> $moveColumns
*/
private function generateAlterTableSql(string $createTableSql, array $moveColumns): string|null
{
$parser = new Parser($createTableSql);
/** @var CreateStatement $statement */
$statement = $parser->statements[0];
/** @var CreateDefinition[] $fields */
$fields = $statement->fields;
$columns = [];
foreach ($fields as $field) {
if ($field->name === null) {
continue;
}

$columns[$field->name] = $field;
}

$columnNames = array_keys($columns);
// Ensure the columns from client match the columns from the table
if (
count($columnNames) !== count($moveColumns) ||
array_diff($columnNames, $moveColumns) !== []
) {
return null;
}

$changes = [];

// move columns from first to last
/** @psalm-var list<string> $moveColumns */
foreach ($moveColumns as $i => $columnName) {
// is this column already correctly placed?
if ($columnNames[$i] == $columnName) {
continue;
}

$changes[] =
'CHANGE ' . Util::backquote($columnName) . ' ' . $columns[$columnName]->build() .
($i === 0 ? ' FIRST' : ' AFTER ' . Util::backquote($columnNames[$i - 1]));

// Move column to its new position
/** @var int $j */
$j = array_search($columnName, $columnNames, true);
array_splice($columnNames, $j, 1);
array_splice($columnNames, $i, 0, $columnName);
}

if ($changes === []) {
return null;
}

assert($statement->name !== null, 'Alter table statement has no name');

return 'ALTER TABLE ' . Util::backquote($statement->name->table) . "\n " . implode(",\n ", $changes);
}
}
120 changes: 120 additions & 0 deletions tests/unit/Controllers/Table/Structure/MoveColumnsControllerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\Tests\Controllers\Table\Structure;

use PhpMyAdmin\Controllers\Table\Structure\MoveColumnsController;
use PhpMyAdmin\Current;
use PhpMyAdmin\Template;
use PhpMyAdmin\Tests\AbstractTestCase;
use PhpMyAdmin\Tests\Stubs\ResponseRenderer as ResponseStub;
use ReflectionClass;

use function preg_replace;

/** @covers \PhpMyAdmin\Controllers\Table\Structure\MoveColumnsController */
class MoveColumnsControllerTest extends AbstractTestCase
{
/**
* @param array<int,string> $columnNames
* @psalm-param list<string> $columnNames
*
* @dataProvider providerForTestGenerateAlterTableSql
*/
public function testGenerateAlterTableSql(string $createStatement, array $columnNames, string|null $expected): void
{
$class = new ReflectionClass(MoveColumnsController::class);
$method = $class->getMethod('generateAlterTableSql');

Current::$database = 'test-db';
Current::$table = 'test';
$dummyDbi = $this->createDbiDummy();
$dbi = $this->createDatabaseInterface($dummyDbi);

$controller = new MoveColumnsController(
new ResponseStub(),
new Template(),
$dbi,
);
/** @var string|null $alterStatement */
$alterStatement = $method->invoke($controller, $createStatement, $columnNames);

$expected = $expected === null ? null : preg_replace('/\r?\n/', "\n", $expected);
$alterStatement = $alterStatement === null ? null : preg_replace('/\r?\n/', "\n", $alterStatement);
self::assertSame($expected, $alterStatement);
}

/**
* Data provider for testGenerateAlterTableSql
*
* @return array<array<string[]|string|null>>
* @psalm-return list<array{string,list<string>,string}>
*/
public static function providerForTestGenerateAlterTableSql(): array
{
return [
// MariaDB / column CHECK constraint
[
<<<'SQL'
CREATE TABLE `test` (
`id` int(11) NOT NULL AUTO_INCREMENT,
`name` varchar(45) DEFAULT NULL,
`data` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL CHECK (json_valid(`json`)),
PRIMARY KEY (`id`)
)
SQL,
['id', 'data', 'name'],
<<<'SQL'
ALTER TABLE `test`
CHANGE `data` `data` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL CHECK (json_valid(`json`)) AFTER `id`
SQL,
],
// MariaDB / text column with uuid() default
[
<<<'SQL'
CREATE TABLE `test` (
`Id` int(11) NOT NULL,
`First` text NOT NULL DEFAULT uuid(),
`Second` text NOT NULL DEFAULT uuid()
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci
SQL,
['Id', 'Second', 'First'],
<<<'SQL'
ALTER TABLE `test`
CHANGE `Second` `Second` text NOT NULL DEFAULT uuid() AFTER `Id`
SQL,
],
// MySQL 8.0.13 text column with uuid() default
[
<<<'SQL'
CREATE TABLE `test` (
`Id` int(11) NOT NULL,
`First` text COLLATE utf8mb4_general_ci NOT NULL DEFAULT (uuid()),
`Second` text COLLATE utf8mb4_general_ci NOT NULL DEFAULT (uuid())
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci
SQL,
['Id', 'Second', 'First'],
<<<'SQL'
ALTER TABLE `test`
CHANGE `Second` `Second` text COLLATE utf8mb4_general_ci NOT NULL DEFAULT (uuid()) AFTER `Id`
SQL,
],
// enum with default
[
<<<'SQL'
CREATE TABLE `test` (
`id` int(11) NOT NULL,
`enum` enum('yes','no') CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'no',
PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci
SQL,
['enum', 'id'],
<<<'SQL'
ALTER TABLE `test`
CHANGE `enum` `enum` enum('yes','no') CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'no' FIRST
SQL,
],
];
}
}

0 comments on commit b852c27

Please sign in to comment.