Preserve check constraint during column move

Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>

src/Controllers/Table/Structure/MoveColumnsController.php

@@ -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]),
@@ -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);
+    }
 }

tests/unit/Controllers/Table/Structure/MoveColumnsControllerTest.php

@@ -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,
+            ],
+        ];
+    }
+}