diff --git a/Sources/Db/APIs/MySQL.php b/Sources/Db/APIs/MySQL.php index d7a3e47497..cbac5a1c0a 100644 --- a/Sources/Db/APIs/MySQL.php +++ b/Sources/Db/APIs/MySQL.php @@ -590,7 +590,7 @@ public function update_from(array $table, array $from_tables, string $set, strin return false; } - return $this->query( + $result = $this->query( 'UPDATE ' . $table['name'] . ' AS ' . $table['alias'] . ' ' . implode(' ', $joins) . ' @@ -599,6 +599,8 @@ public function update_from(array $table, array $from_tables, string $set, strin $db_values, $connection, ); + + return $result !== false; } /** @@ -2276,6 +2278,13 @@ public function remove_index(string $table_name, string $index_name, array $para { $short_table_name = str_replace('{db_prefix}', $this->prefix, $table_name); + // The list_indexes() method will report the name of the primary key as + // 'primary' on MySQL and 'pkey' on PostgreSQL. If we were handed the + // name for the wrong database engine, fix it. + if ($index_name === 'pkey') { + $index_name = 'primary'; + } + // Better exist! $indexes = $this->list_indexes($table_name, true); diff --git a/Sources/Db/APIs/PostgreSQL.php b/Sources/Db/APIs/PostgreSQL.php index 6c4c0fa6a9..103e109e04 100644 --- a/Sources/Db/APIs/PostgreSQL.php +++ b/Sources/Db/APIs/PostgreSQL.php @@ -135,6 +135,13 @@ class PostgreSQL extends DatabaseApi implements DatabaseApiInterface */ protected $connect_errno; + /** + * @var array + * + * Cache for list_indexes() method. + */ + private array $index_cache = []; + /**************** * Public methods ****************/ @@ -433,23 +440,48 @@ public function insert(string $method, string $table, array $columns, array $dat // PostgreSQL doesn't support replace: we implement a MySQL-compatible behavior instead if ($method == 'replace' || $method == 'ignore') { - $key_str = implode(',', $keys); - $col_str = ''; - $count = 0; + // The columns in an ON CONFLICT statement must exactly match the columns + // of some primary or unique index. + $possibly_conflicting_columns = []; + $column_names = array_keys($columns); - // Make a list of the non-pk fields. - foreach ($columns as $columnName => $type) { - if (!\in_array($columnName, $keys) && ($method == 'replace')) { - $col_str .= ($count > 0 ? ',' : ''); - $col_str .= $columnName . ' = EXCLUDED.' . $columnName; - $count++; + foreach ($this->list_indexes($table, true) as $index) { + if ( + // Skip if not a primary or unique index. + !\in_array($index['type'], ['primary', 'unique']) + // Skip if some of the columns in this index are not being inserted into. + || array_intersect($index['columns'], $column_names) !== $index['columns'] + // Prefer the primary index over others. + || ($index['type'] !== 'primary' && !empty($possibly_conflicting_columns)) + ) { + continue; } + + $possibly_conflicting_columns = $index['columns']; } - if ($method == 'replace') { - $replace = ' ON CONFLICT (' . $key_str . ') DO UPDATE SET ' . $col_str; - } else { - $replace = ' ON CONFLICT (' . $key_str . ') DO NOTHING'; + if (!empty($possibly_conflicting_columns)) { + $key_str = implode(',', $possibly_conflicting_columns); + $col_str = ''; + $count = 0; + + // Make a list of the non-pk fields. + foreach ($columns as $column_name => $type) { + if ( + !\in_array($column_name, $possibly_conflicting_columns) + && $method == 'replace' + ) { + $col_str .= ($count > 0 ? ',' : ''); + $col_str .= $column_name . ' = EXCLUDED.' . $column_name; + $count++; + } + } + + if ($method == 'replace' && !empty($col_str)) { + $replace = ' ON CONFLICT (' . $key_str . ') DO UPDATE SET ' . $col_str; + } else { + $replace = ' ON CONFLICT (' . $key_str . ') DO NOTHING'; + } } } @@ -595,7 +627,7 @@ public function update_from(array $table, array $from_tables, string $set, strin // PostgreSQL doesn't like prefixes on the columns to be set. $set = preg_replace('~\b' . $table['alias'] . '\.\b~', '', $set); - return $this->query( + $result = $this->query( 'UPDATE ' . $table['name'] . ' AS ' . $table['alias'] . ' SET ' . $set . ' FROM ' . implode(', ', $from) . (!empty($where) ? ' @@ -603,6 +635,8 @@ public function update_from(array $table, array $from_tables, string $set, strin $db_values, $connection, ); + + return $result !== false; } /** @@ -1360,6 +1394,8 @@ public function add_column(string $table_name, array $column_info, array $parame return $this->change_column($table_name, $column_info['name'], $column_info); } + unset($this->index_cache[$short_table_name]); + return $result !== false; } @@ -1435,6 +1471,8 @@ public function add_index(string $table_name, array $index_info, array $paramete ); } + unset($this->index_cache[$parsed_table_name]); + // Query returns a result or true if successful, false otherwise. return $result !== false; } @@ -1707,11 +1745,23 @@ public function change_column(string $table_name, string $old_column, array $col 'security_override' => true, ], ); + + // In PostgreSQL SET DEFAULT does not backfill existing rows, so do it manually. + if ($default !== 'NULL' && !empty($column_info['not_null'])) { + $this->query( + 'UPDATE ' . $short_table_name . ' + SET ' . $column_info['name'] . ' = ' . $default . ' + WHERE ' . $column_info['name'] . ' IS NULL', + [ + 'security_override' => true, + ], + ); + } } // Is it null - or otherwise? // Just go ahead & honor the setting. Type changes above introduce defaults that we might need to override here... - if (isset($column_info['not_null'])) { + if (!empty($column_info['not_null'])) { $action = 'SET NOT NULL'; } else { $action = 'DROP NOT NULL'; @@ -1725,6 +1775,8 @@ public function change_column(string $table_name, string $old_column, array $col ], ); + unset($this->index_cache[$short_table_name]); + return true; } @@ -1749,6 +1801,8 @@ public function rename_index(string $table_name, string $old_name, string $new_n ); } + unset($this->index_cache[$parsed_table_name]); + return $result !== false; } @@ -1799,6 +1853,8 @@ public function create_table(string $table_name, array $columns, array $indexes } } + unset($this->index_cache[$short_table_name]); + // If we've got this far - good news - no table exists. We can build our own! if (!$db_trans) { $this->transaction('begin'); @@ -1982,6 +2038,8 @@ public function drop_table(string $table_name, array $parameters = [], string $e $tables = $this->list_tables($database); if (\in_array($full_table_name, $tables)) { + unset($this->index_cache[$short_table_name]); + // We can then drop the table. $this->transaction('begin'); @@ -2051,6 +2109,8 @@ public function rename_table(string $old_name, string $new_name, bool $allowed_r return false; } + unset($this->index_cache[$short_old_name]); + $result = $this->query( 'ALTER TABLE ' . $short_old_name . ' RENAME TO ' . $short_new_name, [ @@ -2149,6 +2209,10 @@ public function list_indexes(string $table_name, bool $detail = false, array $pa $real_table_name = preg_match('~^(`?)(.+?)\\1\\.(.*?)$~', $parsed_table_name, $match) === 1 ? $match[3] : $parsed_table_name; $database = !empty($match[2]) ? $match[2] : $this->name; + if (isset($this->index_cache[$parsed_table_name][$detail ? 'detail' : 'simple'])) { + return $this->index_cache[$parsed_table_name][$detail ? 'detail' : 'simple']; + } + $result = $this->query( 'SELECT CASE WHEN i.indisprimary THEN 1 ELSE 0 END AS is_primary, CASE WHEN i.indisunique THEN 1 ELSE 0 END AS is_unique, @@ -2177,15 +2241,12 @@ public function list_indexes(string $table_name, bool $detail = false, array $pa } foreach ($columns as $k => $v) { - $columns[$k] = trim($v); + // Remove the opclass suffix, if present. + $columns[$k] = preg_replace('/\s+\w+_ops$/', '', trim($v)); } - // Fix up the name to be consistent cross databases - if (str_ends_with($row['name'], '_pkey') && $row['is_primary'] == 1) { - $row['name'] = 'PRIMARY'; - } else { - $row['name'] = str_replace($real_table_name . '_', '', $row['name']); - } + // We only want the basic column name. + $row['name'] = str_replace($real_table_name . '_', '', $row['name']); if (!$detail) { $indexes[] = $row['name']; @@ -2197,8 +2258,16 @@ public function list_indexes(string $table_name, bool $detail = false, array $pa ]; } } + $this->free_result($result); + if ($detail) { + $this->index_cache[$parsed_table_name]['detail'] = $indexes; + $this->index_cache[$parsed_table_name]['simple'] = array_keys($indexes); + } else { + $this->index_cache[$parsed_table_name]['simple'] = $indexes; + } + return $indexes; } @@ -2209,6 +2278,8 @@ public function remove_column(string $table_name, string $column_name, array $pa { $short_table_name = str_replace('{db_prefix}', $this->prefix, $table_name); + unset($this->index_cache[$short_table_name]); + // Does it exist? $columns = $this->list_columns($table_name, true); @@ -2248,37 +2319,41 @@ public function remove_index(string $table_name, string $index_name, array $para $parsed_table_name = str_replace('{db_prefix}', $this->prefix, $table_name); $real_table_name = preg_match('~^(`?)(.+?)\\1\\.(.*?)$~', $parsed_table_name, $match) === 1 ? $match[3] : $parsed_table_name; + unset($this->index_cache[$parsed_table_name]); + + // The list_indexes() method will report the name of the primary key as + // 'primary' on MySQL and 'pkey' on PostgreSQL. If we were handed the + // name for the wrong database engine, fix it. + if ($index_name === 'primary') { + $index_name = 'pkey'; + } + // Better exist! $indexes = $this->list_indexes($table_name, true); - // Do not add the table name to the index if it is already there. - if ($index_name != 'primary' && str_contains($index_name, $real_table_name)) { - $index_name = str_replace($real_table_name . '_', '', $index_name); - } + // The list_indexes() method removes the table name from the names of + // the indexes, so make sure to do the same to $index_name. + $index_name = str_replace($real_table_name . '_', '', $index_name); foreach ($indexes as $index) { - // If the name is primary we want the primary key! - if ($index['type'] == 'primary' && $index_name == 'primary') { - // Dropping primary key? - $result = $this->query( - 'ALTER TABLE ' . $real_table_name . ' - DROP CONSTRAINT ' . $index['name'], - [ - 'security_override' => true, - ], - ); - - return $result !== false; - } + if ($index['name'] === $index_name) { + if ($index['type'] == 'primary') { + $result = $this->query( + 'ALTER TABLE ' . $real_table_name . ' + DROP CONSTRAINT ' . $real_table_name . '_' . $index['name'], + [ + 'security_override' => true, + ], + ); + } else { + $result = $this->query( + 'DROP INDEX ' . $real_table_name . '_' . $index['name'], + [ + 'security_override' => true, + ], + ); - if ($index['name'] == $index_name) { - // Drop the bugger... - $result = $this->query( - 'DROP INDEX ' . $real_table_name . '_' . $index_name, - [ - 'security_override' => true, - ], - ); + } return $result !== false; } diff --git a/Sources/Db/Schema/Table.php b/Sources/Db/Schema/Table.php index 73e2e75bfb..446a1edba5 100644 --- a/Sources/Db/Schema/Table.php +++ b/Sources/Db/Schema/Table.php @@ -463,13 +463,22 @@ public function fixIndexName(DbIndex $index): bool continue; } + // There's no need to rename the primary key. + if ($index->type === 'primary' && $existing_index['type'] === 'primary') { + return true; + } + // If the name is already the same, there's nothing to do. if ($index->name === $existing_index['name']) { return true; } // Do the rename. - return Db::$db->rename_index('{db_prefix}' . $this->name, $existing_index['name'], $index->name); + return Db::$db->rename_index( + table_name: '{db_prefix}' . $this->name, + old_name: $existing_index['name'], + new_name: $index->name, + ); } // No matching index was found. diff --git a/Sources/Db/Schema/v3_0/Calendar.php b/Sources/Db/Schema/v3_0/Calendar.php index e46e3e7af6..53ba0fef0a 100644 --- a/Sources/Db/Schema/v3_0/Calendar.php +++ b/Sources/Db/Schema/v3_0/Calendar.php @@ -511,13 +511,13 @@ public function __construct() name: 'rdates', type: 'text', not_null: true, - default: null, + default: '', ), 'exdates' => new Column( name: 'exdates', type: 'text', not_null: true, - default: null, + default: '', ), 'adjustments' => new Column( name: 'adjustments', diff --git a/Sources/Maintenance/Migration/v2_1/AlertsObsolete.php b/Sources/Maintenance/Migration/v2_1/AlertsObsolete.php index bf7bae504f..7e88d3e551 100644 --- a/Sources/Maintenance/Migration/v2_1/AlertsObsolete.php +++ b/Sources/Maintenance/Migration/v2_1/AlertsObsolete.php @@ -15,6 +15,7 @@ namespace SMF\Maintenance\Migration\v2_1; +use SMF\Db\DatabaseApi as Db; use SMF\Maintenance\Migration\MigrationBase; class AlertsObsolete extends MigrationBase @@ -104,17 +105,33 @@ public function execute(): bool $this->handleTimeout(); - $this->query( - 'UPDATE {db_prefix}user_alerts AS a - JOIN {db_prefix}attachments AS f - ON (f.id_attach = a.content_id) - SET - a.content_type = {literal:msg}, - a.content_action = {literal:unapproved_attachment}, - a.content_id = f.id_msg - WHERE content_type = {literal:unapproved} - AND content_action = {literal:attachment}', - [], + Db::$db->update_from( + table: [ + 'name' => '{db_prefix}user_alerts', + 'alias' => 'a', + ], + from_tables: [ + [ + 'name' => '{db_prefix}attachments', + 'alias' => 'f', + 'condition' => 'f.id_attach = a.content_id', + ], + ], + set: implode(', ', [ + 'a.content_type = {string:new_type}', + 'a.content_action = {string:new_action}', + 'a.content_id = f.id_msg', + ]), + where: implode(' AND ', [ + 'content_type = {string:old_type}', + 'content_action = {string:old_action}', + ]), + db_values: [ + 'new_type' => 'msg', + 'new_action' => 'unapproved_attachment', + 'old_type' => 'unapproved', + 'old_action' => 'attachment', + ], ); $this->handleTimeout(); diff --git a/Sources/Maintenance/Migration/v2_1/PostgreSqlSchemaDiff.php b/Sources/Maintenance/Migration/v2_1/PostgreSqlSchemaDiff.php index b1d91af1e4..19d7a5ca1f 100644 --- a/Sources/Maintenance/Migration/v2_1/PostgreSqlSchemaDiff.php +++ b/Sources/Maintenance/Migration/v2_1/PostgreSqlSchemaDiff.php @@ -129,7 +129,7 @@ public function isCandidate(): bool public function execute(): bool { while (Maintenance::getCurrentStart() < \count($this->schema_fixes)) { - $fix = $this->schemaFixes[Maintenance::getCurrentStart()]; + $fix = $this->schema_fixes[Maintenance::getCurrentStart()]; $this->query( 'ALTER TABLE {db_prefix}' . $fix[0] . ' diff --git a/Sources/Maintenance/Migration/v3_0/SearchResultsPrimaryKey.php b/Sources/Maintenance/Migration/v3_0/SearchResultsPrimaryKey.php index 3f4381c0e6..fb3ed536e9 100644 --- a/Sources/Maintenance/Migration/v3_0/SearchResultsPrimaryKey.php +++ b/Sources/Maintenance/Migration/v3_0/SearchResultsPrimaryKey.php @@ -48,9 +48,17 @@ public function isCandidate(): bool $table = new Schema\v3_0\LogSearchResults(); $existing_structure = $table->getCurrentStructure(); - $idx = $existing_structure['indexes']['primary'] ?? null; - - return $idx == null || array_intersect($idx['columns'], self::$columns) !== []; + foreach ($existing_structure['indexes'] as $idx) { + if ($idx['type'] === 'primary') { + break; + } + } + + return ( + !isset($idx) + || $idx['type'] !== 'primary' + || $idx['columns'] !== self::$columns + ); } /** @@ -63,7 +71,10 @@ public function execute(): bool $this->handleTimeout(); - Db::$db->add_index('{db_prefix}' . $table->name, ['type' => 'primary', 'columns' => self::$columns]); + Db::$db->add_index( + '{db_prefix}' . $table->name, + ['type' => 'primary', 'columns' => self::$columns], + ); return true; }