From 59ef19d1a8cacd1baaa5923baa81f173e7161d7d Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Tue, 28 Nov 2023 20:01:49 +0100 Subject: [PATCH] Use platform for column escaping in bulk insert --- .../src/Flow/Doctrine/Bulk/Columns.php | 5 ----- .../src/Flow/Doctrine/Bulk/DbalPlatform.php | 4 ++-- .../Doctrine/Bulk/Dialect/MySQLDialect.php | 17 +++++++++------ .../Bulk/Dialect/PostgreSQLDialect.php | 21 ++++++++----------- .../Doctrine/Bulk/Dialect/SqliteDialect.php | 17 +++++++++------ .../Doctrine/Bulk/Tests/Unit/ColumnsTest.php | 10 --------- 6 files changed, 33 insertions(+), 41 deletions(-) diff --git a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Columns.php b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Columns.php index 7eb7179d2..fd146d9b2 100644 --- a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Columns.php +++ b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Columns.php @@ -34,11 +34,6 @@ public function all() : array return $this->columns; } - public function concat(string $separator) : string - { - return \implode($separator, $this->columns); - } - /** * @param string ...$columnNames * diff --git a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/DbalPlatform.php b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/DbalPlatform.php index 9d0c623ac..39a59533f 100644 --- a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/DbalPlatform.php +++ b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/DbalPlatform.php @@ -29,11 +29,11 @@ public function dialect() : Dialect } if ($this->isMySQL() || $this->isMariaDB()) { - return new MySQLDialect(); + return new MySQLDialect($this->platform); } if ($this->isSqlite()) { - return new SqliteDialect(); + return new SqliteDialect($this->platform); } throw new RuntimeException(\sprintf( diff --git a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/MySQLDialect.php b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/MySQLDialect.php index e6b9c2022..02d0e0595 100644 --- a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/MySQLDialect.php +++ b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/MySQLDialect.php @@ -4,6 +4,7 @@ namespace Flow\Doctrine\Bulk\Dialect; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Flow\Doctrine\Bulk\BulkData; use Flow\Doctrine\Bulk\Columns; use Flow\Doctrine\Bulk\Exception\RuntimeException; @@ -11,6 +12,10 @@ final class MySQLDialect implements Dialect { + public function __construct(private readonly AbstractPlatform $platform) + { + } + /** * @psalm-suppress MoreSpecificImplementedParamType * @@ -30,7 +35,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s ON DUPLICATE KEY UPDATE %4$s=%4$s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders(), \current($bulkData->columns()->all()) ); @@ -42,7 +47,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array VALUES %s ON DUPLICATE KEY UPDATE %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders(), \array_key_exists('update_columns', $insertOptions) && \count($insertOptions['update_columns']) ? $this->updateSelectedColumns($insertOptions['update_columns'], $bulkData->columns()) @@ -53,7 +58,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -72,7 +77,7 @@ public function prepareUpdate(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'REPLACE INTO %s (%s) VALUES %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -87,7 +92,7 @@ private function updateAllColumns(Columns $columns) : string return \implode( ',', $columns->map( - fn (string $column) : string => "{$column} = VALUES({$column})" + fn (string $column) : string => "{$this->platform->quoteIdentifier($column)} = VALUES({$this->platform->quoteIdentifier($column)})" ) ); } @@ -101,7 +106,7 @@ private function updateAllColumns(Columns $columns) : string private function updateSelectedColumns(array $updateColumns, Columns $columns) : string { return \count($updateColumns) - ? \implode(',', \array_map(fn (string $column) : string => "{$column} = VALUES({$column})", $updateColumns)) + ? \implode(',', \array_map(fn (string $column) : string => "{$this->platform->quoteIdentifier($column)} = VALUES({$this->platform->quoteIdentifier($column)})", $updateColumns)) : $this->updateAllColumns($columns); } } diff --git a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/PostgreSQLDialect.php b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/PostgreSQLDialect.php index 8752a0f2d..9d6a71e1f 100644 --- a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/PostgreSQLDialect.php +++ b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/PostgreSQLDialect.php @@ -12,11 +12,8 @@ final class PostgreSQLDialect implements Dialect { - private AbstractPlatform $platform; - - public function __construct(AbstractPlatform $platform) + public function __construct(private readonly AbstractPlatform $platform) { - $this->platform = $platform; } /** @@ -39,7 +36,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s ON CONFLICT (%s) DO UPDATE SET %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders(), \implode(',', $insertOptions['conflict_columns']), (\array_key_exists('update_columns', $insertOptions) && \count($insertOptions['update_columns'])) @@ -52,7 +49,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s ON CONFLICT ON CONSTRAINT %s DO UPDATE SET %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders(), $insertOptions['constraint'], (\array_key_exists('update_columns', $insertOptions) && \count($insertOptions['update_columns'])) @@ -65,7 +62,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s ON CONFLICT DO NOTHING', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -73,7 +70,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -109,7 +106,7 @@ public function prepareUpdate(TableDefinition $table, BulkData $bulkData, array ? $this->updatedSelectedColumns($updateOptions['update_columns'], $bulkData->columns()->without(...$updateOptions['primary_key_columns'])) : $this->updateAllColumns($bulkData->columns()->without(...$updateOptions['primary_key_columns'])), $table->toSqlCastedPlaceholders($bulkData, $this->platform), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $this->updatedIndexColumns($updateOptions['primary_key_columns']) ); } @@ -129,7 +126,7 @@ private function updateAllColumns(Columns $columns) : string return \implode( ',', $columns->map( - fn (string $column) : string => "{$column} = excluded.{$column}" + fn (string $column) : string => "{$this->platform->quoteIdentifier($column)} = {$this->platform->quoteIdentifier('excluded.' . $column)}" ) ); } @@ -141,7 +138,7 @@ private function updateAllColumns(Columns $columns) : string */ private function updatedIndexColumns(array $updateColumns) : string { - return \implode(' AND ', \array_map(fn (string $column) : string => "existing_table.{$column} = excluded.{$column}", $updateColumns)); + return \implode(' AND ', \array_map(fn (string $column) : string => "{$this->platform->quoteIdentifier('existing_table.' . $column)} = {$this->platform->quoteIdentifier('excluded.' . $column)}", $updateColumns)); } /** @@ -158,7 +155,7 @@ private function updatedSelectedColumns(array $updateColumns, Columns $columns) * table's name (or an alias), and to rows proposed for insertion using the special EXCLUDED table. */ return \count($updateColumns) - ? \implode(',', \array_map(fn (string $column) : string => "{$column} = excluded.{$column}", $updateColumns)) + ? \implode(',', \array_map(fn (string $column) : string => "{$this->platform->quoteIdentifier($column)} = {$this->platform->quoteIdentifier('excluded.' . $column)}", $updateColumns)) : $this->updateAllColumns($columns); } } diff --git a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/SqliteDialect.php b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/SqliteDialect.php index 664dfa4b9..83bf913e7 100644 --- a/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/SqliteDialect.php +++ b/src/lib/doctrine-dbal-bulk/src/Flow/Doctrine/Bulk/Dialect/SqliteDialect.php @@ -4,12 +4,17 @@ namespace Flow\Doctrine\Bulk\Dialect; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Flow\Doctrine\Bulk\BulkData; use Flow\Doctrine\Bulk\Columns; use Flow\Doctrine\Bulk\TableDefinition; final class SqliteDialect implements Dialect { + public function __construct(private readonly AbstractPlatform $platform) + { + } + /** * @psalm-suppress MoreSpecificImplementedParamType * @@ -25,7 +30,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s ON CONFLICT (%s) DO UPDATE SET %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders(), \implode(',', $insertOptions['conflict_columns']), (\array_key_exists('update_columns', $insertOptions) && [] !== $insertOptions['update_columns']) @@ -38,7 +43,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s ON CONFLICT DO NOTHING', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -46,7 +51,7 @@ public function prepareInsert(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'INSERT INTO %s (%s) VALUES %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -56,7 +61,7 @@ public function prepareUpdate(TableDefinition $table, BulkData $bulkData, array return \sprintf( 'REPLACE INTO %s (%s) VALUES %s', $table->name(), - $bulkData->columns()->concat(','), + \implode(',', \array_map(fn (string $column) : string => $this->platform->quoteIdentifier($column), $bulkData->columns()->all())), $bulkData->toSqlPlaceholders() ); } @@ -66,7 +71,7 @@ private function updateAllColumns(Columns $columns) : string return \implode( ',', $columns->map( - fn (string $column) : string => "{$column} = excluded.{$column}" + fn (string $column) : string => "{$this->platform->quoteIdentifier($column)} = {$this->platform->quoteIdentifier('excluded.' . $column)}" ) ); } @@ -77,7 +82,7 @@ private function updateAllColumns(Columns $columns) : string private function updateSelectedColumns(array $updateColumns, Columns $columns) : string { return [] !== $updateColumns - ? \implode(',', \array_map(static fn (string $column) : string => "{$column} = excluded.{$column}", $updateColumns)) + ? \implode(',', \array_map(fn (string $column) : string => "{$this->platform->quoteIdentifier($column)} = {$this->platform->quoteIdentifier('excluded.' . $column)}", $updateColumns)) : $this->updateAllColumns($columns); } } diff --git a/src/lib/doctrine-dbal-bulk/tests/Flow/Doctrine/Bulk/Tests/Unit/ColumnsTest.php b/src/lib/doctrine-dbal-bulk/tests/Flow/Doctrine/Bulk/Tests/Unit/ColumnsTest.php index 61ed41aca..870911c8f 100644 --- a/src/lib/doctrine-dbal-bulk/tests/Flow/Doctrine/Bulk/Tests/Unit/ColumnsTest.php +++ b/src/lib/doctrine-dbal-bulk/tests/Flow/Doctrine/Bulk/Tests/Unit/ColumnsTest.php @@ -66,14 +66,4 @@ public function test_transforms_all_columns_to_placeholders() : void $columns->prefix(':') ); } - - public function test_transforms_columns_to_string_using_comma_as_a_separator() : void - { - $columns = new Columns('date', 'title', 'description', 'quantity'); - - $this->assertEquals( - 'date,title,description,quantity', - $columns->concat(',') - ); - } }